Re: [Xen-devel] [PATCH 1/3] spinlock: use local_irq_disable() instead of local_irq_save() where possible

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Jan 08, 2015 at 04:13:03PM +0000, Jan Beulich wrote:
> ... as generally being a cheaper operation.

I was wondering if it would be possible to change some of the
EFLAGS after when we go in the 'cpu_relax' - and an interrupt
happens, we process it, alter the EFLAGS, then when we are
done, the EFLAGS are different - which the original code would
save when it was done sitting on the cpu_relax() loop.

Actually that sounds bad - we only want to restore the flags
that we had when going in this spin lock. Would make sense
to add an ASSERT to check for flags being different from the
EFLAGS?


> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -162,7 +162,7 @@ unsigned long _spin_lock_irqsave(spinloc
>          local_irq_restore(flags);
>          while ( likely(_raw_spin_is_locked(&lock->raw)) )
>              cpu_relax();
> -        local_irq_save(flags);
> +        local_irq_disable();
>      }
>      LOCK_PROFILE_GOT;
>      preempt_disable();
> @@ -313,7 +313,7 @@ unsigned long _read_lock_irqsave(rwlock_
>              local_irq_restore(flags);
>              while ( (x = lock->lock) & RW_WRITE_FLAG )
>                  cpu_relax();
> -            local_irq_save(flags);
> +            local_irq_disable();
>          }
>      } while ( cmpxchg(&lock->lock, x, x+1) != x );
>      preempt_disable();
> @@ -409,7 +409,7 @@ unsigned long _write_lock_irqsave(rwlock
>              local_irq_restore(flags);
>              while ( (x = lock->lock) & RW_WRITE_FLAG )
>                  cpu_relax();
> -            local_irq_save(flags);
> +            local_irq_disable();
>          }
>      } while ( cmpxchg(&lock->lock, x, x|RW_WRITE_FLAG) != x );
>      while ( x != 0 )
> 
> 
> 

> spinlock: use local_irq_disable() instead of local_irq_save() where possible
> 
> ... as generally being a cheaper operation.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -162,7 +162,7 @@ unsigned long _spin_lock_irqsave(spinloc
>          local_irq_restore(flags);
>          while ( likely(_raw_spin_is_locked(&lock->raw)) )
>              cpu_relax();
> -        local_irq_save(flags);
> +        local_irq_disable();
>      }
>      LOCK_PROFILE_GOT;
>      preempt_disable();
> @@ -313,7 +313,7 @@ unsigned long _read_lock_irqsave(rwlock_
>              local_irq_restore(flags);
>              while ( (x = lock->lock) & RW_WRITE_FLAG )
>                  cpu_relax();
> -            local_irq_save(flags);
> +            local_irq_disable();
>          }
>      } while ( cmpxchg(&lock->lock, x, x+1) != x );
>      preempt_disable();
> @@ -409,7 +409,7 @@ unsigned long _write_lock_irqsave(rwlock
>              local_irq_restore(flags);
>              while ( (x = lock->lock) & RW_WRITE_FLAG )
>                  cpu_relax();
> -            local_irq_save(flags);
> +            local_irq_disable();
>          }
>      } while ( cmpxchg(&lock->lock, x, x|RW_WRITE_FLAG) != x );
>      while ( x != 0 )

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


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