From 1e9b0989bc5634cddc59f40da53674030829858c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Mon, 27 Mar 2017 20:21:36 +0200 Subject: [PATCH 1/4] Unlock the zombie threads mtx before leaving critical section --- sys/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/thread.c b/sys/thread.c index ac909e4c08..705e418245 100644 --- a/sys/thread.c +++ b/sys/thread.c @@ -137,9 +137,9 @@ noreturn void thread_exit(int exitcode) { td->td_exitcode = exitcode; sleepq_broadcast(&td->td_exitcode); td->td_state = TDS_INACTIVE; - critical_leave(); mtx_unlock(&zombie_threads_mtx); + critical_leave(); sched_yield(); From 979353ad342a484077a49f7432c3a05d674806c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 29 Mar 2017 14:46:51 +0200 Subject: [PATCH 2/4] Implemented thread_join using a cv --- include/thread.h | 5 +++++ sys/thread.c | 25 ++++++++++++++++--------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/include/thread.h b/include/thread.h index 90d73e5a6a..8d0c96afcb 100644 --- a/include/thread.h +++ b/include/thread.h @@ -6,6 +6,8 @@ #include #include #include +#include +#include typedef uint8_t td_prio_t; typedef uint32_t tid_t; @@ -25,6 +27,9 @@ typedef struct thread { TAILQ_ENTRY(thread) td_zombieq; /* a link on zombie queue */ char *td_name; tid_t td_tid; + /* Locks*/ + mtx_t td_lock; + condvar_t td_waitcv; /* thread state */ enum { TDS_INACTIVE = 0x0, TDS_WAITING, TDS_READY, TDS_RUNNING } td_state; uint32_t td_flags; /* TDF_* flags */ diff --git a/sys/thread.c b/sys/thread.c index 705e418245..1ff5980325 100644 --- a/sys/thread.c +++ b/sys/thread.c @@ -70,6 +70,9 @@ thread_t *thread_create(const char *name, void (*fn)(void *), void *arg) { td->td_kstack.stk_base = (void *)PG_VADDR_START(td->td_kstack_obj); td->td_kstack.stk_size = PAGESIZE; + mtx_init(&td->td_lock, MTX_DEF); + cv_init(&td->td_waitcv, "wait"); + ctx_init(td, fn, arg); /* Do not lock the mutex if this call to thread_create was done before any @@ -119,6 +122,8 @@ noreturn void thread_exit(int exitcode) { log("Thread '%s' {%p} has finished.", td->td_name, td); + mtx_lock(&td->td_lock); + /* Thread must not exit while in critical section! However, we can't use assert here, because assert also calls thread_exit. Thus, in case this condition is not met, we'll log the problem, and try to fix the problem. */ @@ -131,14 +136,16 @@ noreturn void thread_exit(int exitcode) { fdtab_release(td->td_fdtable); mtx_lock(&zombie_threads_mtx); - - critical_enter(); TAILQ_INSERT_TAIL(&zombie_threads, td, td_zombieq); - td->td_exitcode = exitcode; - sleepq_broadcast(&td->td_exitcode); - td->td_state = TDS_INACTIVE; - mtx_unlock(&zombie_threads_mtx); + + critical_enter(); + { + td->td_exitcode = exitcode; + td->td_state = TDS_INACTIVE; + cv_signal(&td->td_waitcv); + mtx_unlock(&td->td_lock); + } critical_leave(); sched_yield(); @@ -153,10 +160,10 @@ void thread_join(thread_t *p) { thread_t *otd = p; log("Joining '%s' {%p} with '%s' {%p}", td->td_name, td, otd->td_name, otd); - critical_enter(); + mtx_lock(&otd->td_lock); while (otd->td_state != TDS_INACTIVE) - sleepq_wait(&otd->td_exitcode, "Joining threads"); - critical_leave(); + cv_wait(&otd->td_waitcv, &otd->td_lock); + mtx_unlock(&otd->td_lock); } /* It would be better to have a hash-map from tid_t to thread_t, From 5c45237447ea33f7acdf7f637c07dc10adec3924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 29 Mar 2017 17:17:31 +0200 Subject: [PATCH 3/4] Renamed td_waitcv to td_waitcv --- sys/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sys/thread.c b/sys/thread.c index 1ff5980325..2a9e676635 100644 --- a/sys/thread.c +++ b/sys/thread.c @@ -71,7 +71,7 @@ thread_t *thread_create(const char *name, void (*fn)(void *), void *arg) { td->td_kstack.stk_size = PAGESIZE; mtx_init(&td->td_lock, MTX_DEF); - cv_init(&td->td_waitcv, "wait"); + cv_init(&td->td_waitcv, "td_waitcv"); ctx_init(td, fn, arg); From 954305d9beb1f3401f2f296c373cc1905b421e37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Sun, 2 Apr 2017 14:57:52 +0200 Subject: [PATCH 4/4] Minor cleanup --- include/thread.h | 7 ++++--- sys/thread.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/include/thread.h b/include/thread.h index 8d0c96afcb..f2c415e61f 100644 --- a/include/thread.h +++ b/include/thread.h @@ -21,15 +21,16 @@ typedef struct fdtab fdtab_t; #define TDF_NEEDSWITCH 0x00000002 /* must switch on next opportunity */ typedef struct thread { + /* Locks*/ + mtx_t td_lock; + condvar_t td_waitcv; /* CV for thread exit, used by join */ + /* List links */ TAILQ_ENTRY(thread) td_all; /* a link on all threads list */ TAILQ_ENTRY(thread) td_runq; /* a link on run queue */ TAILQ_ENTRY(thread) td_sleepq; /* a link on sleep queue */ TAILQ_ENTRY(thread) td_zombieq; /* a link on zombie queue */ char *td_name; tid_t td_tid; - /* Locks*/ - mtx_t td_lock; - condvar_t td_waitcv; /* thread state */ enum { TDS_INACTIVE = 0x0, TDS_WAITING, TDS_READY, TDS_RUNNING } td_state; uint32_t td_flags; /* TDF_* flags */ diff --git a/sys/thread.c b/sys/thread.c index 2a9e676635..a6ed8ed3a9 100644 --- a/sys/thread.c +++ b/sys/thread.c @@ -143,7 +143,7 @@ noreturn void thread_exit(int exitcode) { { td->td_exitcode = exitcode; td->td_state = TDS_INACTIVE; - cv_signal(&td->td_waitcv); + cv_broadcast(&td->td_waitcv); mtx_unlock(&td->td_lock); } critical_leave();