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

[Florian Schmidt <Florian.Schmidt@xxxxxxxxx>]

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