Re: [Minios-devel] [UNIKRAFT PATCH 5/8] lib/uksched: Revisit thread exiting logic

[Felipe Huici <Felipe.Huici@xxxxxxxxx>]

Hi,


On 9/18/18, 5:27 PM, "Costin Lupu" <costin.lupu@xxxxxxxxx> wrote:

>We use a list for terminated threads on all schedulers because it
>keeps references to those threads until wait will be called for
>them.

s/until wait will be/until wait is

The rest is fine, thanks.

— Felipe

>Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>---
> lib/uksched/include/uk/sched.h  |  1 +
> lib/uksched/include/uk/thread.h |  4 ++++
> lib/uksched/sched.c             |  7 ++++---
> lib/ukschedcoop/schedcoop.c     | 11 +++--------
> 4 files changed, 12 insertions(+), 11 deletions(-)
>
>diff --git a/lib/uksched/include/uk/sched.h
>b/lib/uksched/include/uk/sched.h
>index 443dbf3..c3e2866 100644
>--- a/lib/uksched/include/uk/sched.h
>+++ b/lib/uksched/include/uk/sched.h
>@@ -87,6 +87,7 @@ struct uk_sched {
> 
>       /* internal */
>       struct uk_thread idle;
>+      struct uk_thread_list exited_threads;
>       struct ukplat_ctx_callbacks plat_ctx_cbs;
>       struct uk_alloc *allocator;
>       struct uk_sched *next;
>diff --git a/lib/uksched/include/uk/thread.h
>b/lib/uksched/include/uk/thread.h
>index d28c458..7a1b630 100644
>--- a/lib/uksched/include/uk/thread.h
>+++ b/lib/uksched/include/uk/thread.h
>@@ -87,11 +87,15 @@ struct uk_thread *uk_thread_current(void)
> }
> 
> #define RUNNABLE_FLAG   0x00000001
>+#define EXITED_FLAG     0x00000002
> 
> #define is_runnable(_thread)    ((_thread)->flags &   RUNNABLE_FLAG)
> #define set_runnable(_thread)   ((_thread)->flags |=  RUNNABLE_FLAG)
> #define clear_runnable(_thread) ((_thread)->flags &= ~RUNNABLE_FLAG)
> 
>+#define is_exited(_thread)      ((_thread)->flags &   EXITED_FLAG)
>+#define set_exited(_thread)     ((_thread)->flags |=  EXITED_FLAG)
>+
> int uk_thread_init(struct uk_thread *thread,
>               struct ukplat_ctx_callbacks *cbs, struct uk_alloc *allocator,
>               const char *name, void *stack,
>diff --git a/lib/uksched/sched.c b/lib/uksched/sched.c
>index 968723c..c5b7c9f 100644
>--- a/lib/uksched/sched.c
>+++ b/lib/uksched/sched.c
>@@ -122,6 +122,7 @@ struct uk_sched *uk_sched_create(struct uk_alloc *a,
>size_t prv_size)
>       }
> 
>       sched->allocator = a;
>+      UK_TAILQ_INIT(&sched->exited_threads);
>       sched->prv = (void *) sched + sizeof(struct uk_sched);
> 
>       return sched;
>@@ -217,6 +218,9 @@ void uk_sched_thread_destroy(struct uk_sched *sched,
>struct uk_thread *thread)
> {
>       UK_ASSERT(sched != NULL);
>       UK_ASSERT(thread != NULL);
>+      UK_ASSERT(is_exited(thread));
>+
>+      UK_TAILQ_REMOVE(&sched->exited_threads, thread, thread_list);
> 
>       uk_free(sched->allocator, thread->sched_info);
>       uk_thread_fini(thread, sched->allocator);
>@@ -238,9 +242,6 @@ void uk_sched_thread_exit(void)
>       struct uk_thread *thread;
> 
>       thread = uk_thread_current();
>-
>-      uk_printd(DLVL_INFO, "Thread \"%s\" exited.\n", thread->name);
>-
>       UK_ASSERT(thread->sched);
>       uk_sched_thread_remove(thread->sched, thread);
>       UK_CRASH("Error stopping thread.");
>diff --git a/lib/ukschedcoop/schedcoop.c b/lib/ukschedcoop/schedcoop.c
>index f616330..a2bf2ad 100644
>--- a/lib/ukschedcoop/schedcoop.c
>+++ b/lib/ukschedcoop/schedcoop.c
>@@ -37,7 +37,6 @@
> 
> struct schedcoop_private {
>       struct uk_thread_list thread_list;
>-      struct uk_thread_list exited_threads;
>       int threads_started;
> };
> 
>@@ -124,12 +123,9 @@ static void schedcoop_schedule(struct uk_sched *s)
>       if (prev != next)
>               uk_sched_thread_switch(s, prev, next);
> 
>-      UK_TAILQ_FOREACH_SAFE(thread, &prv->exited_threads, thread_list, tmp) {
>-              if (thread != prev) {
>-                      UK_TAILQ_REMOVE(&prv->exited_threads,
>-                                      thread, thread_list);
>+      UK_TAILQ_FOREACH_SAFE(thread, &s->exited_threads, thread_list, tmp) {
>+              if (thread != prev)
>                       uk_thread_destroy(thread);
>-              }
>       }
> }
> 
>@@ -166,7 +162,7 @@ static void schedcoop_thread_remove(struct uk_sched
>*s, struct uk_thread *t)
>       clear_runnable(t);
> 
>       /* Put onto exited list */
>-      UK_TAILQ_INSERT_HEAD(&prv->exited_threads, t, thread_list);
>+      UK_TAILQ_INSERT_HEAD(&s->exited_threads, t, thread_list);
> 
>       ukplat_lcpu_restore_irqf(flags);
> 
>@@ -211,7 +207,6 @@ struct uk_sched *uk_schedcoop_init(struct uk_alloc *a)
>       ukplat_ctx_callbacks_init(&sched->plat_ctx_cbs, ukplat_ctx_sw);
> 
>       prv = sched->prv;
>-      UK_TAILQ_INIT(&prv->exited_threads);
>       UK_TAILQ_INIT(&prv->thread_list);
>       prv->threads_started = 0;
> 
>-- 
>2.11.0
>

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel