Re: [PATCH v3 2/6] xen: sched: fix locking for insert_vcpu() in credit1 and RTDS

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

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

On Fri, 2015-10-30 at 19:00 -0400, Meng Xu wrote:
> Hi Dario,
>
Hi,

> > This is fixed as follows:
> >  - take the lock in the hook implementations, in specific
> >    schedulers' code;
> >  - avoid calling insert_vcpu(), for the idle vCPU, in
> >    schedule_cpu_switch(). In fact, idle vCPUs are set to run
> >    immediately, and the various schedulers won't insert them
> >    in their runqueues anyway, even when explicitly asked to.
> > 
> > While there, still in schedule_cpu_switch(), locking with
> > _irq() is enough (there's no need to do *_irqsave()).
> > 
> > Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> > ---

> > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> > index 6f71e0d..e16bd3a 100644
> > --- a/xen/common/sched_credit.c
> > +++ b/xen/common/sched_credit.c
> > @@ -903,10 +903,16 @@ static void
> >  csched_vcpu_insert(const struct scheduler *ops, struct vcpu *vc)
> >  {
> >      struct csched_vcpu *svc = vc->sched_priv;
> > +    spinlock_t *lock;
> > +    unsigned long flags;
> > +
> > +    lock = vcpu_schedule_lock_irqsave(vc, &flags);
> 
> 
> This is inconsistent with the commit log.
>
Mmm... no, the changelog speaks about schedule_cpu_switch(), not this
functions.

For this function, the conversion from _irqsave() to _irq() is done
later in the series, when the call to insert_vcpu() is removed from the
boot path, and hence does not require IRQs to be disabled when called
(and the changelog of that later patch explains this).

> I also checked the
> 
> branch rel/sched/fix-vcpu-ins-rem-v2 in your repo., it is the
> following code:
> 
> lock = vcpu_schedule_lock_irq(vc, &flags);
> 
> I guess maybe you forgot to change it in this commit but change it
> the
> following commit?
> 
No, this is one of the few thing that changed between v2 and v3.

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: 181 bytes --]

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

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