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

["Jan Beulich" <jbeulich@xxxxxxxxxx>]

>>> Juergen Gross <juergen.gross@xxxxxxxxxxxxxxxxxxx> 26.03.09 13:36 >>>
>Jan Beulich wrote:
>>>>> Juergen Gross <juergen.gross@xxxxxxxxxxxxxxxxxxx> 26.03.09 10:00 >>>
>>> Attached patch reduces interrupt latency in lock handling.
>>> spin_lock_irq and spin_lock_irqsave used to turn off IRQs and then tried to
>>> get the lock. If the lock was already held, waiting for the lock was done 
>>> with
>>> IRQs still off.
>>> The patch reenables IRQs during the wait loop.
>> 
>> This is wrong - you must not enable interrupts if they weren't enabled upon
>> entry to these two functions.
>
>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.)

>I'm not enabling IRQs unconditionally in spin_lock_irqsave, of course, but use
>the flags value saved before...

Oh, sorry, I blindly implied the second function to use the same methods as
the first one. And really I'd think it's cheaper to use local_irq_disable() here
(but of course retain local_irq_restore()).

Btw., why do you think the issue is more important to address in Xen than
in Linux (where not to long ago the opposite move happened, since re-
enabling interrupts with ticket locks is much trickier and hence wasn't
considered worthwhile afair.

Jan


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