Attempts to fix uth_disable_notif()
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 19 Nov 2014 00:43:43 +0000 (16:43 -0800)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 19 Nov 2014 00:56:32 +0000 (16:56 -0800)
There were two problems with disabling notifs.  First, using the uthread's TLS
is a bad idea.  The uthread might not have it, and then it's using the vcore's
TLS.  That alone might be okay, since the uthread pins itself to the vcore
(we'd have to do the check after disabling migrations and other shit).

The bigger issue is with locks grabbed in uth context and unlocked in vcore
context.  When we reenable, we would not reenable notifs, nor even adjust the
depth counter.  Vcore context just blows past those checks.  Likewise, uthread
code will never unlock it, since the unlock calls typically happen in the
uthread_yield() callbacks.  We could try and have the vcore code figure out who
was locking and adjust the depth, but that's just a pain in the ass (and
doesn't work well with locks help by both vcore context and uth code (e.g. the
pthread runqueue lock)).

This commit changes three things: uses the uthread struct for the depth, has
vcore code reset the depth during yield, and does some cleanup/debugging.

The rule for uthread code that grabs locks (or otherwise disables notifs) is
that vcore context will reset the disabled depth to 0, and uthread code should
not try to reenable notifs past the yield point.  Vcore code will enable notifs
when it relaunches the uthread.  There might be issues with this, where some
uthread disables notifs, makes some call that yields (or blocks on a syscall!)
and then reenables notifs.  Note that PDR locks count as enable/disable sites.

For cleanup, I don't see why there would not be a current_uthread, and put in
an assert to catch things (for now).

user/parlib/include/uthread.h
user/parlib/uthread.c

index a205c69..9d6b94c 100644 (file)
@@ -26,6 +26,7 @@ struct uthread {
        void *tls_desc;
        int flags;
        int state;
+       int notif_disabled_depth;
        struct syscall *sysc;   /* syscall we're blocking on, if any */
        struct syscall local_sysc;      /* for when we don't want to use the stack */
        void (*yield_func)(struct uthread*, void*);
index 55236e9..fd9d8a3 100644 (file)
@@ -12,7 +12,6 @@ struct schedule_ops default_2ls_ops = {0};
 struct schedule_ops *sched_ops __attribute__((weak)) = &default_2ls_ops;
 
 __thread struct uthread *current_uthread = 0;
-__thread bool __uth_disable_depth = 0;
 /* ev_q for all preempt messages (handled here to keep 2LSs from worrying
  * extensively about the details.  Will call out when necessary. */
 struct event_queue *preempt_ev_q;
@@ -56,6 +55,7 @@ static void uthread_manage_thread0(struct uthread *uthread)
        uthread->flags &= ~(UTHREAD_SAVED | UTHREAD_FPSAVED);
        /* need to track thread0 for TLS deallocation */
        uthread->flags |= UTHREAD_IS_THREAD0;
+       uthread->notif_disabled_depth = 0;
        /* Change temporarily to vcore0s tls region so we can save the newly created
         * tcb into its current_uthread variable and then restore it.  One minor
         * issue is that vcore0's transition-TLS isn't TLS_INITed yet.  Until it is
@@ -227,6 +227,7 @@ void uthread_init(struct uthread *new_thread, struct uth_thread_attr *attr)
         * There is no FP context to be restored yet.  We only save the FPU when we
         * were interrupted off a core. */
        new_thread->flags |= UTHREAD_SAVED;
+       new_thread->notif_disabled_depth = 0;
        if (attr && attr->want_tls) {
                /* Get a TLS.  If we already have one, reallocate/refresh it */
                if (new_thread->tls_desc)
@@ -280,6 +281,10 @@ __uthread_yield(void)
         * uthread_destroy() */
        uthread->flags &= ~UTHREAD_DONT_MIGRATE;
        uthread->state = UT_NOT_RUNNING;
+       /* Any locks that were held before the yield must be unlocked in the
+        * callback.  That callback won't get a chance to update our disabled depth.
+        * This sets us up for the next time the uthread runs. */
+       uthread->notif_disabled_depth = 0;
        /* Do whatever the yielder wanted us to do */
        assert(uthread->yield_func);
        uthread->yield_func(uthread, uthread->yield_arg);
@@ -429,8 +434,12 @@ void __ros_mcp_syscall_blockon(struct syscall *sysc)
        assert(current_uthread);
        if (current_uthread->flags & UTHREAD_DONT_MIGRATE) {
                assert(!notif_is_enabled(vcore_id()));  /* catch bugs */
+               /* if we had a notif_disabled_depth, then we should also have
+                * DONT_MIGRATE set */
                __ros_syscall_spinon(sysc);
+               return;
        }
+       assert(!current_uthread->notif_disabled_depth);
        /* double check before doing all this crap */
        if (atomic_read(&sysc->flags) & (SC_DONE | SC_PROGRESS))
                return;
@@ -706,23 +715,26 @@ bool __check_preempt_pending(uint32_t vcoreid)
 void uth_disable_notifs(void)
 {
        if (!in_vcore_context() && in_multi_mode()) {
-               if (__uth_disable_depth++)
-                       return;
-               if (current_uthread)
-                       current_uthread->flags |= UTHREAD_DONT_MIGRATE;
+               assert(current_uthread);
+               if (current_uthread->notif_disabled_depth++)
+                       goto out;
+               current_uthread->flags |= UTHREAD_DONT_MIGRATE;
                cmb();  /* don't issue the flag write before the vcore_id() read */
                disable_notifs(vcore_id());
        }
+out:
+       if (in_multi_mode())
+               assert(!notif_is_enabled(vcore_id()));
 }
 
 /* Helper: Pair this with uth_disable_notifs(). */
 void uth_enable_notifs(void)
 {
        if (!in_vcore_context() && in_multi_mode()) {
-               if (--__uth_disable_depth)
+               assert(current_uthread);
+               if (--current_uthread->notif_disabled_depth)
                        return;
-               if (current_uthread)
-                       current_uthread->flags &= ~UTHREAD_DONT_MIGRATE;
+               current_uthread->flags &= ~UTHREAD_DONT_MIGRATE;
                cmb();  /* don't enable before ~DONT_MIGRATE */
                enable_notifs(vcore_id());
        }