Re: [Xen-devel] [Patch] don't spin with irq disabled

[Keir Fraser <keir.fraser@xxxxxxxxxxxxx>]

On 27/03/2009 07:41, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:

>> spin_lock_irq disables always IRQs. spin_unlock_irq enables always IRQs. They
>> are always used in pairs, so IRQs should always be enabled on entry of
>> spin_lock_irq.
> 
> No, I wouldn't suggest making an assumption like this - some code could allow
> interrupts to be disabled when acquiring the lock, but intentionally enabling
> them when releasing it. (Personally, I think there shouldn't be any users of
> this function pair in the first place, as I don't think forcibly enabling
> interrupts
> is a correct thing to do in all but *very* few cases.)

It seems to me a perfectly reasonable function pair to use when using an
IRQ-safe lock from code that you know cannot possibly already have IRQs
disabled. Anyone using the function pair to implicitly enable interrupts is
an evil person, and I already put assertions in to prevent that kind of
thing, in _{spin,read,write}_lock_irq(). The assertions only fired in a few
cases, and all of them looked like genuine bugs.

Anyway, as to the more general question of is this all worthwhile, I'm not
sure. I actually like the potential advantage of needing fewer and simpler
asm stubs in arch-specific spinlock.h, since Juergen has accidentally fallen
on an opportunity to get rid of _raw_spin_lock. I'm not sure whether
_raw_trylock is more expensive than _raw_lock though. On x86 it's the
difference between lock;dec and xchg -- I don't see any reason why one would
be more expensive than the other.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel