kern/thread: fix and improve termination

2 files changed, 47 insertions, 28 deletions

diff --git a/kern/thread.c b/kern/thread.c
index 29173d94..78c75bad 100644
--- a/kern/thread.c
+++ b/kern/thread.c
@@ -99,7 +99,6 @@
 #include <kern/list.h>
 #include <kern/llsync.h>
 #include <kern/macros.h>
-#include <kern/mutex.h>
 #include <kern/panic.h>
 #include <kern/percpu.h>
 #include <kern/shell.h>
@@ -1825,9 +1824,9 @@ thread_init(struct thread *thread, void *stack,
     thread_set_user_priority(thread, attr->priority);
     thread_reset_real_priority(thread);
     memset(thread->tsd, 0, sizeof(thread->tsd));
-    mutex_init(&thread->join_lock);
-    condition_init(&thread->join_cond);
-    thread->exited = 0;
+    thread->join_waiter = NULL;
+    spinlock_init(&thread->join_lock);
+    thread->exiting = false;
     thread->task = task;
     thread->stack = stack;
     strlcpy(thread->name, attr->name, sizeof(thread->name));
@@ -1957,16 +1956,8 @@ thread_free_stack(void *stack)
 void
 thread_destroy(struct thread *thread)
 {
-    struct thread_runq *runq;
-    unsigned long flags, state;
-
     assert(thread != thread_self());
-
-    do {
-        runq = thread_lock_runq(thread, &flags);
-        state = thread->state;
-        thread_unlock_runq(runq, flags);
-    } while (state != THREAD_DEAD);
+    assert(thread->state == THREAD_DEAD);
 
     /* See task_info() */
     task_remove_thread(thread->task, thread);
@@ -1981,15 +1972,29 @@ thread_destroy(struct thread *thread)
 static void
 thread_join_common(struct thread *thread)
 {
-    assert(thread != thread_self());
+    struct thread_runq *runq;
+    unsigned long flags, state;
+    struct thread *self;
 
-    mutex_lock(&thread->join_lock);
+    self = thread_self();
+    assert(thread != self);
+
+    spinlock_lock(&thread->join_lock);
 
-    while (!thread->exited) {
-        condition_wait(&thread->join_cond, &thread->join_lock);
+    assert(!thread->join_waiter);
+    thread->join_waiter = self;
+
+    while (!thread->exiting) {
+        thread_sleep(&thread->join_lock, thread, "exit");
     }
 
-    mutex_unlock(&thread->join_lock);
+    spinlock_unlock(&thread->join_lock);
+
+    do {
+        runq = thread_lock_runq(thread, &flags);
+        state = thread->state;
+        thread_unlock_runq(runq, flags);
+    } while (state != THREAD_DEAD);
 
     thread_unref(thread);
 }
@@ -2386,13 +2391,17 @@ thread_exit(void)
         work_schedule(&zombie.work, 0);
     }
 
-    mutex_lock(&thread->join_lock);
-    thread->exited = 1;
-    condition_signal(&thread->join_cond);
+    /*
+     * Disable preemption before waking up since step 2 of the termination
+     * protocol involves actively polling the thread state.
+     */
+    thread_preempt_disable();
 
-    mutex_unlock(&thread->join_lock);
+    spinlock_lock(&thread->join_lock);
+    thread->exiting = true;
+    thread_wakeup(thread->join_waiter);
+    spinlock_unlock(&thread->join_lock);
 
-    thread_preempt_disable();
     runq = thread_runq_local();
     spinlock_lock_intr_save(&runq->lock, &flags);
 
diff --git a/kern/thread_i.h b/kern/thread_i.h
index 3f350889..7cbd3c72 100644
--- a/kern/thread_i.h
+++ b/kern/thread_i.h
@@ -25,7 +25,7 @@
 #include <kern/condition_types.h>
 #include <kern/cpumap.h>
 #include <kern/list_types.h>
-#include <kern/mutex_types.h>
+#include <kern/spinlock_types.h>
 #include <kern/turnstile_types.h>
 #include <machine/cpu.h>
 #include <machine/tcb.h>
@@ -169,10 +169,20 @@ struct thread {
      */
     void *tsd[THREAD_KEYS_MAX];
 
-    /* Members related to termination */
-    struct mutex join_lock;
-    struct condition join_cond;     /* (j) */
-    int exited;                     /* (j) */
+    /*
+     * Members related to termination.
+     *
+     * The termination protocol is made of two steps :
+     *  1/ The thread exits, thereby reporting that it is exiting while
+     *     holding the join lock. This includes waking up any thread
+     *     currently joining.
+     *  2/ The thread sets its state to dead and calls the scheduler.
+     *     The join operation polls the state, and releases a reference
+     *     when it sees the dead state.
+     */
+    struct thread *join_waiter;     /* (j) */
+    struct spinlock join_lock;
+    bool exiting;                   /* (j) */
 
     struct task *task;              /* (T) */
     struct list task_node;          /* (T) */