Re: lockdep and threaded IRQs (was: ...)

[David Brownell <david-b@pacbell.net>]

On Friday 27 February 2009, Andrew Morton wrote:
> On Fri, 27 Feb 2009 20:46:50 -0800 David Brownell <david-b@pacbell.net> wrote:
> 
> >   drivers/mfd/twl4030-irq.c
> > 
> > Where you'll observe twl_init_irq() at line 688 setting
> > up the thread and the Primary IRQ Handler (PIH) dispatch.
> > That's pretty much bog-standard chained IRQ setup code,
> > except that it chains through a thread.
> 
> OK, that's clever.  I never knew that anyone was doing that.  afaict
> MFD is the only such place...

This is the Mark II code, which can look clever
because it uses genirq better than the fairly
ugly Mark I code (and doesn't clone the same
logic into three separate drivers).

What's unique about this particular code is that
it chains those IRQs, instead of creating its own
little irq core (like menelaus.c in that directory)
or only needing to handle one IRQ (as with RTC
alarm drivers).


> Yes, it's regrettable that it's a private-to-mfd implementation.

More like:  private-to-twl4030.  If a more generic
version (Mark III) comes along at some point, I'd
hope switching would be easy...


> I expect a lot of i2c clients (at least) would like this.

I2C, SPI, maybe some more ... yes.  RTC alarm handling,
as one fairly routine example.


> Perhaps it would be somewhat less dirty to do something like

Yeah, that #ifdef is pretty ugly.

Thomas sent some threaded IRQ patches around sometime
last fall, which handled the top level dispatch but
ISTR not the chaining part.  This kind of thing would
be appropriate for the chained IRQs.  Maybe the flag
would better be named IRQF_THREADED.

(However, this flavor hack wouldn't address the lockdep
breakage for drivers like the AT91 and OMAP1 MMC hosts.)

- Dave


> --- a/kernel/irq/manage.c~a
> +++ a/kernel/irq/manage.c
> @@ -689,7 +689,8 @@ int request_irq(unsigned int irq, irq_ha
>  	/*
>  	 * Lockdep wants atomic interrupt handlers:
>  	 */
> -	irqflags |= IRQF_DISABLED;
> +	if (!(irqflags & IRQF_NO_LOCKDEP_HACK))
> +		irqflags |= IRQF_DISABLED;
>  #endif
>  	/*
>  	 * Sanity-check: shared interrupts must pass in a real dev-ID,
>