Re: [PATCH 05/15] drivers/char: minor irq handler cleanups

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2008-04-22 at 12:13 +0200, Rogier Wolff wrote:
> > You should let the kernel generic code deal with the runaway
> interrupt,
> > it should be capable of doing so nowadays pretty reliably.
> > 
> > free_irq() is definitely not going to be happy when it start messing
> > with /proc from an interrupt... It will at least give you a WARN_ON.
> 
> The situation is NOT normal operation. It is an emergency measure in
> an attempt to prevent a full hang. It is great that other parts of the
> kernel also "shout" that something is wrong.

But free_irq at interrupt time has great chances of crashing the machine
or currupting data structures in subtle ways. You should -not- do that.

Either let the kernel handle the runaway interrupt, or eventually if you
want to be brutal, call disable_irq_nosync yourself, but not free_irq.

> Consider it similar to a "kernel null pointer dereference". Once that
> happens, all bets are off. In practise you've probably seen one, and
> you were able to continue to work. It is advisable to save everything
> you can, and reboot. This is similar. 

It would be if you used something like disable_irq_nosync. Not free_irq.
The later is just bogus.

> The "generic code for runaway interrupts" didn't exist when this was
> written. If it exists, and works for the case that this was written
> for, then all is fine, and we can remove my code. As you can see, I
> copied over the code from one driver to the next after I got bitten
> again with the second driver. So having something generic is of course
> preferable. :-)

Well, if you know how to trigger the bug, it would be useful to verify
that the kernel generic code properly detects and masks the runaway
interrupt. If that works, then remove your code completely. If not, it
would be useful to figure out why :-) But either way, just replace it
with disable_irq_nosync.

Cheers,
Ben.