Herbert Xu wrote:
> On Fri, Apr 29, 2005 at 09:35:21AM -0700, David S. Miller wrote:
>
>>On i386 (or any other platform using the generic IRQ layer),
>>for example, unless you specify SA_INTERRUPT at
>>request_irq() time, the handler dispatch is:
>>
>> local_irq_enable();
>
>
> Yes you're absolutely correct.
>
>
>>However, if you have multiple devices on that IRQ line, you
>>run into a problem. Let's say TUlip interrupts first and
>>we go into the Tulip driver and grab the lock, next the other
>>device interrupts and we jump into the Tulip interrupt handler
>>again, we will deadlock but what we should have done is use
>>IRQ disabling spin locking like Mark's fix does.
>
>
> However, I don't see how this can happen. __do_IRQ ensures
> that the handlers on a single IRQ aren't reentered by desc->lock
> and desc->status. Softirqs are also kept out by irq_enter. Am
> I missing something?
As far as I can see desc->lock is dropped before handle_IRQ_event() is
called in __do_IRQ() (kernel/irq/handle.c:170) and desc->status does not
prevent the execution of the IRQ handler. Same with softirqs,
interrupts are enabled when the handler is called (kernel/softirq.c:89).
Thanks
Mark
>>Therefore I think his patch is perfectly fine and this is an
>>excellent area for an audit of the entire tree. I even just
>>noticed recently that some of the Sparc drivers/serial/
>>drivers were not taking the interrupt handler lock correctly like
>>this (ie. using irqsave).
>
>
> Unless these drivers are registering two different IRQ lines that
> can nest within each other, I still think that a plain spin_lock is
> safe and desirable.
>
> Cheers,
|