[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi Dario,

>> > 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@xxxxxxxxxx>
>> > ---
>
>> > 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

Thanks for the explanation! Then the patch looks good to me, at least
for RTDS scheduler. :-)

Best,

Meng


-----------
Meng Xu
PhD Student in Computer and Information Science
University of Pennsylvania
http://www.cis.upenn.edu/~mengxu/

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.