linux-kernel - Re: Get rid of IRQF_DISABLED - (was [PATCH] genirq: warn about IRQF_SHARED|IRQF

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: <20091130143703.GA7028@n2100.arm.linux.org.uk>
Date:	Mon, 30 Nov 2009 14:37:03 +0000
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Rusty Russell <rusty@...tcorp.com.au>,
	David Brownell <dbrownell@...rs.sourceforge.net>,
	Eric Miao <eric.y.miao@...il.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	John Stultz <johnstul@...ibm.com>,
	Nicolas Pitre <nico@...vell.com>,
	Jamie Lokier <jamie@...reable.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Remy Bohmer <linux@...mer.net>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	linux-arm-kernel@...ts.infradead.org,
	Andrea Gallo <andrea.gallo@...ricsson.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: Get rid of IRQF_DISABLED - (was [PATCH] genirq: warn about
	IRQF_SHARED|IRQF_DISABLED)

On Mon, Nov 30, 2009 at 02:54:54PM +0100, Thomas Gleixner wrote:
> The correct solution IMNSHO is to get rid of IRQF_DISABLED and run
> interrupt handlers always with interrupts disabled and require them
> not to reenable interrupts themself.

I think Linus advocated at one point making the default be "irqs disabled"
and only only if a flag was passed would handlers run with IRQs enabled.
However, that might have been a step towards having all handlers running
with IRQs disabled.

I'm all in favour of it.  There are a large number of relatively simple
interrupt handlers out there which don't care about whether they're
called with IRQs disabled or not.

However, I think we still have a number of corner cases.  The SMC91x
driver comes to mind, with its stupidly small FIFOs, where the majority
of implementations have to have the packets loaded via PIO - and this
seems to generally happen from IRQ context.

The upshot of that is switching the SMC91x interrupt handler to run with
IRQs disabled will mean that serial can suffer with overruns, especially
if the serial port FIFO is also small.

The alternative is to push the "expensive" packet-loading parts of SMC91x
support into a tasklet, but that's probably going to impact performance
of the driver.

What I'm saying is that I think it's a good idea, but we should be
cautious about forcing a blanket change - to do so I believe risks creating
performance regressions.

Maybe a "safer" way forward is to go and find all those request_irq()
sites and add IRQF_DISABLED to them all, wait for regression reports and
selectively remove the IRQF_DISABLED flags?  We would then be able to
build up a picture of the problematical drivers that need to be reworked,
and whether the "run everything with irqs disabled" is even a practical
proposition.

Now, at the risk of covering old ground, how about we have two separate
irqaction lists, one for handlers to be called with irqs disabled and
one for handlers with irqs enabled.  We run the irqs-disabled list
first, naturally with irqs disabled.  If, at the end of that run (or
maybe after each handler), IRQs have ended being enabled, print some
diagnostics.  (We're going to need something like this to ensure that
drivers interrupt handlers don't enable IRQs themselves.)  Then enable
IRQs and run the irqs-enabled chain.
--
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/