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

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

OK, I see, then this is indeed fine. Do you think though to remove that 
outdated comment then? You could in theory just do it with this patch 
and quickly point out in the comment why that is an incorrect comment, 
which at the same time clarifies this one?
Reviewed-by: Florian Schmidt <florian.schmidt@xxxxxxxxx>

On 5/16/19 11:17 AM, Costin Lupu wrote:
> 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)
--
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