Fixes issues with proc_yield and preempt_pending
authorBarret Rhoden <brho@cs.berkeley.edu>
Sat, 7 Apr 2012 01:18:38 +0000 (18:18 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Sat, 7 Apr 2012 01:26:18 +0000 (18:26 -0700)
preempt_pending has been neglected a little with some of the recent
kernel/user core management code.

This fix stops the kernel from aborting a yield when a process has a
preempt pending *and* some race happened in userspace where amt_wanted
wasn't decreased.

This also decreases the chance of that race by keeping userspace from
decrementing its amt_wanted when it is being nice (preempt pending).

kern/src/process.c
user/parlib/vcore.c

index 4f6dcac..8fa0045 100644 (file)
@@ -996,9 +996,17 @@ void proc_yield(struct proc *SAFE p, bool being_nice)
        /* At this point, AFAIK there should be no preempt/death messages on the
         * way, and we're on the online list.  So we'll go ahead and do the yielding
         * business. */
-       /* no need to preempt later, since we are yielding (nice or otherwise) */
-       if (vc->preempt_pending)
+       /* If there's a preempt pending, we don't need to preempt later since we are
+        * yielding (nice or otherwise).  If not, this is just a regular yield. */
+       if (vc->preempt_pending) {
                vc->preempt_pending = 0;
+       } else {
+               /* Optional: on a normal yield, check to see if we are putting them
+                * below amt_wanted (help with user races) and bail. */
+               if (p->procdata->res_req[RES_CORES].amt_wanted >=
+                                      p->procinfo->num_vcores)
+                       goto out_failed;
+       }
        /* Don't let them yield if they are missing a notification.  Userspace must
         * not leave vcore context without dealing with notif_pending.  pop_ros_tf()
         * handles leaving via uthread context.  This handles leaving via a yield.
@@ -1008,10 +1016,6 @@ void proc_yield(struct proc *SAFE p, bool being_nice)
         * posting). */
        if (vcpd->notif_pending)
                goto out_failed;
-       /* Optional: check to see if we are putting them below amt_wanted (help with
-        * correctness-benign user races) and bail */
-       if (p->procdata->res_req[RES_CORES].amt_wanted >= p->procinfo->num_vcores)
-               goto out_failed;
        /* Now we'll actually try to yield */
        printd("[K] Process %d (%p) is yielding on vcore %d\n", p->pid, p,
               get_vcoreid(p, coreid));
index 6085e80..418e055 100644 (file)
@@ -321,13 +321,15 @@ void vcore_yield(bool preempt_pending)
                vcpd->can_rcv_msg = TRUE;
                return;
        }
-       /* Tell the kernel we want one less vcore.  If yield fails (slight race), we
-        * may end up having more vcores than amt_wanted for a while, and might lose
-        * one later on (after a preempt/timeslicing) - the 2LS will have to notice
-        * eventually if it actually needs more vcores (which it already needs to
-        * do).  We need to atomically decrement, though I don't want the kernel's
-        * data type here to be atomic_t (only userspace cares in this one case). */
-       __sync_fetch_and_sub(&__procdata.res_req[RES_CORES].amt_wanted, 1);
+       /* If we are yielding since we don't want the core, tell the kernel we want
+        * one less vcore.  If yield fails (slight race), we may end up having more
+        * vcores than amt_wanted for a while, and might lose one later on (after a
+        * preempt/timeslicing) - the 2LS will have to notice eventually if it
+        * actually needs more vcores (which it already needs to do).  We need to
+        * atomically decrement, though I don't want the kernel's data type here to
+        * be atomic_t (only userspace cares in this one case). */
+       if (!preempt_pending)
+               __sync_fetch_and_sub(&__procdata.res_req[RES_CORES].amt_wanted, 1);
        /* We can probably yield.  This may pop back up if notif_pending became set
         * by the kernel after we cleared it and we lost the race. */
        sys_yield(preempt_pending);