Fixes race with SC_DONE and event overflow (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Mon, 6 Jun 2011 19:11:39 +0000 (12:11 -0700)
committerKevin Klues <klueska@cs.berkeley.edu>
Thu, 3 Nov 2011 00:36:04 +0000 (17:36 -0700)
In VMs, or possibly with poor interrupt timing, the kernel would be
delayed such that the 2LS would deregister and handle all events before
the kernel would send an ev_msg, causing a uthread to be restarted
twice.

Rebuild your cross-compiler/glibc.

Documentation/async_events.txt
kern/include/ros/syscall.h
kern/include/syscall.h
kern/src/syscall.c
user/parlib/include/uthread.h
user/parlib/uthread.c
user/pthread/pthread.c

index 7f83521..976a8a6 100644 (file)
@@ -216,6 +216,13 @@ If it doesn't fail, they are now ready to receive messages.  This can be
 tweaked a bit.
 5) Unset the overflow-handling flag.
 
+One thing to be careful of is that when we turn off event delivery, you need to
+be sure the kernel isn't in the process of sending an event.  This is why we
+have the SC_K_LOCK syscall flag.  Uthread code will not consider deregistration
+complete while that flag is set, since the kernel is still mucking with the
+syscall (and sending an event).  Once the flag is clear, the event has been
+delivered (the ev_msg is in the ev_mbox), and our assumptions remain true.
+
 There are a couple implications of this style.  If you have a shared event
 queue (with other event sources), those events can get mixed in with the
 recovery.  Don't leave the vcore context due to other events.  This'll
index 28f6ff0..c57204b 100644 (file)
 #define SC_DONE                                        0x0001          /* SC is done */
 #define SC_PROGRESS                            0x0002          /* SC made progress */
 #define SC_UEVENT                              0x0004          /* user has an ev_q */
+#define SC_K_LOCK                              0x0008          /* kernel locked sysc */
 
 struct syscall {
        unsigned int                            num;
-       long                                            retval;
        int                                                     err;                    /* errno */
+       long                                            retval;
        atomic_t                                        flags;
        struct event_queue                      *ev_q;
        void                                            *u_data;
@@ -64,6 +65,10 @@ static inline long __ros_syscall(unsigned int _num, long _a0, long _a1, long _a2
        /* Don't proceed til we are done */
        while (!(atomic_read(&sysc.flags) & SC_DONE))
                ros_syscall_blockon(&sysc);
+       /* Need to wait til it is unlocked.  It's not really done until SC_DONE &
+        * !SC_K_LOCK. */
+       while (atomic_read(&sysc.flags) & SC_K_LOCK)
+               cpu_relax();
        if (errno_loc)
                *errno_loc = sysc.err;
        return sysc.retval;
index c24b69b..e84ebbe 100644 (file)
@@ -45,7 +45,7 @@ void run_local_syscall(struct syscall *sysc);
 intreg_t syscall(struct proc *p, uintreg_t sc_num, uintreg_t a0, uintreg_t a1,
                  uintreg_t a2, uintreg_t a3, uintreg_t a4, uintreg_t a5);
 void set_errno(int errno);
-void signal_syscall(struct syscall *sysc, struct proc *p);
+void __signal_syscall(struct syscall *sysc, struct proc *p);
 
 /* Tracing functions */
 void systrace_start(bool silent);
index bfc3a6f..e00cdc6 100644 (file)
@@ -1425,10 +1425,14 @@ void run_local_syscall(struct syscall *sysc)
                               sysc->arg2, sysc->arg3, sysc->arg4, sysc->arg5);
        /* Need to re-load pcpui, in case we migrated */
        pcpui = &per_cpu_info[core_id()];
-       /* Atomically turn on the SC_DONE flag.  Need the atomics since we're racing
-        * with userspace for the event_queue registration. */
-       atomic_or(&sysc->flags, SC_DONE); 
-       signal_syscall(sysc, pcpui->cur_proc);
+       /* Atomically turn on the LOCK and SC_DONE flag.  The lock tells userspace
+        * we're messing with the flags and to not proceed.  We use it instead of
+        * CASing with userspace.  We need the atomics since we're racing with
+        * userspace for the event_queue registration.  The 'lock' tells userspace
+        * to not muck with the flags while we're signalling. */
+       atomic_or(&sysc->flags, SC_K_LOCK | SC_DONE); 
+       __signal_syscall(sysc, pcpui->cur_proc);
+       atomic_and(&sysc->flags, ~SC_K_LOCK); 
        /* Can unpin (UMEM) at this point */
        pcpui->cur_sysc = 0;    /* no longer working on sysc */
 }
@@ -1452,8 +1456,10 @@ void prep_syscalls(struct proc *p, struct syscall *sysc, unsigned int nr_syscs)
 
 /* Call this when something happens on the syscall where userspace might want to
  * get signaled.  Passing p, since the caller should know who the syscall
- * belongs to (probably is current). */
-void signal_syscall(struct syscall *sysc, struct proc *p)
+ * belongs to (probably is current). 
+ *
+ * You need to have SC_K_LOCK set when you call this. */
+void __signal_syscall(struct syscall *sysc, struct proc *p)
 {
        struct event_queue *ev_q;
        struct event_msg local_msg;
index 1356b93..f9f8766 100644 (file)
@@ -60,8 +60,7 @@ void ros_syscall_blockon(struct syscall *sysc);
 bool check_preempt_pending(uint32_t vcoreid);
 
 bool register_evq(struct syscall *sysc, struct event_queue *ev_q);
-void deregister_sysc(struct syscall *sysc);
-bool reregister_sysc(struct syscall *sysc);
+void deregister_evq(struct syscall *sysc);
 
 /* Helpers, which sched_entry() can call */
 void run_current_uthread(void);
index b109b4a..378a28c 100644 (file)
@@ -332,6 +332,9 @@ bool register_evq(struct syscall *sysc, struct event_queue *ev_q)
        do {
                cmb();
                old_flags = atomic_read(&sysc->flags);
+               /* Spin if the kernel is mucking with syscall flags */
+               while (old_flags & SC_K_LOCK)
+                       old_flags = atomic_read(&sysc->flags);
                /* If the kernel finishes while we are trying to sign up for an event,
                 * we need to bail out */
                if (old_flags & (SC_DONE | SC_PROGRESS)) {
@@ -347,40 +350,28 @@ bool register_evq(struct syscall *sysc, struct event_queue *ev_q)
  * to unset SC_UEVENT.
  *
  * There is a chance the kernel sent an event if you didn't do this in time, but
- * once it is done, the kernel won't send a message.  You can later turn it back
- * on with reregister_sysc (which doesn't need to know the ev_q. */
-void deregister_sysc(struct syscall *sysc)
+ * once this returns, the kernel won't send a message.
+ *
+ * If the kernel is trying to send a message right now, this will spin (on
+ * SC_K_LOCK).  We need to make sure we deregistered, and that if a message
+ * is coming, that it already was sent (and possibly overflowed), before
+ * returning. */
+void deregister_evq(struct syscall *sysc)
 {
        int old_flags;
+       sysc->ev_q = 0;
        /* Try and unset the SC_UEVENT flag */
        do {
                cmb();
                old_flags = atomic_read(&sysc->flags);
+               /* Spin if the kernel is mucking with syscall flags */
+               while (old_flags & SC_K_LOCK)
+                       old_flags = atomic_read(&sysc->flags);
                /* Note we don't care if the SC_DONE flag is getting set.  We just need
                 * to avoid clobbering flags */
        } while (!atomic_comp_swap(&sysc->flags, old_flags, old_flags & ~SC_UEVENT));
 }
 
-/* Turns SC handling back on.  Returns true if it succeeded, and false
- * otherwise.  False means that the syscall is done, and does not need an event
- * set (and should be handled accordingly). */
-bool reregister_sysc(struct syscall *sysc)
-{
-       int old_flags;
-       /* Try and set the SC_UEVENT flag (so the kernel knows to look at ev_q) */
-       do {
-               cmb();
-               old_flags = atomic_read(&sysc->flags);
-               /* If the kernel finishes while we are trying to sign up for an event,
-                * we need to bail out */
-               if (old_flags & (SC_DONE | SC_PROGRESS)) {
-                       sysc->ev_q = 0;         /* not necessary, but might help with bugs */
-                       return FALSE;
-               }
-       } while (!atomic_comp_swap(&sysc->flags, old_flags, old_flags | SC_UEVENT));
-       return TRUE;
-}
-
 /* TLS helpers */
 static int __uthread_allocate_tls(struct uthread *uthread)
 {
index 45d38d2..bf7d7d2 100644 (file)
@@ -291,7 +291,7 @@ static void pth_handle_syscall(struct event_msg *ev_msg, unsigned int ev_type,
                 * on the pending_sysc list. */
                TAILQ_FOREACH(i, &sysc_mgmt[vcoreid].pending_syscs, next) {
                        sysc = ((struct uthread*)i)->sysc;
-                       deregister_sysc(sysc);
+                       deregister_evq(sysc);
                }
                /* Handle event msgs, to get any syscs that sent messages.  We don't
                 * care about bits, since we're dealing with overflow already.  Note
@@ -308,7 +308,7 @@ static void pth_handle_syscall(struct event_msg *ev_msg, unsigned int ev_type,
                 * this loop. */
                TAILQ_FOREACH_SAFE(i, &sysc_mgmt[vcoreid].pending_syscs, next, temp) {
                        sysc = ((struct uthread*)i)->sysc;
-                       if (!reregister_sysc(sysc)) {
+                       if (!register_evq(sysc, &vc_sysc_mgmt->ev_q)) {
                                /* They are already done, can't sign up for events, just like
                                 * when we blocked on them the first time. */
                                restart_thread(sysc);
@@ -360,7 +360,8 @@ void pth_blockon_sysc(struct syscall *sysc)
        TAILQ_INSERT_TAIL(&sysc_mgmt[vcoreid].pending_syscs, pthread, next);
        /* Safety: later we'll make sure we restart on the core we slept on */
        pthread->vcoreid = vcoreid;
-       /* Register our vcore's syscall ev_q to hear about this syscall. */
+       /* Register our vcore's syscall ev_q to hear about this syscall.  Keep this
+        * in sync with how we handle overflowed syscalls later. */
        if (!register_evq(sysc, &sysc_mgmt[vcoreid].ev_q)) {
                /* Lost the race with the call being done.  The kernel won't send the
                 * event.  Just restart him. */