Re: [Minios-devel] [UNIKRAFT PATCH 1/8] lib/uknetdev: Destroy dispatcher thread using public scheduling API

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Florian,

In deed, the comment "Schedule will free resources" is outdated since we
introduced the thread waiting function. So, in order to free thread
resources one should either call uk_thread_wait() or set the thread as
detached in order to delegate the free operation to the scheduler as before.

Costin

On 5/16/19 11:03 AM, Florian Schmidt wrote:
> Hi Costin,
> 
> the patch looks right in its concept. I'm just wondering: is the
> uk_thread_wait actually necessary and wanted at this stage? It seems
> uk_thread_kill() calls uk_sched_thread_remove(), which calls
> ukschedcoop_thread_remove() (or in theory any other implementation), and
> that one runs a scheduler loop with the comment "Schedule will free
> resources". So isn't uk_thread_kill(h->dispatcher) already enough?
> 
> Cheers,
> Florian
> 
> On 4/23/19 12:41 PM, Costin Lupu wrote:
>> When thread destruction is wanted, one should kill the thread and wait
>> for it in order to free its resources. uk_sched_thread_destroy() is not
>> part of public scheduling API.
>>
>> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
>> ---
>>   lib/uknetdev/netdev.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/uknetdev/netdev.c b/lib/uknetdev/netdev.c
>> index cda8a82b..90ecd0a4 100644
>> --- a/lib/uknetdev/netdev.c
>> +++ b/lib/uknetdev/netdev.c
>> @@ -311,8 +311,10 @@ static void _destroy_event_handler(struct
>> uk_netdev_event_handler *h
>>   #ifdef CONFIG_LIBUKNETDEV_DISPATCHERTHREADS
>>       UK_ASSERT(h->dispatcher_s);
>>   -    if (h->dispatcher)
>> -        uk_sched_thread_destroy(h->dispatcher_s, h->dispatcher);
>> +    if (h->dispatcher) {
>> +        uk_thread_kill(h->dispatcher);
>> +        uk_thread_wait(h->dispatcher);
>> +    }
>>       h->dispatcher = NULL;
>>         if (h->dispatcher_name)
>>
> 

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