Fix the userland scheduler. When the scheduler releases the P_CURPROC
authorMatthew Dillon <dillon@dragonflybsd.org>
Thu, 16 Oct 2003 22:26:42 +0000 (22:26 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Thu, 16 Oct 2003 22:26:42 +0000 (22:26 +0000)
designation it unconditionally handed it off to the highest priority
process on the userland process queue, ignoring the fact that the 'current'
process might have had a higher priority.  There was also a missing call to
lwkt_maybe_switch() in the resched_wanted() case that could cause interrupt
threads to stall for a long period of time when they could not preempt.

In SMP there are still some issues.  Niced processes work better, but at
the moment the P_CURPROC handoff does not take into account the fact that
the new higher priority process might better be handed off to another cpu
that is running a lower priority process then the current cpu.

sys/i386/i386/trap.c
sys/kern/kern_exit.c
sys/kern/kern_switch.c
sys/kern/kern_synch.c
sys/platform/pc32/i386/trap.c
sys/sys/proc.h
sys/sys/thread.h

index 5a6811a..4ca3bf8 100644 (file)
@@ -36,7 +36,7 @@
  *
  *     from: @(#)trap.c        7.4 (Berkeley) 5/13/91
  * $FreeBSD: src/sys/i386/i386/trap.c,v 1.147.2.11 2003/02/27 19:09:59 luoqi Exp $
- * $DragonFly: src/sys/i386/i386/Attic/trap.c,v 1.34 2003/08/26 21:42:18 rob Exp $
+ * $DragonFly: src/sys/i386/i386/Attic/trap.c,v 1.35 2003/10/16 22:26:35 dillon Exp $
  */
 
 /*
@@ -183,11 +183,11 @@ passive_release(struct thread *td)
 {
        struct proc *p = td->td_proc;
 
-       td->td_release = NULL;
        lwkt_setpri_self(TDPRI_KERN_USER);
-       if (p->p_flag & P_CURPROC) {
-               release_curproc(p);
-       }
+       if (p->p_flag & P_CURPROC)
+               release_curproc(p, (td->td_flags & TDF_RUNQ) == 0);
+       if ((p->p_flag & P_CURPROC) == 0)
+               td->td_release = NULL;
 }
 
 /*
@@ -208,18 +208,18 @@ userexit(struct proc *p)
        struct thread *td = p->p_thread;
 
        /*
-        * If we did not have to release we should already be P_CURPROC.  If
-        * we did have to release we must acquire P_CURPROC again and then
-        * restore our priority for user return.
+        * Reacquire our P_CURPROC status and adjust the LWKT priority
+        * for our return to userland.  We can fast path the case where
+        * td_release was not called and P_CURPROC is still set, otherwise
+        * do it the slow way.
         *
         * Lowering our priority may make other higher priority threads
         * runnable. lwkt_setpri_self() does not switch away, so call
         * lwkt_maybe_switch() to deal with it.
         */
-       if (td->td_release) {
+       if (td->td_release && (p->p_flag & P_CURPROC)) {
                ++fast_release;
                td->td_release = NULL;
-               KKASSERT(p->p_flag & P_CURPROC);
        } else {
                ++slow_release;
                acquire_curproc(p);
@@ -255,16 +255,16 @@ userret(struct proc *p, struct trapframe *frame, u_quad_t oticks)
        /*
         * If a reschedule has been requested then the easiest solution
         * is to run our passive release function which will possibly
-        * shift our P_CURPROC designation to another user process.
-        * We don't actually switch here because that would be a waste
-        * of cycles (the newly scheduled user process would just switch
-        * back to us since we might be running at a kernel priority).
-        * Instead we fall through and will switch away when we attempt
-        * to reacquire our P_CURPROC designation.
+        * shift our P_CURPROC designation to another user process. 
+        *
+        * A reschedule can also occur due to a higher priority LWKT thread
+        * becoming runable, we have to call lwkt_maybe_switch() to deal
+        * with it.
         */
        if (resched_wanted()) {
                if (curthread->td_release)
                        passive_release(curthread);
+               lwkt_maybe_switch();
        }
 
        /*
index 02ba507..0e97c25 100644 (file)
@@ -37,7 +37,7 @@
  *
  *     @(#)kern_exit.c 8.7 (Berkeley) 2/12/94
  * $FreeBSD: src/sys/kern/kern_exit.c,v 1.92.2.11 2003/01/13 22:51:16 dillon Exp $
- * $DragonFly: src/sys/kern/kern_exit.c,v 1.23 2003/08/26 21:09:02 rob Exp $
+ * $DragonFly: src/sys/kern/kern_exit.c,v 1.24 2003/10/16 22:26:37 dillon Exp $
  */
 
 #include "opt_compat.h"
@@ -367,7 +367,7 @@ exit1(int rv)
         * Release the P_CURPROC designation on the process so the userland
         * scheduler can work in someone else.
         */
-       release_curproc(p);
+       release_curproc(p, 1);
 
        /*
         * Finally, call machine-dependent code to release the remaining
index 6ad37fc..7601009 100644 (file)
@@ -24,7 +24,7 @@
  * SUCH DAMAGE.
  *
  * $FreeBSD: src/sys/kern/kern_switch.c,v 1.3.2.1 2000/05/16 06:58:12 dillon Exp $
- * $DragonFly: src/sys/kern/Attic/kern_switch.c,v 1.9 2003/07/25 05:26:50 dillon Exp $
+ * $DragonFly: src/sys/kern/Attic/kern_switch.c,v 1.10 2003/10/16 22:26:37 dillon Exp $
  */
 
 #include <sys/param.h>
@@ -335,7 +335,7 @@ remrunqueue(struct proc *p)
  * useable.
  */
 void
-release_curproc(struct proc *p)
+release_curproc(struct proc *p, int force)
 {
        int cpuid;
        struct proc *np;
@@ -352,10 +352,16 @@ release_curproc(struct proc *p)
                if (try_mplock()) {
                        KKASSERT(curprocmask & (1 << cpuid));
                        if ((np = chooseproc()) != NULL) {
-                               np->p_flag |= P_CURPROC;
-                               USCHED_COUNTER(np->p_thread);
-                               lwkt_acquire(np->p_thread);
-                               lwkt_schedule(np->p_thread);
+                               if (force || test_resched(p, np)) {
+                                       np->p_flag |= P_CURPROC;
+                                       USCHED_COUNTER(np->p_thread);
+                                       lwkt_acquire(np->p_thread);
+                                       lwkt_schedule(np->p_thread);
+                               } else {
+                                       p->p_flag &= ~P_CP_RELEASED;
+                                       p->p_flag |= P_CURPROC;
+                                       setrunqueue(np);
+                               }
                        } else {
                                curprocmask &= ~(1 << cpuid);
                        }
@@ -461,7 +467,7 @@ uio_yield(void)
        if (p) {
                p->p_flag |= P_PASSIVE_ACQ;
                acquire_curproc(p);
-               release_curproc(p);
+               release_curproc(p, 1);
                p->p_flag &= ~P_PASSIVE_ACQ;
        }
 }
index 8216f14..cb69b95 100644 (file)
@@ -37,7 +37,7 @@
  *
  *     @(#)kern_synch.c        8.9 (Berkeley) 5/19/95
  * $FreeBSD: src/sys/kern/kern_synch.c,v 1.87.2.6 2002/10/13 07:29:53 kbyanc Exp $
- * $DragonFly: src/sys/kern/kern_synch.c,v 1.22 2003/09/25 01:47:56 dillon Exp $
+ * $DragonFly: src/sys/kern/kern_synch.c,v 1.23 2003/10/16 22:26:37 dillon Exp $
  */
 
 #include "opt_ktrace.h"
@@ -149,7 +149,9 @@ roundrobin_interval(void)
 }
 
 /*
- * Force switch among equal priority processes every 100ms.
+ * Force switch among equal priority processes every 100ms. 
+ *
+ * WARNING!  The MP lock is not held on ipi message remotes.
  */
 #ifdef SMP
 
index c91e0db..5103899 100644 (file)
@@ -36,7 +36,7 @@
  *
  *     from: @(#)trap.c        7.4 (Berkeley) 5/13/91
  * $FreeBSD: src/sys/i386/i386/trap.c,v 1.147.2.11 2003/02/27 19:09:59 luoqi Exp $
- * $DragonFly: src/sys/platform/pc32/i386/trap.c,v 1.34 2003/08/26 21:42:18 rob Exp $
+ * $DragonFly: src/sys/platform/pc32/i386/trap.c,v 1.35 2003/10/16 22:26:35 dillon Exp $
  */
 
 /*
@@ -183,11 +183,11 @@ passive_release(struct thread *td)
 {
        struct proc *p = td->td_proc;
 
-       td->td_release = NULL;
        lwkt_setpri_self(TDPRI_KERN_USER);
-       if (p->p_flag & P_CURPROC) {
-               release_curproc(p);
-       }
+       if (p->p_flag & P_CURPROC)
+               release_curproc(p, (td->td_flags & TDF_RUNQ) == 0);
+       if ((p->p_flag & P_CURPROC) == 0)
+               td->td_release = NULL;
 }
 
 /*
@@ -208,18 +208,18 @@ userexit(struct proc *p)
        struct thread *td = p->p_thread;
 
        /*
-        * If we did not have to release we should already be P_CURPROC.  If
-        * we did have to release we must acquire P_CURPROC again and then
-        * restore our priority for user return.
+        * Reacquire our P_CURPROC status and adjust the LWKT priority
+        * for our return to userland.  We can fast path the case where
+        * td_release was not called and P_CURPROC is still set, otherwise
+        * do it the slow way.
         *
         * Lowering our priority may make other higher priority threads
         * runnable. lwkt_setpri_self() does not switch away, so call
         * lwkt_maybe_switch() to deal with it.
         */
-       if (td->td_release) {
+       if (td->td_release && (p->p_flag & P_CURPROC)) {
                ++fast_release;
                td->td_release = NULL;
-               KKASSERT(p->p_flag & P_CURPROC);
        } else {
                ++slow_release;
                acquire_curproc(p);
@@ -255,16 +255,16 @@ userret(struct proc *p, struct trapframe *frame, u_quad_t oticks)
        /*
         * If a reschedule has been requested then the easiest solution
         * is to run our passive release function which will possibly
-        * shift our P_CURPROC designation to another user process.
-        * We don't actually switch here because that would be a waste
-        * of cycles (the newly scheduled user process would just switch
-        * back to us since we might be running at a kernel priority).
-        * Instead we fall through and will switch away when we attempt
-        * to reacquire our P_CURPROC designation.
+        * shift our P_CURPROC designation to another user process. 
+        *
+        * A reschedule can also occur due to a higher priority LWKT thread
+        * becoming runable, we have to call lwkt_maybe_switch() to deal
+        * with it.
         */
        if (resched_wanted()) {
                if (curthread->td_release)
                        passive_release(curthread);
+               lwkt_maybe_switch();
        }
 
        /*
index 73ee5c8..b50cc90 100644 (file)
@@ -37,7 +37,7 @@
  *
  *     @(#)proc.h      8.15 (Berkeley) 5/19/95
  * $FreeBSD: src/sys/sys/proc.h,v 1.99.2.9 2003/06/06 20:21:32 tegge Exp $
- * $DragonFly: src/sys/sys/proc.h,v 1.31 2003/09/01 01:14:55 hmp Exp $
+ * $DragonFly: src/sys/sys/proc.h,v 1.32 2003/10/16 22:26:42 dillon Exp $
  */
 
 #ifndef _SYS_PROC_H_
@@ -419,7 +419,7 @@ int suser (struct thread *td);
 int    suser_proc (struct proc *p);
 int    suser_cred (struct ucred *cred, int flag);
 void   remrunqueue (struct proc *);
-void   release_curproc (struct proc *curp);
+void   release_curproc (struct proc *curp, int force);
 void   acquire_curproc (struct proc *curp);
 void   cpu_heavy_switch (struct thread *);
 void   cpu_lwkt_switch (struct thread *);
index 9fb1003..79a714b 100644 (file)
@@ -4,7 +4,7 @@
  *     Implements the architecture independant portion of the LWKT 
  *     subsystem.
  * 
- * $DragonFly: src/sys/sys/thread.h,v 1.34 2003/10/15 23:27:05 dillon Exp $
+ * $DragonFly: src/sys/sys/thread.h,v 1.35 2003/10/16 22:26:42 dillon Exp $
  */
 
 #ifndef _SYS_THREAD_H_
@@ -199,6 +199,7 @@ struct thread {
 #define TDPRI_USER_IDLE                4       /* user scheduler idle */
 #define TDPRI_USER_NORM                6       /* user scheduler normal */
 #define TDPRI_USER_REAL                8       /* user scheduler real time */
+#define TDPRI_KERN_LPSCHED     9       /* scheduler helper for userland sch */
 #define TDPRI_KERN_USER                10      /* kernel / block in syscall */
 #define TDPRI_KERN_DAEMON      12      /* kernel daemon (pageout, etc) */
 #define TDPRI_SOFT_NORM                14      /* kernel / normal */