Re: [PATCH 1/3] xen: sched: introduce the 'null' semi-static scheduler

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2236 bytes --]

On Mon, 2017-03-27 at 11:31 +0100, George Dunlap wrote:
> On 17/03/17 18:42, Dario Faggioli wrote:
> > +static void null_vcpu_insert(const struct scheduler *ops, struct
> > vcpu *v)
> > +{
> > +    struct null_private *prv = null_priv(ops);
> > +    struct null_vcpu *nvc = null_vcpu(v);
> > +    unsigned int cpu;
> > +    spinlock_t *lock;
> > +
> > +    ASSERT(!is_idle_vcpu(v));
> > +
> > + retry:
> > +    lock = vcpu_schedule_lock_irq(v);
> > +
> > +    cpu = pick_cpu(prv, v);
> > +
> > +    /* We hold v->processor's runq lock, but we need cpu's one */
> > +    if ( cpu != v->processor )
> > +    {
> > +        spin_unlock(lock);
> > +        lock = pcpu_schedule_lock(cpu);
> 
> Don't we need to hold the lock for v->processor until we change
> v->processor?  Otherwise someone might call vcpu_schedule_lock(v) at
> this point and reasonably believe that is has the right to modify v.
> 
Yes, this is actually the case.

> Or does this not matter because we're just now calling insert (and so
> nobody else is going to call vcpu_schedule_lock() on v?
> 
Indeed no one will. But I still prefer to turn this into something much
more similar to what other schedulers do, and what is the most correct
and safe to do, i.e., as you suggest, change v->processor while holding
the original lock.

> > diff --git a/xen/common/schedule.c b/xen/common/schedule.c
> > index 223a120..b482037 100644
> > --- a/xen/common/schedule.c
> > +++ b/xen/common/schedule.c
> > @@ -1785,6 +1785,8 @@ int schedule_cpu_switch(unsigned int cpu,
> > struct cpupool *c)
> >  
> >   out:
> >      per_cpu(cpupool, cpu) = c;
> > +    /* Trigger a reschedule so the CPU can pick up some work ASAP.
> > */
> > +    cpu_raise_softirq(cpu, SCHEDULE_SOFTIRQ);
> 
> Is this a more generic fix / improvement?
> 
Yep, I'm moving it in its own patch.

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel