can_rcv_msg is now a VC flag (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 5 Oct 2012 00:09:31 +0000 (17:09 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 5 Oct 2012 00:20:12 +0000 (17:20 -0700)
Solves a race with can_rcv_msg between a preemption handler and the
kernel when that vcore starts back up.  Though the kernel would
eventually have found a message recipient.  Anyway, it's better now.

Rebuild userspace libraries / apps (anything touching vcpd).

Documentation/async_events.txt
Documentation/memory_barriers.txt
kern/include/ros/event.h
kern/src/event.c
kern/src/process.c
user/parlib/uthread.c
user/parlib/vcore.c

index e7fde52..2ec3132 100644 (file)
@@ -416,7 +416,7 @@ not work if the vcore yielded and then the entire process was preempted or
 otherwise not running.  Another way to put this is that we need a field to
 determine whether a vcore is offline temporarily or permanently.
 
-This is why we have the VCPD field 'can_rcv_msg'.  It tells the kernel's event
+This is why we have the VCPD flag 'VC_CAN_RCV_MSG'.  It tells the kernel's event
 delivery code that the vcore will check the messages: it is an acceptable
 destination for a FALLBACK.  There are two reasons to put this in VCPD:
 1) Userspace can remotely turn off a vcore's msg reception.  This is necessary
@@ -428,7 +428,7 @@ about turning off that flag - userspace can do it when it wants to yield.  (turn
 off the flag, check messages, then yield).  This is less big of a deal now that
 the kernel races with vcore membership in the online_vcs list.
 
-Two aspects of the code make this work nicely.  The 'can_rcv_msg' flag greatly
+Two aspects of the code make this work nicely.  The VC_CAN_RCV_MSG flag greatly
 simplifies the kernel's job.  There are a lot of weird races we'd have to deal
 with, such as process state (RUNNING_M), whether a mass preempt is going on, or
 just one core, or a bunch of cores, mass yields, etc.  A flag that does one
@@ -437,11 +437,11 @@ other useful thing is being able to handle spurious events.  Vcore code can
 handle extra IPIs and INDIRs to non-VCPD ev_qs.  Any vcore can handle an ev_q
 that is "non-VCPD business".
 
-Worth mentioning is the difference between 'notif_pending' and 'can_rcv_msg'.
-'can_rcv_msg' is the process saying it will check for messages.  'notif_pending'
-is when the kernel says it *has* sent a message.  'notif_pending' is also used
-by the kernel in proc_yield() and the 2LS in pop_ros_tf() to make sure the sent
-message is not missed.
+Worth mentioning is the difference between 'notif_pending' and VC_CAN_RCV_MSG.
+VC_CAN_RCV_MSG is the process saying it will check for messages.
+'notif_pending' is when the kernel says it *has* sent a message.
+'notif_pending' is also used by the kernel in proc_yield() and the 2LS in
+pop_ros_tf() to make sure the sent message is not missed.
 
 Also, in case this comes up, there's a slight race on changing the mbox* and the
 vcore number within the event_q.  The message could have gone to the wrong (old)
@@ -496,7 +496,7 @@ are good for process management, but also for helping alert_vcore() find
 potentially alertable vcores.  alert_vcore() and its associated helpers are
 failry complicated and heavily commented.  I've set things up so both the
 online_vcs and the bulk_preempted_vcs lists can be handled the same way: post to
-the first element, then see if it still 'can_rcv_msg'.  If not, if it is still
+the first element, then see if it still VC_CAN_RCV_MSG.  If not, if it is still
 the first on the list, then it hasn't proc_yield()ed yet, and it will eventually
 restart when it tries to yield.  And this all works without locking the
 proc_lock.  There are a bunch more details and races avoided.  Check the code
index 29b186b..125d3e6 100644 (file)
@@ -101,8 +101,8 @@ of the 5 memory barriers.
 When writing code that synchronizes with other threads via shared memory, we
 have a variety of patterns.  Most infamous is the "signal, then check if the
 receiver is still listening", which is the critical part of the "check,
-signal, check again" pattern.  For exmaples, look at things like
-'notif_pending' and 'can_rcv_msg'.  
+signal, check again" pattern.  For examples, look at things like
+'notif_pending' and when we check VC_CAN_RCV_MSG in event.c.
 
 In these examples, "write" and "read" include things such as posting events or
 checking flags (which ultimately involve writes and reads).  You need to be
index e038267..ecec750 100644 (file)
@@ -100,7 +100,7 @@ struct event_queue_big {
 /* Vcore state flags.  K_LOCK means the kernel is writing */
 #define VC_K_LOCK                              0x001                           /* CASing with the kernel */
 #define VC_PREEMPTED                   0x002                           /* VC is preempted */
-#define VC_RECOVERING                  0x004                           /* VC being recovered */
+#define VC_CAN_RCV_MSG                 0x004                           /* can receive FALLBACK */
 #define VC_UTHREAD_STEALING            0x008                           /* Uthread being stolen */
 #define VC_SCP_NOVCCTX                 0x010                           /* can't go into vc ctx */
 
@@ -113,7 +113,6 @@ struct preempt_data {
        atomic_t                                        flags;
        bool                                            notif_disabled;         /* vcore unwilling to recv*/
        bool                                            notif_pending;          /* notif k_msg on the way */
-       bool                                            can_rcv_msg;            /* can receive FALLBACK */
        struct event_mbox                       ev_mbox_public;         /* can be read remotely */
        struct event_mbox                       ev_mbox_private;        /* for this vcore only */
 };
index 5e1ab3c..fea5d8d 100644 (file)
@@ -52,7 +52,7 @@ static struct event_mbox *get_vcpd_mbox(uint32_t vcoreid, int ev_flags)
 static bool can_msg_vcore(uint32_t vcoreid)
 {
        struct preempt_data *vcpd = &__procdata.vcore_preempt_data[vcoreid];
-       return vcpd->can_rcv_msg;
+       return atomic_read(&vcpd->flags) & VC_CAN_RCV_MSG;
 }
 
 /* Says a vcore can be messaged.  Only call this once you are sure this is true
@@ -60,7 +60,7 @@ static bool can_msg_vcore(uint32_t vcoreid)
 static void set_vcore_msgable(uint32_t vcoreid)
 {
        struct preempt_data *vcpd = &__procdata.vcore_preempt_data[vcoreid];
-       vcpd->can_rcv_msg = TRUE;
+       atomic_or(&vcpd->flags, VC_CAN_RCV_MSG);
 }
 
 /* Posts a message to the mbox, subject to flags.  Feel free to send 0 for the
@@ -118,7 +118,7 @@ static void spam_vcore(struct proc *p, uint32_t vcoreid,
        try_notify(p, vcoreid, ev_flags);
 }
 
-/* Attempts to message a vcore that may or may not have 'can_rcv_msg' set.  If
+/* Attempts to message a vcore that may or may not have VC_CAN_RCV_MSG set.  If
  * so, we'll post the message and the message will eventually get dealt with
  * (when the vcore runs or when it is preempte-recovered). */
 static bool try_spam_vcore(struct proc *p, uint32_t vcoreid,
@@ -170,7 +170,7 @@ static bool spam_list_member(struct vcore_tailq *list, struct proc *p,
                 * will either be turned on later, or have a preempt message sent about
                 * their demise.
                 *
-                * We race on list membership (and not exclusively 'can_rcv_msg', so
+                * We race on list membership (and not exclusively VC_CAN_RCV_MSG, so
                 * that when it fails we can get a new vcore to try (or know WHP there
                 * are none). */
                vc_first = TAILQ_FIRST(list);
@@ -197,7 +197,7 @@ static bool spam_list_member(struct vcore_tailq *list, struct proc *p,
  * this must be able to handle spurious reads, since more than one vcore is
  * likely to get the message and handle it.
  *
- * We try the desired vcore, using 'can_rcv_msg'.  Failing that, we'll search
+ * We try the desired vcore, using VC_CAN_RCV_MSG.  Failing that, we'll search
  * the online and then the bulk_preempted lists.  These lists serve as a way to
  * find likely messageable vcores.  spam_list_member() helps us with them,
  * failing if anything seems to go wrong.  At which point we just lock and try
index 0f943a0..317c916 100644 (file)
@@ -1676,18 +1676,16 @@ static void __set_curtf_to_vcoreid(struct proc *p, uint32_t vcoreid)
        struct per_cpu_info *pcpui = &per_cpu_info[core_id()];
        struct preempt_data *vcpd = &p->procdata->vcore_preempt_data[vcoreid];
 
-       /* We could let userspace do this, though they come into vcore entry many
-        * times, and we just need this to happen when the cores comes online the
-        * first time.  That, and they want this turned on as soon as we know a
-        * vcore *WILL* be online.  We could also do this earlier, when we map the
-        * vcore to its pcore, though we don't always have current loaded or
-        * otherwise mess with the VCPD in those code paths. */
-       vcpd->can_rcv_msg = TRUE;
        /* Mark that this vcore as no longer preempted.  No danger of clobbering
         * other writes, since this would get turned on in __preempt (which can't be
         * concurrent with this function on this core), and the atomic is just
         * toggling the one bit (a concurrent VC_K_LOCK will work) */
        atomic_and(&vcpd->flags, ~VC_PREEMPTED);
+       /* Once the VC is no longer preempted, we allow it to receive msgs.  We
+        * could let userspace do it, but handling it here makes it easier for them
+        * to handle_indirs (when they turn this flag off).  Note the atomics
+        * provide the needed barriers (cmb and mb on flags). */
+       atomic_or(&vcpd->flags, VC_CAN_RCV_MSG);
        printd("[kernel] startcore on physical core %d for process %d's vcore %d\n",
               core_id(), p->pid, vcoreid);
        /* If notifs are disabled, the vcore was in vcore context and we need to
index 85be6e7..719f65c 100644 (file)
@@ -597,6 +597,29 @@ static void stop_uth_stealing(struct preempt_data *vcpd)
                             old_flags & ~VC_UTHREAD_STEALING));
 }
 
+/* Handles INDIRS for another core (the public mbox).  We synchronize with the
+ * kernel (__set_curtf_to_vcoreid). */
+static void handle_indirs(uint32_t rem_vcoreid)
+{
+       long old_flags;
+       struct preempt_data *rem_vcpd = vcpd_of(rem_vcoreid);
+       /* Turn off their message reception if they are still preempted.  If they
+        * are no longer preempted, we do nothing - they will handle their own
+        * messages.  Turning on CAN_RCV will route this vcore's messages to
+        * fallback vcores (if applicable). */
+       do {
+               old_flags = atomic_read(&rem_vcpd->flags);
+               while (old_flags & VC_K_LOCK)
+                       old_flags = atomic_read(&rem_vcpd->flags);
+               if (!(old_flags & VC_PREEMPTED))
+                       return;
+       } while (!atomic_cas(&rem_vcpd->flags, old_flags,
+                            old_flags & ~VC_CAN_RCV_MSG));
+       wrmb(); /* don't let the CAN_RCV write pass reads of the mbox status */
+       /* handle all INDIRs of the remote vcore */
+       handle_vcpd_mbox(rem_vcoreid);
+}
+
 /* Helper.  Will ensure a good attempt at changing vcores, meaning we try again
  * if we failed for some reason other than the vcore was already running. */
 static void __change_vcore(uint32_t rem_vcoreid, bool enable_my_notif)
@@ -700,7 +723,6 @@ static void handle_vc_preempt(struct event_msg *ev_msg, unsigned int ev_type)
        if (!(atomic_read(&rem_vcpd->flags) & VC_PREEMPTED))
                return;
        /* At this point, we need to try to recover */
-       /* TODO: if we want to bother with VC_RECOVERING, set it here */
        /* This case handles when the remote core was in vcore context */
        if (rem_vcpd->notif_disabled) {
                printd("VC %d recovering %d, notifs were disabled\n", vcoreid, rem_vcoreid);
@@ -725,7 +747,16 @@ static void handle_vc_preempt(struct event_msg *ev_msg, unsigned int ev_type)
        if (!(atomic_read(&rem_vcpd->flags) & VC_PREEMPTED))
                goto out_stealing;
        /* Might be preempted twice quickly, and the second time had notifs
-        * disabled. */
+        * disabled.
+        *
+        * Also note that the second preemption event had another
+        * message sent, which either we or someone else will deal with.  And also,
+        * we don't need to worry about how we are stealing still and plan to
+        * abort.  If another vcore handles that second preemption message, either
+        * the original vcore is in vc ctx or not.  If so, we bail out and the
+        * second preemption handling needs to change_to.  If not, we aren't
+        * bailing out, and we'll handle the preemption as normal, and the second
+        * handler will bail when it fails to steal. */
        if (rem_vcpd->notif_disabled)
                goto out_stealing;
        /* At this point, we're clear to try and steal the uthread.  Need to switch
@@ -762,15 +793,8 @@ static void handle_vc_preempt(struct event_msg *ev_msg, unsigned int ev_type)
        wmb();
        /* Fallthrough */
 out_stealing:
-       /* Turn off the UTHREAD_STEALING */
        stop_uth_stealing(rem_vcpd);
-out_indirs:
-       /* Last thing: handle their INDIRs */
-       /* First, start routing this vcore's messages to fallback vcores */
-       rem_vcpd->can_rcv_msg = FALSE;
-       wrmb(); /* don't let the can_rcv write pass reads of the mbox status */
-       /* handle all INDIRs of the remote vcore */
-       handle_vcpd_mbox(rem_vcoreid);
+       handle_indirs(rem_vcoreid);
 }
 
 /* Attempts to register ev_q with sysc, so long as sysc is not done/progress.
index 8c34c2c..2349d92 100644 (file)
@@ -264,23 +264,26 @@ try_handle_it:
        return 0;
 }
 
-/* This can return, if you failed to yield due to a concurrent event. */
+/* This can return, if you failed to yield due to a concurrent event.  Note
+ * we're atomicly setting the CAN_RCV flag, and aren't bothering with CASing
+ * (either with the kernel or uthread's handle_indirs()).  We don't particularly
+ * care what other code does - we intend to set those flags no matter what. */
 void vcore_yield(bool preempt_pending)
 {
        uint32_t vcoreid = vcore_id();
        struct preempt_data *vcpd = vcpd_of(vcoreid);
-       vcpd->can_rcv_msg = FALSE;
+       __sync_fetch_and_and(&vcpd->flags, ~VC_CAN_RCV_MSG);
        /* no wrmb() necessary, clear_notif() has an mb() */
        /* Clears notif pending.  If we had an event outstanding, this will handle
         * it and return TRUE, at which point we want to unwind and return to the
         * 2LS loop (where we may not want to yield anymore).  Note that the kernel
-        * only cares about can_rcv_msg for the desired vcore, not for a FALLBACK.
+        * only cares about CAN_RCV_MSG for the desired vcore, not for a FALLBACK.
         * We need to deal with this notif_pending business regardless of
-        * can_rcv_msg.  We just want to avoid a yield syscall if possible.  It is
+        * CAN_RCV_MSG.  We just want to avoid a yield syscall if possible.  It is
         * important that clear_notif_pending will handle_events().  That is
-        * necessary to do/check after setting can_rcv_msg to FALSE. */
+        * necessary to do/check after turning off CAN_RCV_MSG. */
        if (clear_notif_pending(vcoreid)) {
-               vcpd->can_rcv_msg = TRUE;
+               __sync_fetch_and_or(&vcpd->flags, VC_CAN_RCV_MSG);
                return;
        }
        /* If we are yielding since we don't want the core, tell the kernel we want
@@ -295,7 +298,7 @@ void vcore_yield(bool preempt_pending)
        /* 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);
-       vcpd->can_rcv_msg = TRUE;
+       __sync_fetch_and_or(&vcpd->flags, VC_CAN_RCV_MSG);
 }
 
 /* Clear pending, and try to handle events that came in between a previous call