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

Re: [Minios-devel] [UNIKRAFT PATCH 2/8] lib/uksched: Do not reset sched attribute on thread removal



Hi Costin,

On 5/16/19 11:25 AM, Costin Lupu wrote:
I'm not sure I totally understand what you are asking, so I'm going to
try to answer.

It worked
so far because threads aren't destroyed in the upstreamed code or if
they are then it's in some error handling code and it's not done
properly (see the previous patch).

That was exactly what I was wondering, thanks.

Reviewed-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>



Costin


On 5/16/19 11:14 AM, Florian Schmidt wrote:
Hi Costin,

could you give a short explanation what the semantics of t->sched and
setting it to NULL is? Is this value actually used (and expected to be
non-null) at a later stage during the thread teardown? I first though
"oh, uk_sched_thread_destroy() probably wants to look up the scheduler,
but wait, how could this ever have worked then?", but it is called from
the main scheduler loop and so gets the scheduler info from there
instead of the thread.

So, I'm wondering whether I'm missing something. Sure, a superfluous
call to setting something to null is extra work for no gain (and
arguably it's semantically wrong, because the thread still kinda is
assigned to the scheduler, if only because it's the scheduler's job to
clean up the thread), but is there something more pressing and buggy
that I'm missing?

Cheers,
Florian

On 4/23/19 12:41 PM, Costin Lupu wrote:
Scheduler reference should be kept until destroying the thread,
basically during the entire life of the thread. uk_sched_thread_remove()
is always called before destroying the thread, therefore it must not
remove the scheduler reference from thread.

Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
---
   lib/uksched/include/uk/sched.h | 1 -
   1 file changed, 1 deletion(-)

diff --git a/lib/uksched/include/uk/sched.h
b/lib/uksched/include/uk/sched.h
index cc5fbe93..f9dc16d0 100644
--- a/lib/uksched/include/uk/sched.h
+++ b/lib/uksched/include/uk/sched.h
@@ -130,7 +130,6 @@ static inline int uk_sched_thread_remove(struct
uk_sched *s,
       UK_ASSERT(t);
       UK_ASSERT(t->sched == s);
       s->thread_remove(s, t);
-    t->sched = NULL;
       return 0;
   }


--
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®.