Re: [PATCH for 4.7 1/4] xen: sched: avoid spuriously re-enabling IRQs in csched2_switch_sched()

[George Dunlap <george.dunlap@citrix.com>]

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.faggioli@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.dunlap@citrix.com>
> Cc: Wei Liu <wei.liu2@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