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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080422101326.GD23482@bitwizard.nl>
Date:	Tue, 22 Apr 2008 12:13:26 +0200
From:	Rogier Wolff <R.E.Wolff@...Wizard.nl>
To:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc:	Rogier Wolff <R.E.Wolff@...Wizard.nl>,
	Jeff Garzik <jeff@...zik.org>, alan@...hat.com,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/15] drivers/char: minor irq handler cleanups

On Tue, Apr 22, 2008 at 06:05:40PM +1000, Benjamin Herrenschmidt wrote:
> 
> On Sat, 2008-04-19 at 08:00 +0200, Rogier Wolff wrote:
> > 
> > You added a "XXX Using free_irq in the interrupt is not wise!". When I
> > wrote that code, I didn't know about this. These lines triggered when
> > the level-triggered PCI interrupt stuck "active" this would mean that
> > NO userspace code would get executed anymore: Hard lock up. Difficult
> > to debug. This happend a few times during development when the code
> > behind the "if (!polled)": "tell the hardware we've seen the
> > interrupt" didn't work. On the other hand, some failures in the field
> > have triggered this. So I think it's wise to keep it in. Disabling the
> > interrupt on the card is not an option, because that's exactly what
> > this is supposed to catch: We're unable to make the card stop
> > interrupting the CPU.
> > 
> > Note that it also doesn't work (i.e. hard lock of the machine) if some
> > other driver is using the same interupt.
> 
> 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.

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. 

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. :-)

	Roger. 

-- 
** R.E.Wolff@...Wizard.nl ** http://www.BitWizard.nl/ ** +31-15-2600998 **
**    Delftechpark 26 2628 XH  Delft, The Netherlands. KVK: 27239233    **
*-- BitWizard writes Linux device drivers for any device you may have! --*
Q: It doesn't work. A: Look buddy, doesn't work is an ambiguous statement. 
Does it sit on the couch all day? Is it unemployed? Please be specific! 
Define 'it' and what it isn't doing. --------- Adapted from lxrbot FAQ
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/