[PATCH] Fixes to scheduler

Juan Perez-Sanchez <[email protected]>
Newsgroups org.kernel.vger.linux-8086
Message-ID <CAD6VGuYw0wdHdD+HdTHKzNMaUWVyiep0=_GxZL-NAci6hEzQJA@mail.gmail.com>
Hi,

  This patch fixes some issues with scheduler code.

Greetings,

Juan

PREVIOUS OPERATION AND BUGS

1. Function "kernel/sched.c/run_timer_list()" should run through the
timer list removing structures whose timeout expired and issuing their
callbacks. This process ends prematurely because after removing the
first structure, the link to the next structure in the list is taken
from the removed timer, which by then is NULL.

2. Function "kernel/sched.c/run_timer_list()" is called on every
invocation to schedule(), but it's necessary to run it only when
variable jiffies is incremented.

3. In function "kernel/sched.c/schedule()", when looking for the next
task to run, checks for the next task not to be the idle task and
variable nr_running > 0. But it's only necessary to check for the idle
task.

4. If schedule() is called within an interrupt handler, task swapping
is skipped. By the moment when the skip is taken, it already allocated
a timer structure in the stack and might have it queued in the timer
list, and these actions are not reversed, corrupting the timer list.

NEW OPERATION

1. Function "run_timer_list()" was completely rewritten, avoiding its
original bug, exploiting the fact that the timers to be removed are
always at the beginning of the list, resulting in a more compact
implementation.

2. Function "run_timer_list()" is called only at the end of
"do_timer()" function.

3. Function "add_timer()" was completely rewritten, resulting in a
more compact implementation.

4. Checking if running within an interrupt handler and skipping task
switching is done before allocating a timer structure in the stack.
Removed unnecessary check of nr_running.

OTHER CHANGES

1. A small optimization to reduce code size was done in file irqtab.c.

   There is a reduction in code size of 80 bytes.

 The Image builded without errors. The kernel was tested with QEMU and
dioscuri emulators. Also in a PPro pc booting from floppy.
elksL.patch (application/octet-stream, 5.2 KB)
diff -Nurb elks.orig/arch/i86/kernel/irqtab.c elks/arch/i86/kernel/irqtab.c
--- elks.orig/arch/i86/kernel/irqtab.c	2012-07-07 10:05:25.000000000 -0500
+++ elks/arch/i86/kernel/irqtab.c	2012-07-08 16:34:27.000000000 -0500
@@ -63,19 +63,17 @@
         xor ax,ax
         mov es,ax      ;intr table
 
-	seg es                     ;insert new timer intr 
-	mov bx,[32]
-	mov off_stashed_irq0_l, bx   ; the old one
-	lea ax,_irq0
 	seg es
-	mov [32],ax
+	mov ax,[32]
+	mov off_stashed_irq0_l, ax   ; the old timer intr
 	seg es
-	mov bx,[34]
-	mov seg_stashed_irq0_l, bx
-	mov ax,cs
-	seg es
-	mov [34],ax
+	mov ax,[34]
+	mov seg_stashed_irq0_l, ax
 
+	seg es
+	mov [32],#_irq0   ;timer
+	seg es
+	mov [34],cs
 
 #ifndef CONFIG_CONSOLE_BIOS
 	seg es
@@ -98,21 +96,17 @@
 	seg es
         mov [46],cs
 	
-	lea ax,_irq4     ;com1
 	seg es
-	mov [48],ax
-	mov ax,cs
+	mov [48],#_irq4   ;com1
 	seg es
-	mov [50],ax
+	mov [50],cs
 	
 
 ! Setup INT 0x80 (for syscall)
-	lea ax,_syscall_int
 	seg es
-	mov [512],ax
-	mov ax,cs
+	mov [512],#_syscall_int
 	seg es
-	mov [514],ax
+	mov [514],cs
 ! Tidy up
 
         mov dx,ds      ;the original value
diff -Nurb elks.orig/fs/select.c elks/fs/select.c
--- elks.orig/fs/select.c	2012-05-11 13:26:27.000000000 -0500
+++ elks/fs/select.c	2012-07-07 10:06:09.000000000 -0500
@@ -224,7 +224,7 @@
 		      /*(fd_set *) */ &res_out,
 		      /*(fd_set *) */ &res_ex);
 
-    current->timeout = 0L;
+    current->timeout = 0UL;
     if (error < 0)
 	goto out;
     if (!error) {
diff -Nurb elks.orig/kernel/sched.c elks/kernel/sched.c
--- elks.orig/kernel/sched.c	2012-07-07 10:05:25.000000000 -0500
+++ elks/kernel/sched.c	2012-07-08 13:23:39.000000000 -0500
@@ -79,7 +79,7 @@
 {
     register __ptask prev;
     register __ptask next;
-    jiff_t timeout = 0L;
+    jiff_t timeout = 0UL;
 
     prev = current;
     next = prev->next_run;
@@ -92,8 +92,6 @@
     if (prev->state == TASK_EXITING)
         return;
 
-    run_timer_list();
-    
     clr_irq();
     switch (prev->state) {
     case TASK_INTERRUPTIBLE:
@@ -102,8 +100,8 @@
         
         timeout = prev->timeout;
     
-        if (timeout && (timeout <= jiffies)) {
-           prev->timeout = timeout = 0;
+        if (prev->timeout && (prev->timeout <= jiffies)) {
+            prev->timeout = timeout = 0UL;
 makerunnable:
             prev->state = TASK_RUNNING;
             break;
@@ -117,9 +115,11 @@
     }
     set_irq();
     
-    while(next == &init_task && nr_running > 0){
+    if(next == &init_task)
         next = next->next_run;
-    }
+
+    if (intr_count > 0)
+        goto scheduling_in_interrupt;
 
     if (next != prev) {
         struct timer_list timer;
@@ -132,9 +132,6 @@
             add_timer(&timer);
         }
 
-        if (intr_count > 0)
-            goto scheduling_in_interrupt;
-
 #ifdef CONFIG_SWAP
         if(do_swapper_run(next) == -1){
             printk("Can't become runnable %d\n", next->pid);
@@ -142,7 +139,7 @@
         }
 #endif
 
-        previous = current; 
+        previous = prev;
         current = next;
 
         tswitch();  /* Won't return for a new task */
@@ -167,7 +164,6 @@
 
 static int detach_timer(struct timer_list *timer)
 {
-    int ret = 0;
     register struct timer_list *next;
     register struct timer_list *prev;
     next = timer->tl_next;
@@ -176,10 +172,10 @@
         next->tl_prev = prev;
     }
     if (prev) {
-        ret = 1;
         prev->tl_next = next;
+	return 1;
     }
-    return ret;
+    return 0;
 }
 
 int del_timer(register struct timer_list *timer)
@@ -202,49 +198,37 @@
 void add_timer(register struct timer_list *timer)
 {
     flag_t flags;
-    register struct timer_list *next = tl_list.tl_next;
-    struct timer_list *prev = &tl_list;
+    register struct timer_list *next = &tl_list;
+    struct timer_list *prev;
 
     save_flags(flags);
     clr_irq();
 
-    while (next) {
-        if (next->tl_expires > timer->tl_expires) {
-            timer->tl_prev = next->tl_prev;
-            timer->tl_next = next;
-            timer->tl_prev->tl_next = timer;
-            next->tl_prev = timer;
-            restore_flags(flags);
-            return;
-        }
+    do {
         prev = next;
-        next = next->tl_next;
-    }
-    (timer->tl_prev = prev)->tl_next = timer;
+        if (!(next = next->tl_next))
+            goto link_tmr;
+    } while(next->tl_expires < timer->tl_expires);
 
-#if 0
-    timer->tl_next = NULL;
-#endif
+    (timer->tl_next = next)->tl_prev = timer;
 
+ link_tmr:
+    (timer->tl_prev = prev)->tl_next = timer;
     restore_flags(flags);
 }
 
 static void run_timer_list(void)
 {
-    register struct timer_list *timer = tl_list.tl_next;
+    register struct timer_list *timer;
 
     clr_irq();
-    while (timer && timer->tl_expires < jiffies) {
-        void (*fn) () = timer->tl_function;
-        int data = timer->tl_data;
+    while ((timer = tl_list.tl_next) && timer->tl_expires <= jiffies) {
         detach_timer(timer);
         timer->tl_next = timer->tl_prev = NULL;
         set_irq();
-        fn(data);
+        timer->tl_function(timer->tl_data);
         clr_irq();
-        timer = timer->tl_next;
     }
-
     set_irq();
 }
 
@@ -284,6 +268,8 @@
 	need_resched = 1;	/* how primitive can you get? */
 #endif
 
+    run_timer_list();
+
 }
 
 void sched_init(void)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.