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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.01.15 at 17:02, <konrad.wilk@xxxxxxxxxx> wrote:
> 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?

I'm not sure I'm following what you try to tell me. For one,
interrupts don't change EFLAGS - the IRET restores them. And
second, local_irq_restore() is only guaranteed to restore
EFLAGS.IF - whether it touches other flags if largely undefined.

Jan

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