notif_pending short circuits handle_events()
authorBarret Rhoden <brho@cs.berkeley.edu>
Tue, 8 Jan 2013 01:14:40 +0000 (17:14 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Tue, 8 Jan 2013 01:55:24 +0000 (17:55 -0800)
The kernel always sets notif_pending after posting to a VC mbox, so we
can use this check to avoid checking the full mbox (UCQs and other
stuff).

This also cleans up a couple things and removes clear_notif_pending(),
which was doing basically what the new handle_events does.  Also,
clear_notif had a bug in it, where it would never return TRUE.

Finally, note that it is only required to have one handle_events() call
when we're in VC context.  Right now we have one in uth_vc_entry and
another in the 2LS (in pth's loop).  Due to how the pth code is set up,
we need the uth one at least (in case we immediately call run_cur_uth).

We also have a few 'optimization' locations when leaving VC ctx (either
by cur_uth or by yielding/idling).

Slimming these down yielded a couple ns on the pth_test benchmark, so we
might go with that in the future.

tests/eth_audio.c
tests/msr_get_cores.c
tests/msr_get_singlecore.c
user/parlib/event.c
user/parlib/include/vcore.h
user/parlib/uthread.c
user/parlib/vcore.c

index 977edbf..41ccc40 100644 (file)
@@ -117,7 +117,7 @@ void vcore_entry(void)
         * set the appropriate TLS.  On x86, this will involve changing the LDT
         * entry for this vcore to point to the TCB of the new user-thread. */
        if (vcoreid == 0) {
-               clear_notif_pending(vcoreid);
+               handle_events(vcoreid);
                set_tls_desc(core0_tls, 0);
                assert(__vcoreid == 0); /* in case anyone uses this */
                /* Load silly state (Floating point) too */
index 62b564b..3fdb406 100644 (file)
@@ -23,6 +23,7 @@
 #include <timing.h>
 #include <rassert.h>
 #include <uthread.h>
+#include <event.h>
 
 #ifdef __sparc_v8__
 # define udelay(x) udelay((x)/2000)
@@ -97,7 +98,7 @@ void vcore_entry(void)
         * set the appropriate TLS.  On x86, this will involve changing the LDT
         * entry for this vcore to point to the TCB of the new user-thread. */
        if (vcoreid == 0) {
-               clear_notif_pending(vcoreid);
+               handle_events(vcoreid);
                set_tls_desc(core0_tls, 0);
                assert(__vcoreid == 0); /* in case anyone uses this */
                /* Load silly state (Floating point) too */
index a998c0f..5b323f9 100644 (file)
@@ -15,6 +15,7 @@
 #include <timing.h>
 #include <rassert.h>
 #include <uthread.h>
+#include <event.h>
 
 #ifdef __sparc_v8__
 # define udelay(x) udelay((x)/2000)
@@ -98,7 +99,7 @@ void vcore_entry(void)
         * set the appropriate TLS.  On x86, this will involve changing the LDT
         * entry for this vcore to point to the TCB of the new user-thread. */
        if (vcoreid == 0) {
-               clear_notif_pending(vcoreid);
+               handle_events(vcoreid);
                set_tls_desc(core0_tls, 0);
                assert(__vcoreid == 0); /* in case anyone uses this */
                /* Load silly state (Floating point) too */
index b4880c3..4259bc8 100644 (file)
@@ -186,8 +186,8 @@ int handle_one_mbox_msg(struct event_mbox *ev_mbox)
 }
 
 /* Handle an mbox.  This is the receive-side processing of an event_queue.  It
- * takes an ev_mbox, since the vcpd mbox isn't a regular ev_q.  Returns the
- * number handled/attempted: counts if you don't have a handler. */
+ * takes an ev_mbox, since the vcpd mbox isn't a regular ev_q.  Returns 1 if we
+ * handled something, 0 o/w. */
 int handle_mbox(struct event_mbox *ev_mbox)
 {
        int retval = 0;
@@ -196,15 +196,15 @@ int handle_mbox(struct event_mbox *ev_mbox)
                printd("[event] Bit: ev_type: %d\n", bit);
                if (ev_handlers[bit])
                        ev_handlers[bit](0, bit);
-               retval++;
+               retval = 1;
                /* Consider checking the queue for incoming messages while we're here */
        }
        printd("[event] handling ev_mbox %08p on vcore %d\n", ev_mbox, vcore_id());
        /* Some stack-smashing bugs cause this to fail */
        assert(ev_mbox);
-       /* Handle all full messages, tracking the number of attempts. */
+       /* Handle all full messages, tracking if we do at least one. */
        while (handle_one_mbox_msg(ev_mbox))
-               retval++;
+               retval = 1;
        /* Process all bits, if the kernel tells us any bit is set.  We don't clear
         * the flag til after we check everything, in case one of the handlers
         * doesn't return.  After we clear it, we recheck. */
@@ -267,16 +267,24 @@ void handle_check_msgs(struct event_msg *ev_msg, unsigned int ev_type)
        handle_vcpd_mbox(rem_vcoreid);
 }
 
-/* 2LS will probably call this in vcore_entry and places where it wants to check
- * for / handle events.  This will process all the events for the given vcore.
- * Note, it probably should be the calling vcore you do this to...  Returns the
- * number of events handled. */
+/* Attempts to handle events, if notif_pending.  The kernel always sets
+ * notif_pending after posting a message to either public or private mailbox.
+ * When this returns, as far as we are concerned, notif_pending is FALSE.
+ * However, a concurrent kernel writer could have reset it to true.  This is
+ * fine; whenever we leave VC ctx we double check notif_pending.  Returns 1 or 2
+ * if we actually handled a message, 0 o/w.
+ *
+ * WARNING: this might not return and/or current_uthread may change. */
 int handle_events(uint32_t vcoreid)
 {
        struct preempt_data *vcpd = vcpd_of(vcoreid);
        int retval = 0;
-       retval += handle_mbox(&vcpd->ev_mbox_private);
-       retval += handle_mbox(&vcpd->ev_mbox_public);
+       if (vcpd->notif_pending) {
+               vcpd->notif_pending = FALSE;
+               wrmb(); /* prevent future reads from happening before notif_p write */
+               retval += handle_mbox(&vcpd->ev_mbox_private);
+               retval += handle_mbox(&vcpd->ev_mbox_public);
+       }
        return retval;
 }
 
index d906de2..549bd8e 100644 (file)
@@ -55,7 +55,6 @@ void vcore_event_init(void);
 void vcore_change_to_m(void);
 int vcore_request(long nr_new_vcores);
 void vcore_yield(bool preempt_pending);
-bool clear_notif_pending(uint32_t vcoreid);
 void enable_notifs(uint32_t vcoreid);
 void disable_notifs(uint32_t vcoreid);
 void vcore_idle(void);
index db9f675..f4d759e 100644 (file)
@@ -382,7 +382,7 @@ static void __run_cur_uthread(void)
        struct preempt_data *vcpd = vcpd_of(vcoreid);
        struct uthread *uthread;
        /* Last check for messages.  Might not return, or cur_uth might be unset. */
-       clear_notif_pending(vcoreid);
+       handle_events(vcoreid);
        /* clear_notif might have handled a preemption event, and we might not have
         * a current_uthread anymore.  Need to recheck */
        cmb();
index 10b9883..78fb0ab 100644 (file)
@@ -261,16 +261,14 @@ void vcore_yield(bool preempt_pending)
        uint32_t vcoreid = vcore_id();
        struct preempt_data *vcpd = vcpd_of(vcoreid);
        __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.
-        * 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
-        * important that clear_notif_pending will handle_events().  That is
-        * necessary to do/check after turning off CAN_RCV_MSG. */
-       if (clear_notif_pending(vcoreid)) {
+       /* no wrmb() necessary, handle_events() has an mb() if it is checking */
+       /* Clears notif pending and tries to handle events.  This is an optimization
+        * to avoid the yield syscall if we have an event pending.  If there is one,
+        * 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.  */
+       if (handle_events(vcoreid)) {
                __sync_fetch_and_or(&vcpd->flags, VC_CAN_RCV_MSG);
                return;
        }
@@ -289,30 +287,6 @@ void vcore_yield(bool preempt_pending)
        __sync_fetch_and_or(&vcpd->flags, VC_CAN_RCV_MSG);
 }
 
-/* Clear pending, and try to handle events that came in between a previous call
- * to handle_events() and the clearing of pending.  While it's not a big deal,
- * we'll loop in case we catch any.  Will break out of this once there are no
- * events, and we will have send pending to 0. 
- *
- * Note that this won't catch every race/case of an incoming event.  Future
- * events will get caught in pop_ros_tf() or proc_yield().
- *
- * Also note that this handles events, which may change your current uthread or
- * might not return!  Be careful calling this.  Check run_uthread for an example
- * of how to use this. */
-bool clear_notif_pending(uint32_t vcoreid)
-{
-       bool handled_event = FALSE;
-       do {
-               vcpd_of(vcoreid)->notif_pending = 0;
-               /* need a full mb(), since handle events might be just a read or might
-                * be a write, either way, it needs to happen after notif_pending */
-               mb();
-               handled_event = handle_events(vcoreid);
-       } while (handled_event);
-       return handled_event;
-}
-
 /* Enables notifs, and deals with missed notifs by self notifying.  This should
  * be rare, so the syscall overhead isn't a big deal.  The other alternative
  * would be to uthread_yield(), which would require us to revert some uthread
@@ -338,11 +312,13 @@ void disable_notifs(uint32_t vcoreid)
 }
 
 /* Like smp_idle(), this will put the core in a state that it can only be woken
- * up by an IPI.  In the future, we may halt or something. */
-void __attribute__((noreturn)) vcore_idle(void)
+ * up by an IPI.  In the future, we may halt or something.  This will return if
+ * an event was pending (could be the one you were waiting for). */
+void vcore_idle(void)
 {
        uint32_t vcoreid = vcore_id();
-       clear_notif_pending(vcoreid);
+       if (handle_events(vcoreid))
+               return;
        enable_notifs(vcoreid);
        while (1) {
                cpu_relax();