Re: [Xen-devel] [PATCH v3 35/47] xen/sched: make vcpu_wake() and vcpu_sleep() core scheduling aware

[Jürgen Groß <jgross@xxxxxxxx>]

On 27.09.19 09:32, Dario Faggioli wrote:
> On Fri, 2019-09-27 at 06:42 +0200, Jürgen Groß wrote:
> > On 25.09.19 15:07, Jürgen Groß wrote:
> > > On 24.09.19 13:55, Jan Beulich wrote:
> > > > On 14.09.2019 10:52, Juergen Gross wrote:
> > > > > @@ -765,16 +774,22 @@ void vcpu_wake(struct vcpu *v)
> > > > >    {
> > > > >        unsigned long flags;
> > > > >        spinlock_t *lock;
> > > > > +    struct sched_unit *unit = v->sched_unit;
> > > > >        TRACE_2D(TRC_SCHED_WAKE, v->domain->domain_id, v-
> > > > > > vcpu_id);
> > > > > -    lock = unit_schedule_lock_irqsave(v->sched_unit, &flags);
> > > > > +    lock = unit_schedule_lock_irqsave(unit, &flags);
> > > > >        if ( likely(vcpu_runnable(v)) )
> > > > >        {
> > > > >            if ( v->runstate.state >= RUNSTATE_blocked )
> > > > >                vcpu_runstate_change(v, RUNSTATE_runnable,
> > > > > NOW());
> > > > > -        sched_wake(vcpu_scheduler(v), v->sched_unit);
> > > > > +        sched_wake(vcpu_scheduler(v), unit);
> > > > Is this correct / necessary when the unit is not asleep as a
> > > > whole?
> > > > After all the corresponding sched_sleep() further up is called
> > > > conditionally only.
> > > Oh, indeed. Will change that.
> > It turned out this is not so easy at it seemed.
> >
> > I encountered dom0 boot hangs with making the call conditional, even
> > when running in cpu scheduling mode. I guess the reason is that a
> > vcpu
> > can call do_poll() which will try to put itself to sleep and in some
> > cases call vcpu_wake() in case the condition already changed. In that
> > case we need the sched_wake() call even if the unit is still running.
> TBH, I think it is ok for this call to be unconditional. Indeed it
> looks a bit weird when you compare this to the sched_sleep() calls in
> vcpu_sleep_nosync_locked(), as they are conditional, but I think a
> comment explaining why this has to be the case would be enough.
>
> E.g., something like what the changelog already say, in
> vcpu_sleep_nosync_locked(), and maybe something like what you said
> here, in vcpu_wake().
Okay, will add comments.


Juergen

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