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

[Dario Faggioli <dfaggioli@xxxxxxxx>]

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().

Regards
-- 
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)

Attachment: signature.asc
Description: This is a digitally signed message part

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