Subject: Re: PATCH 2.6.17-rc5 tulip free_irq() called too late

From: Jeff Garzik <[email-masked]>
Date: Thu, 08 Jun 2006 11:32:39 -0400

Grant Grundler wrote:
> On Thu, Jun 08, 2006 at 10:43:04AM -0400, Jeff Garzik wrote:
>> (CC'ing our newly minted tulip maintainer, Val)
> 
> Excellent!
> Has MAINTAINERS file been updated? :)

It should be updated, yes.

>> Calling free_irq() while the chip is still active is just a bad idea, 
>> because the chip could raise an interrupt, creating a 
>> screaming-interrupts situation. Consider especially the case of shared 
>> interrupts here, as a concrete example of how this won't work.
> 
> The chip IRQ gets turned off in tulip_down().
> It won't be screaming for very long.

Then you admit that you add a race.

> No one will hear it screaming and no one will care.

False.  Easy counter-example already provided:  Shared interrupts.

For some IRQ architectures, even without shared interrupts, this could 
easily trigger the kernel's screaming-irq detection code.

> Secondly, if tulip has a problem with shared IRQs, it's already there.
> We would have called free_irq() anyway - just a bit later.

Yes... AFTER we told the chip to stop DMA'ing, and delivering interrupts.

I'm frankly surprised you don't see the obvious, natural ordering.  You 
stop the hardware from wanting to deliver interrupts, before 
unregistering the irq handler.  IOW you turn out the lights, before 
leaving the room.

> In the shared IRQ case, I expect free_irq() to unlink this instance
> of the tulip interrupt handler from the CPU vector list. If that

Irrelevant -- that doesn't stop the tulip hardware from raising the irq, 
nor stop the system from attempting to deliver it [in all cases].  In 
the shared interrupt case, that means that another driver's irq handler 
will be called for an event tulip hardware raised.

	Jeff