[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Minios-devel] [UNIKRAFT PATCH v2 5/8] lib/uksched: Add support for waiting threads



Actually, now that I look at the next patch...

On 1/11/19 12:22 AM, Costin Lupu wrote:

diff --git a/lib/ukschedcoop/schedcoop.c b/lib/ukschedcoop/schedcoop.c
index e565240..f7ab92d 100644
--- a/lib/ukschedcoop/schedcoop.c
+++ b/lib/ukschedcoop/schedcoop.c
@@ -125,6 +125,12 @@ static void schedcoop_schedule(struct uk_sched *s)
                uk_sched_thread_switch(s, prev, next);
UK_TAILQ_FOREACH_SAFE(thread, &prv->exited_threads, thread_list, tmp) {
+               struct thread_info_base *tib = thread->sched_info;
+
+               if (!tib->is_detached)
+                       /* someone will eventually wait for it */
+                       continue;
+
                if (thread != prev) {
                        UK_TAILQ_REMOVE(&prv->exited_threads,
                                        thread, thread_list);
@@ -167,6 +173,7 @@ static void schedcoop_thread_remove(struct uk_sched *s, 
struct uk_thread *t)
/* Put onto exited list */
        UK_TAILQ_INSERT_HEAD(&prv->exited_threads, t, thread_list);
+       uk_thread_exit(t);
ukplat_lcpu_restore_irqf(flags);

Doesn't calling uk_thread_exit(t) only after putting it on the exited_threads list a race condition? Because if now the main scheduling loop (as in schedcoop_schedule() is called again after inserting, but before thread_exit() is called, the could lead to an assertion failure when schedcoop_schedule() calls uk_sched_thread_destroy() on that thread, but uk_thread_exit() hasn't finished yet.

I understand that this probably is a crazy corner case, and I'm not sure I can even ever occur without SMP support, but, on the other hand: is there any harm in switching the order? And actually, wouldn't it make sense to put the list insertion into uk_thread_exit()? It's an important part of thread handling, so it could just be done in there, right? Although I realize that you only do this in the next patch, so... maybe only do that in there? Seems regardless of the order, one thing from the earlier patch always better waits until the latter patch.

Cheers,
Florian

--
Dr. Florian Schmidt
フローリアン・シュミット
Research Scientist,
Systems and Machine Learning Group
NEC Laboratories Europe
Kurfürsten-Anlage 36, D-69115 Heidelberg
Tel.     +49 (0)6221 4342-265
Fax:     +49 (0)6221 4342-155
e-mail:  florian.schmidt@xxxxxxxxx
============================================================
Registered at Amtsgericht Mannheim, Germany, HRB728558

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.