This patch series implements support for threaded irq handlers for the
generic IRQ layer.
Changes vs. V2:
- review comments addressed
- simplified design
- synchronize_irq adapted to threaded handlers
Threaded interrupt handlers are not only interesting in the preempt-rt
context. Threaded interrupt handlers can help to address common
problems in the interrupt handling code:
- move long running handlers out of the hard interrupt context
- avoid complex hardirq -> tasklet/softirq interaction and locking
problems by integration of this functionality into the threaded
handler code
- improved debugability of the kernel: faulty handlers do not take
down the system.
- allows prioritizing of the handlers which share an interrupt line
The implementation provides an opt-in mechanism to convert drivers to
the threaded interrupt handler model contrary to the preempt-rt patch
where the threaded handlers are enabled by a brute force switch. The
brute force switch is suboptimal as it does not change the interrupt
handler -> tasklet/softirq interaction problems, but was the only way
which was possible for the limited man power of the preempt-rt
developers.
Converting an interrupt to threaded makes only sense when the handler
code takes advantage of it by integrating tasklet/softirq
functionality and simplifying the locking.
When a driver wants to use threaded interrupt handlers it needs to
provide a separate thread function, which is called from the per
device thread created in irq_request_threaded(). The primary handler
still needs to be supplied. It checks whether the interrupt was
originated from the device or not.
In case it was originated from the device the primary handler must
disable the interrupt at the device level and return
IRQ_WAKE_THREAD. The generic interrupt handling core then sets the
IRQF_RUNTHREAD in the irqaction of the handler and wakes the
associated thread.
The irqaction is referenced in the threads task_struct. The reference
is used to prevent further referencing of the thread in the interrupt
code in the case of segfault to make sure that the system (minus the
now dead interrupt handler) survives and debug information can be
retrieved. In the best case the driver can be restarted, but dont
expect that as a given.
The code was tested with a converted USB EHCI driver. The EHCI shares
an interrupt line with another device and both the threaded and the
non threaded handlers coexist nicely.
Further the AHCI driver for ICH chipsets was converted and
tested. Interestingly enough this conversion did not cause worse
benchmark results with iobench and fio compared to the non threaded
mainline handler.
I'm still looking into a clean solution for the threaded demultiplex
handler case which was brought up by Dave to allow both the handling
of the demultiplexed devices in the context of the demultiplexer
interrupt thread and the wakeup of separate handler threads. But this
is an orthogonal extension of the existing patch set and does not
change the general design.
The patches apply on
git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git irq/threaded
The irq/threaded branch contains the generic irq related changes which are
already queued for .30
Thanks,
tglx
It still looks OK as a basic extension handling one
common model.
Once this is on track to merge (when?), I'd hope various
I2C and SPI device drivers would start to convert. The
most common example might be handling RTC alarms from the
numerous I2C and SPI based RTCs.
(The ones that don't need to manage an irq_chip across
that queued-message bus, and demux its IRQs. PMICs are
a bit more likely to need to demux lots of IRQs.)
On Monday 23 March 2009, Thomas Gleixner wrote:
> I'm still looking into a clean solution for the threaded demultiplex
> handler case which was brought up by Dave to allow both the handling
> of the demultiplexed devices in the context of the demultiplexer
> interrupt thread and the wakeup of separate handler threads. But this
> is an orthogonal extension of the existing patch set and does not
> change the general design.
No comments on the patch I sent?
http://marc.info/?l=linux-kernel&m=123734579014392&w=2
Or is that what you meant by "orthogonal"? Admittedly that
patch sort of begs the question about which request_irq()
variant should be used for such demuxed IRQs; the "current"
assumption is that request_irq() suffices, but that could
be improved so the handle_threaded_irq() flow handler could
use the action->thread_fn not action->handler.
- Dave
On Tue, 24 Mar 2009, David Brownell wrote:
> On Monday 23 March 2009, Thomas Gleixner wrote:
> > I'm still looking into a clean solution for the threaded demultiplex
> > handler case which was brought up by Dave to allow both the handling
> > of the demultiplexed devices in the context of the demultiplexer
> > interrupt thread and the wakeup of separate handler threads. But this
> > is an orthogonal extension of the existing patch set and does not
> > change the general design.
>
> No comments on the patch I sent?
Looked at it briefly, but I still try to figure out what the best
solution for this will be. As I said I'd like to support both
variants:
1) demux handlers run in the primary interrupt thread context
2) demux handlers kick their own handler threads
> Or is that what you meant by "orthogonal"? Admittedly that
> patch sort of begs the question about which request_irq()
> variant should be used for such demuxed IRQs; the "current"
> assumption is that request_irq() suffices, but that could
> be improved so the handle_threaded_irq() flow handler could
> use the action->thread_fn not action->handler.
I don't want to special case that. See above.
Thanks,
tglx
On Tuesday 24 March 2009, Thomas Gleixner wrote:
> On Tue, 24 Mar 2009, David Brownell wrote:
> > On Monday 23 March 2009, Thomas Gleixner wrote:
> > > I'm still looking into a clean solution for the threaded demultiplex
> > > handler case which was brought up by Dave to allow both the handling
> > > of the demultiplexed devices in the context of the demultiplexer
> > > interrupt thread and the wakeup of separate handler threads. But this
> > > is an orthogonal extension of the existing patch set and does not
> > > change the general design.
> >
> > No comments on the patch I sent?
>
> Looked at it briefly, but I still try to figure out what the best
> solution for this will be. As I said I'd like to support both
> variants:
>
> 1) demux handlers run in the primary interrupt thread context
> 2) demux handlers kick their own handler threads
I have no need for the latter, at least in current systems.
The IRQs are neither frequent nor time-critical, else they'd
not be managed via I2C in the first place ... even high speed
I2C (these mostly use 2.6 Mbit/sec, not 100 Kbit/sec) isn't
as quick as accessing any kind of local PIC.
The simplest case, FWIW, is with chips like the mcp23s08
GPIO expanders, or their I2C siblings: just one level of
dispatch needed. The Linux driver for those chips doesn't
yet support its IRQs, primarily because genirq doesn't cope
with such stuff very well yet. (You might want to think
about something a lot simpler than TWL4030-family PMICs!)
> > Or is that what you meant by "orthogonal"? Admittedly that
> > patch sort of begs the question about which request_irq()
> > variant should be used for such demuxed IRQs; the "current"
> > assumption is that request_irq() suffices, but that could
> > be improved so the handle_threaded_irq() flow handler could
> > use the action->thread_fn not action->handler.
>
> I don't want to special case that. See above.
What's a special case though? If you're serious about
wanting to support more than one case, it's always going
to be possible to call some of them "special". As in,
"threaded IRQs are a special case in genirq". That should
not mean they don't get handled.
- Dave
On Tue, 24 Mar 2009, David Brownell wrote:
> On Tuesday 24 March 2009, Thomas Gleixner wrote:
> > On Tue, 24 Mar 2009, David Brownell wrote:
> > > On Monday 23 March 2009, Thomas Gleixner wrote:
> > > > I'm still looking into a clean solution for the threaded demultiplex
> > > > handler case which was brought up by Dave to allow both the handling
> > > > of the demultiplexed devices in the context of the demultiplexer
> > > > interrupt thread and the wakeup of separate handler threads. But this
> > > > is an orthogonal extension of the existing patch set and does not
> > > > change the general design.
> > >
> > > No comments on the patch I sent?
> >
> > Looked at it briefly, but I still try to figure out what the best
> > solution for this will be. As I said I'd like to support both
> > variants:
> >
> > 1) demux handlers run in the primary interrupt thread context
> > 2) demux handlers kick their own handler threads
>
> I have no need for the latter, at least in current systems.
Groan, the fact that you do not need it is definitely _not_ a good
reason to just add a irq_is_sufficient_for_dave_handler.
> > I don't want to special case that. See above.
>
> What's a special case though? If you're serious about
> wanting to support more than one case, it's always going
> to be possible to call some of them "special". As in,
> "threaded IRQs are a special case in genirq". That should
> not mean they don't get handled.
I don't like the idea of another action dispatcher in a special case
handler. The goal is to reuse the code i.e. simple_handler and
handle_IRQ_event. It just needs some thoughts to implement it in a
sane way.
Thanks,
tglx
On Wednesday 25 March 2009, Thomas Gleixner wrote:
> > I have no need for the latter, at least in current systems.
>
> Groan, the fact that you do not need it is definitely _not_ a good
> reason to just add a irq_is_sufficient_for_dave_handler.
Groan ... don't *you* be puttin' words in *my* mouth.
When I've been in the position you're now in, I've
found it useful to know the actual requirements of
interface users. Without knowing that, it's kind of
hard to deliver a usable solution, n'est-ce pas?
I'm fairly certain you've seen systems that needlessly
pulled in complicating requirements, and thereby caused
trouble. If you can provide something to efficiently
address both modes, fine. But "the latter" seems like
one of those needless complicating additions, which
could easily slow progress.
> > > I don't want to special case that. See above.
> >
> > What's a special case though? If you're serious about
> > wanting to support more than one case, it's always going
> > to be possible to call some of them "special". As in,
> > "threaded IRQs are a special case in genirq". That should
> > not mean they don't get handled.
>
> I don't like the idea of another action dispatcher in a special case
> handler. The goal is to reuse the code i.e. simple_handler and
> handle_IRQ_event. It just needs some thoughts to implement it in a
> sane way.
You were the one to suggest a flow handler specifically
for cases like this, though ... you seem to have changed
your mind on this topic. ISTR the rationale was to get
past the current IRQF_DISABLED special casing found in
handle_IRQ_event(), by using a flow handler which didn't
call that routine.
- Dave