parlib: Tease out uth_sync_t from has_blocked()
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 28 Apr 2017 16:05:29 +0000 (12:05 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 3 May 2017 16:16:41 +0000 (12:16 -0400)
Some callers of uthread_has_blocked() might want to separate the blocking
on the sync from the notification to the 2LS - even though both of those
are 2LS ops.

The specific case is pthread_barriers (in an upcoming patch).  There, we
have an optimization where we don't hold the spinlock across
uthread_yield(), and instead reacquire it and double-check if we should
block.  In that case, we'd rather call the 2LS has_blocked *before*
grabbing the barrier's spinlock (> 25% performance improvement).  Other
sync primitives might want to do the same.

Overall, this is probably a better fit - it removes those has_blocked cases
that take NULL and provides a little more flexibility to the sync primitive
code.  It also gets rid of the slightly ugly __uth_default_sync_enqueue()
and provides a nicer sync object interface.  The downside is that the 2LS
gets its has_blocked info independent of the source of the blocking.
Probably doesn't matter.

Similar to the commit 'Make sync objects static', I considered rebasing
this change, but I didn't want to break git bisect or otherwise introduce
hard-to-debug problems, which I've already had a few of due to the
toolchain changes.

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
user/parlib/event.c
user/parlib/include/parlib/uthread.h
user/parlib/mutex.c
user/parlib/thread0_sched.c
user/parlib/uthread.c
user/pthread/futex.c
user/pthread/pthread.c
user/vmm/sched.c

index cbd6964..bcc8e49 100644 (file)
@@ -760,7 +760,7 @@ static void __uth_blockon_evq_cb(struct uthread *uth, void *arg)
 {
        struct uth_sleep_ctlr *uctlr = arg;
 
-       uthread_has_blocked(uth, NULL, UTH_EXT_BLK_EVENTQ);
+       uthread_has_blocked(uth, UTH_EXT_BLK_EVENTQ);
        cmb();  /* actually block before saying 'blocked' */
        uctlr->blocked = TRUE;  /* can be woken up now */
        wrmb(); /* write 'blocked' before read 'check_evqs' */
index 171f426..8ce3e20 100644 (file)
@@ -92,10 +92,9 @@ typedef struct __uth_sync_opaque {
  * these (where applicable). */
 void __uth_sync_init(uth_sync_t *sync);
 void __uth_sync_destroy(uth_sync_t *sync);
+void __uth_sync_enqueue(struct uthread *uth, uth_sync_t *sync);
 struct uthread *__uth_sync_get_next(uth_sync_t *sync);
 bool __uth_sync_get_uth(uth_sync_t *sync, struct uthread *uth);
-/* 2LSs that use default sync objs will call this in their has_blocked op. */
-void __uth_default_sync_enqueue(struct uthread *uth, uth_sync_t *sync);
 
 /* 2L-Scheduler operations.  Examples in pthread.c. */
 struct schedule_ops {
@@ -105,13 +104,14 @@ struct schedule_ops {
        void (*thread_runnable)(struct uthread *);
        void (*thread_paused)(struct uthread *);
        void (*thread_blockon_sysc)(struct uthread *, void *);
-       void (*thread_has_blocked)(struct uthread *, uth_sync_t *, int);
+       void (*thread_has_blocked)(struct uthread *, int);
        void (*thread_refl_fault)(struct uthread *, struct user_context *);
        void (*thread_exited)(struct uthread *);
        struct uthread *(*thread_create)(void *(*)(void *), void *);
        /**** Defining these functions is optional. ****/
        void (*sync_init)(uth_sync_t *);
        void (*sync_destroy)(uth_sync_t *);
+       void (*sync_enqueue)(struct uthread *, uth_sync_t *);
        struct uthread *(*sync_get_next)(uth_sync_t *);
        bool (*sync_get_uth)(uth_sync_t *, struct uthread *);
        void (*preempt_pending)(void);
@@ -165,7 +165,7 @@ void uthread_yield(bool save_state, void (*yield_func)(struct uthread*, void*),
 void uthread_sleep(unsigned int seconds);
 void uthread_usleep(unsigned int usecs);
 void __attribute__((noreturn)) uthread_sleep_forever(void);
-void uthread_has_blocked(struct uthread *uthread, uth_sync_t *sync, int flags);
+void uthread_has_blocked(struct uthread *uthread, int flags);
 void uthread_paused(struct uthread *uthread);
 
 /* Utility functions */
index d2a9c8e..a9a3bb6 100644 (file)
@@ -108,7 +108,8 @@ static void __semaphore_cb(struct uthread *uth, void *arg)
         *
         * Also note the lock-ordering rule.  The sem lock is grabbed before any
         * locks the 2LS might grab. */
-       uthread_has_blocked(uth, &sem->sync_obj, UTH_EXT_BLK_MUTEX);
+       uthread_has_blocked(uth, UTH_EXT_BLK_MUTEX);
+       __uth_sync_enqueue(uth, &sem->sync_obj);
        spin_pdr_unlock(&sem->lock);
 }
 
@@ -396,7 +397,8 @@ static void __cv_wait_cb(struct uthread *uth, void *arg)
         *
         * Also note the lock-ordering rule.  The cv lock is grabbed before any
         * locks the 2LS might grab. */
-       uthread_has_blocked(uth, &cv->sync_obj, UTH_EXT_BLK_MUTEX);
+       uthread_has_blocked(uth, UTH_EXT_BLK_MUTEX);
+       __uth_sync_enqueue(uth, &cv->sync_obj);
        spin_pdr_unlock(&cv->lock);
        /* This looks dangerous, since both the CV and MTX could use the
         * uth->sync_next TAILQ_ENTRY (or whatever the 2LS uses), but the uthread
@@ -608,7 +610,8 @@ static void __rwlock_rd_cb(struct uthread *uth, void *arg)
 {
        struct uth_rwlock *rwl = (struct uth_rwlock*)arg;
 
-       uthread_has_blocked(uth, &rwl->readers, UTH_EXT_BLK_MUTEX);
+       uthread_has_blocked(uth, UTH_EXT_BLK_MUTEX);
+       __uth_sync_enqueue(uth, &rwl->readers);
        spin_pdr_unlock(&rwl->lock);
 }
 
@@ -645,7 +648,8 @@ static void __rwlock_wr_cb(struct uthread *uth, void *arg)
 {
        struct uth_rwlock *rwl = (struct uth_rwlock*)arg;
 
-       uthread_has_blocked(uth, &rwl->writers, UTH_EXT_BLK_MUTEX);
+       uthread_has_blocked(uth, UTH_EXT_BLK_MUTEX);
+       __uth_sync_enqueue(uth, &rwl->writers);
        spin_pdr_unlock(&rwl->lock);
 }
 
@@ -748,6 +752,13 @@ static void uth_default_sync_destroy(uth_sync_t *sync)
        assert(TAILQ_EMPTY(tq));
 }
 
+static void uth_default_sync_enqueue(struct uthread *uth, uth_sync_t *sync)
+{
+       struct uth_tailq *tq = (struct uth_tailq*)sync;
+
+       TAILQ_INSERT_TAIL(tq, uth, sync_next);
+}
+
 static struct uthread *uth_default_sync_get_next(uth_sync_t *sync)
 {
        struct uth_tailq *tq = (struct uth_tailq*)sync;
@@ -775,14 +786,6 @@ static bool uth_default_sync_get_uth(uth_sync_t *sync, struct uthread *uth)
 
 /************** External uthread sync interface **************/
 
-/* Called by the 2LS->has_blocked op, if they are using the default sync.*/
-void __uth_default_sync_enqueue(struct uthread *uth, uth_sync_t *sync)
-{
-       struct uth_tailq *tq = (struct uth_tailq*)sync;
-
-       TAILQ_INSERT_TAIL(tq, uth, sync_next);
-}
-
 /* Called by 2LS-independent sync code when a sync object needs initialized. */
 void __uth_sync_init(uth_sync_t *sync)
 {
@@ -803,6 +806,16 @@ void __uth_sync_destroy(uth_sync_t *sync)
        uth_default_sync_destroy(sync);
 }
 
+/* Called by 2LS-independent sync code when a thread blocks on sync */
+void __uth_sync_enqueue(struct uthread *uth, uth_sync_t *sync)
+{
+       if (sched_ops->sync_enqueue) {
+               sched_ops->sync_enqueue(uth, sync);
+               return;
+       }
+       uth_default_sync_enqueue(uth, sync);
+}
+
 /* Called by 2LS-independent sync code when a thread needs to be woken. */
 struct uthread *__uth_sync_get_next(uth_sync_t *sync)
 {
index 27b8208..5c7e97d 100644 (file)
@@ -23,12 +23,12 @@ static void thread0_thread_blockon_sysc(struct uthread *uthread, void *sysc);
 static void thread0_thread_refl_fault(struct uthread *uth,
                                       struct user_context *ctx);
 static void thread0_thread_runnable(struct uthread *uth);
-static void thread0_thread_has_blocked(struct uthread *uth, uth_sync_t *sync,
-                                       int flags);
+static void thread0_thread_has_blocked(struct uthread *uth, int flags);
 static void thread0_thread_exited(struct uthread *uth);
 static struct uthread *thread0_thread_create(void *(*func)(void *), void *arg);
 static void thread0_sync_init(uth_sync_t *s);
 static void thread0_sync_destroy(uth_sync_t *s);
+static void thread0_sync_enqueue(struct uthread *uth, uth_sync_t *s);
 static struct uthread *thread0_sync_get_next(uth_sync_t *s);
 static bool thread0_sync_get_uth(uth_sync_t *s, struct uthread *uth);
 
@@ -45,6 +45,7 @@ struct schedule_ops thread0_2ls_ops = {
        .thread_create = thread0_thread_create,
        .sync_init = thread0_sync_init,
        .sync_destroy = thread0_sync_destroy,
+       .sync_enqueue = thread0_sync_enqueue,
        .sync_get_next = thread0_sync_get_next,
        .sync_get_uth = thread0_sync_get_uth,
 };
@@ -107,7 +108,7 @@ static void thread0_sched_entry(void)
 static void thread0_thread_blockon_sysc(struct uthread *uthread, void *arg)
 {
        struct syscall *sysc = (struct syscall*)arg;
-       thread0_thread_has_blocked(uthread, NULL, 0);
+       thread0_thread_has_blocked(uthread, 0);
        if (!register_evq(sysc, sysc_evq))
                thread0_thread_runnable(uthread);
 }
@@ -157,8 +158,7 @@ static void thread0_thread_runnable(struct uthread *uth)
        thread0_info.is_blocked = FALSE;
 }
 
-static void thread0_thread_has_blocked(struct uthread *uth, uth_sync_t *sync,
-                                       int flags)
+static void thread0_thread_has_blocked(struct uthread *uth, int flags)
 {
        assert(!thread0_info.is_blocked);
        thread0_info.is_blocked = TRUE;
@@ -185,6 +185,10 @@ static void thread0_sync_destroy(uth_sync_t *s)
 {
 }
 
+static void thread0_sync_enqueue(struct uthread *uth, uth_sync_t *s)
+{
+}
+
 static struct uthread *thread0_sync_get_next(uth_sync_t *s)
 {
        if (thread0_info.is_blocked) {
index ef0ffc3..d68ef82 100644 (file)
@@ -358,15 +358,11 @@ void uthread_runnable(struct uthread *uthread)
  * own callback/yield_func instead of some 2LS code, that callback needs to
  * call this.
  *
- * If sync is set, then the 2LS must save the uthread in the sync object.  Using
- * the default implementatin is fine.  If sync is NULL, then there's nothing the
- * 2LS should do regarding sync; it'll be told when the thread is runnable.
- *
  * AKA: obviously_a_uthread_has_blocked_in_lincoln_park() */
-void uthread_has_blocked(struct uthread *uthread, uth_sync_t *sync, int flags)
+void uthread_has_blocked(struct uthread *uthread, int flags)
 {
        assert(sched_ops->thread_has_blocked);
-       sched_ops->thread_has_blocked(uthread, sync, flags);
+       sched_ops->thread_has_blocked(uthread, flags);
 }
 
 /* Function indicating an external event has temporarily paused a uthread, but
@@ -509,7 +505,7 @@ void uthread_usleep(unsigned int usecs)
 
 static void __sleep_forever_cb(struct uthread *uth, void *arg)
 {
-       uthread_has_blocked(uth, NULL, UTH_EXT_BLK_MISC);
+       uthread_has_blocked(uth, UTH_EXT_BLK_MISC);
 }
 
 void __attribute__((noreturn)) uthread_sleep_forever(void)
@@ -1340,7 +1336,7 @@ static void __uth_join_cb(struct uthread *uth, void *arg)
 {
        struct uth_join_kicker *jk = (struct uth_join_kicker*)arg;
 
-       uthread_has_blocked(uth, NULL, UTH_EXT_BLK_MISC);
+       uthread_has_blocked(uth, UTH_EXT_BLK_MISC);
        /* After this, and after all threads join, we could be woken up. */
        kref_put(&jk->kref);
 }
@@ -1380,7 +1376,7 @@ void uthread_join(struct uthread *uth, void **retval_loc)
 
 static void __uth_sched_yield_cb(struct uthread *uth, void *arg)
 {
-       uthread_has_blocked(uth, NULL, UTH_EXT_BLK_YIELD);
+       uthread_has_blocked(uth, UTH_EXT_BLK_YIELD);
        uthread_runnable(uth);
 }
 
index 916e6fe..f9be2a5 100644 (file)
@@ -84,7 +84,7 @@ static void __futex_block(struct uthread *uthread, void *arg) {
   }
 
   // Notify the scheduler of the type of yield we did
-  uthread_has_blocked(uthread, NULL, UTH_EXT_BLK_MUTEX);
+  uthread_has_blocked(uthread, UTH_EXT_BLK_MUTEX);
 
   // Unlock the pdr_lock 
   mcs_pdr_unlock(&__futex.lock);
index 1f73bdf..c992b40 100644 (file)
@@ -42,8 +42,7 @@ static void pth_sched_entry(void);
 static void pth_thread_runnable(struct uthread *uthread);
 static void pth_thread_paused(struct uthread *uthread);
 static void pth_thread_blockon_sysc(struct uthread *uthread, void *sysc);
-static void pth_thread_has_blocked(struct uthread *uthread,
-                                   uth_sync_t *sync_obj, int flags);
+static void pth_thread_has_blocked(struct uthread *uthread, int flags);
 static void pth_thread_refl_fault(struct uthread *uth,
                                   struct user_context *ctx);
 static void pth_thread_exited(struct uthread *uth);
@@ -252,8 +251,7 @@ static void pth_thread_blockon_sysc(struct uthread *uthread, void *syscall)
        /* GIANT WARNING: do not touch the thread after this point. */
 }
 
-static void pth_thread_has_blocked(struct uthread *uthread,
-                                   uth_sync_t *sync_obj, int flags)
+static void pth_thread_has_blocked(struct uthread *uthread, int flags)
 {
        struct pthread_tcb *pthread = (struct pthread_tcb*)uthread;
 
@@ -271,8 +269,6 @@ static void pth_thread_has_blocked(struct uthread *uthread,
        default:
                pthread->state = PTH_BLK_MISC;
        };
-       if (sync_obj)
-               __uth_default_sync_enqueue(uthread, sync_obj);
 }
 
 static void __signal_and_restart(struct uthread *uthread,
index 33765c4..5324464 100644 (file)
@@ -38,8 +38,7 @@ static void vmm_sched_entry(void);
 static void vmm_thread_runnable(struct uthread *uth);
 static void vmm_thread_paused(struct uthread *uth);
 static void vmm_thread_blockon_sysc(struct uthread *uth, void *sysc);
-static void vmm_thread_has_blocked(struct uthread *uth, uth_sync_t *sync_obj,
-                                   int flags);
+static void vmm_thread_has_blocked(struct uthread *uth, int flags);
 static void vmm_thread_refl_fault(struct uthread *uth,
                                   struct user_context *ctx);
 static void vmm_thread_exited(struct uthread *uth);
@@ -274,16 +273,13 @@ static void vmm_thread_blockon_sysc(struct uthread *uth, void *syscall)
        /* GIANT WARNING: do not touch the thread after this point. */
 }
 
-static void vmm_thread_has_blocked(struct uthread *uth, uth_sync_t *sync_obj,
-                                   int flags)
+static void vmm_thread_has_blocked(struct uthread *uth, int flags)
 {
        /* The thread blocked on something like a mutex.  It's not runnable, so we
         * don't need to put it on a list, but we do need to account for it not
         * running.  We'll find out (via thread_runnable) when it starts up again.
         */
        acct_thread_blocked((struct vmm_thread*)uth);
-       if (sync_obj)
-               __uth_default_sync_enqueue(uth, sync_obj);
 }
 
 static void refl_error(struct uthread *uth, unsigned int trap_nr,