Messages in this thread | | | From | David Brownell <> | Subject | Re: lockdep and threaded IRQs (was: ...) | Date | Fri, 27 Feb 2009 18:30:36 -0800 |
| |
On Friday 27 February 2009, Andrew Morton wrote: > On Fri, 27 Feb 2009 15:32:04 -0800 > Andrew Morton <akpm@linux-foundation.org> wrote: > > > Why does this function: > > > > static irqreturn_t powerbutton_irq(int irq, void *dev_id) > > { > > ... threaded irq handler body elided ... > > } > > > > Which is connected up via this statement: > > > > err = request_irq(irq, powerbutton_irq, > > IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, > > "twl4030-pwrbutton", NULL); > > > > reenable local interrupts?
Because threaded IRQ handlers are, well, threaded. And all the twl4030 IRQ handlers are threaded -- must be. But when CONFIG_LOCKDEP is enabled, it goofs such handlers ... as well as a bunch of other perfectly functional driver code.
In the absense of the lockdep IRQF_DISABLED goofage, the IRQs are properly dispatched -- IRQs stay enabled while these handlers run, all the relevant locking invariants are obeyed.
> ah, OK, twl4030_i2c_read_u8() does i2c I/O. > > Can't do that.
Threaded IRQ handlers *can* do that. That's the point.
Now, if you were to say "keep waiting a few more years until some threaded IRQ framework finally merges" ... the question comes up, "What to do in the meanwhile". (Ditto, "well, we've been waiting a long time now to see those threaded IRQs, what's up with them?")
"Nothing" is not an option. The "something" being done here is a reasonably clean approach, and doesn't call for any surgery to kernel/irq/* ... the *only* problem is the lockdep bug, which causes trouble for a variety of other drivers too.
> If some random process currently holds > mutex_lock(&twl->xfer_lock) and an interrupt occurs then this interrupt > handler will try to acquire mutex_lock(&twl->xfer_lock). Deadlock.
No, no, no. *THREADED IRQ HANDLER* at work here. Bzzt.
Threaded IRQ handler.
The relevant mutexes are *never* accessed outside of a thread context. Not by this code. Not by any other code.
| |