Re: [PATCH] Tulip interrupt uses non IRQ safe spinlock

[Sergey Vlasov <vsu@altlinux.ru>]

[-- Attachment #1: Type: text/plain, Size: 1699 bytes --]

On Fri, 29 Apr 2005 09:35:21 -0700 David S. Miller wrote:

> Look at most interrupt handlers in the kernel, they use
> spin_lock_irqsave() rather consistently.  If an interrupt
> is registered with SA_SHIRQ, this is a requirement.
> Here is why.
> 
> 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();
> 
> 	for each irq registered {
> 		x->handler();
> 	}
> 	local_irq_disable();
> 
> (see kernel/irq/handle.c)
> 
> At the top level from that handle_IRQ_event() function, the
> IRQ source is ACK'd after those calls.
> 
> 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.

If what you wrote above is really correct, this means that
Documentation/DocBook/kernel-locking.sgml contains wrong information:

>>> The irq handler does not to use spin_lock_irq(), because the
>>> softirq cannot run while the irq handler is running: it can use
>>> spin_lock(), which is slightly faster. The only exception would
>>> be if a different hardware irq handler uses the same lock:
>>> spin_lock_irq() will stop that from interrupting us.

AFAIK, even if interrupts are enabled, the IRQ line which is currently
handled is disabled in the interrupt controller, therefore the
interrupt handler cannot be reentered (for the same device instance).
Did this really change?

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]