[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 1/6] xen: sched: fix locking of remove_vcpu() in credit1
On 04/11/15 17:17, Dario Faggioli wrote: > In fact, csched_vcpu_remove() (i.e., the credit1 > implementation of remove_vcpu()) manipulates runqueues, > so holding the runqueue lock is necessary. > > However, the vCPU just can't be on the runqueue, when > the function is called. We can therefore ASSERT() that, > and avoid doing any runqueue manipulations (rather than > adding the runqueue locking around it). > > Also, while there, *_lock_irq() (for the private lock) is > enough, there is no need to *_lock_irqsave(). > > Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx> > --- > Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > Cc: Jan Beulich <JBeulich@xxxxxxxx> > --- > Changes from v3: > * instead of adding locking, get rid of __runq_remove(), > and add an ASSERT() about vCPU not being in runq already, > as suggested during review. > > Changes from the other series: > * split the patch (wrt the original patch, in the original > series), and take care, in this one, only of remove_vcpu(); > * removed pointless parentheses. > --- > The fact that vCPU can't be in runqueue when calling remove_vcpu() is true for > other schedulers as well. In them, though, there isn't any race condition to > fix. Therefore, taking care of the other schedulers will happen in a followup > series. > --- > xen/common/sched_credit.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c > index 1b30e67..6dfcff6 100644 > --- a/xen/common/sched_credit.c > +++ b/xen/common/sched_credit.c > @@ -934,28 +934,25 @@ csched_vcpu_remove(const struct scheduler *ops, struct > vcpu *vc) > struct csched_private *prv = CSCHED_PRIV(ops); > struct csched_vcpu * const svc = CSCHED_VCPU(vc); > struct csched_dom * const sdom = svc->sdom; > - unsigned long flags; > > SCHED_STAT_CRANK(vcpu_remove); > > + ASSERT(!__vcpu_on_runq(svc)); > + > if ( test_and_clear_bit(CSCHED_FLAG_VCPU_PARKED, &svc->flags) ) > { > SCHED_STAT_CRANK(vcpu_unpark); > vcpu_unpause(svc->vcpu); > } > > - if ( __vcpu_on_runq(svc) ) > - __runq_remove(svc); > - > - spin_lock_irqsave(&(prv->lock), flags); > + spin_lock_irq(&prv->lock); > > if ( !list_empty(&svc->active_vcpu_elem) ) > __csched_vcpu_acct_stop_locked(prv, svc); > > - spin_unlock_irqrestore(&(prv->lock), flags); > + spin_unlock_irq(&prv->lock); > > BUG_ON( sdom == NULL ); > - BUG_ON( !list_empty(&svc->runq_elem) ); > } > > static void > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |