On 03/05/16 22:46, Dario Faggioli wrote: > In fact, interrupts are already disabled when calling > the hook from schedule_cpu_switch(), and hence using > anything different than just spin_lock() is wrong (and > ASSERT()-s in debug builds) or unnecessary. > > Signed-off-by: Dario Faggioli <dario.faggi...@citrix.com>
Good catch. But would it be better to either 1) add an assert that irqs are disabled, or 2) use spin_lock_irqsave(), just to make sure the two bits of code won't silently go out of sync? Re Jan's comment -- you meant to say that spin_lock_irq() is wrong and spin_lock_irqsave() is unnecessary? That's probably a bit more inference than we want changeset messages to have, ideally. :-) I wouldn't require a re-spin just for that, but if you are going to respin it, I would suggest just dropping the "and hence...". Thanks, -George > --- > Cc: George Dunlap <george.dun...@citrix.com> > Cc: Wei Liu <wei.l...@citrix.com> > --- > xen/common/sched_credit2.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c > index 46b9279..50c1b9d 100644 > --- a/xen/common/sched_credit2.c > +++ b/xen/common/sched_credit2.c > @@ -2232,7 +2232,7 @@ csched2_switch_sched(struct scheduler *new_ops, > unsigned int cpu, > * And owning exactly that one (the lock of the old scheduler of this > * cpu) is what is necessary to prevent races. > */ > - spin_lock_irq(&prv->lock); > + spin_lock(&prv->lock); > > idle_vcpu[cpu]->sched_priv = vdata; > > @@ -2257,7 +2257,7 @@ csched2_switch_sched(struct scheduler *new_ops, > unsigned int cpu, > smp_mb(); > per_cpu(schedule_data, cpu).schedule_lock = &prv->rqd[rqi].lock; > > - spin_unlock_irq(&prv->lock); > + spin_unlock(&prv->lock); > } > > static void > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
