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

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Florian,

I'm not sure I totally understand what you are asking, so I'm going to
try to answer.

Threads are allocated using the scheduler's allocator, therefore the
scheduler reference is needed until freeing thread resources. 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).


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;
>>   }
>>  
> 

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