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

[Andrew Morton <akpm@linux-foundation.org>]

On Fri, 27 Feb 2009 15:18:57 -0800
David Brownell <david-b@pacbell.net> wrote:

> On Friday 27 February 2009, Andrew Morton wrote:
> > 
> > > There are actually two issues. __The lockdep issue is
> > > one ... the above #ifdef suffices to work around it
> > > for all the TWL4030 (family) IRQs.
> 
> (And other similar cases, of course, including those
> MMC cases that crap out in the ARM memory management
> code when lockdep wrongly disables IRQs.)
> 
> 
> > > The other is that Linux needs real support for threaded
> > > interrupts. __Almost every I2C (or SPI) device that raises
> > > an IRQ needs its IRQ handler to run in a thread, and most
> > > of them have the same type of workqueue-based hack to
> > > get such a thread. __(Some others have bugs instead...)
> > > 
> > > Obviously, if any threaded IRQ handler grabs a mutex,
> > > but lockdep has disabled IRQs, trouble ensues...
> > 
> > What's going on here? __hardirq handlers cannot take mutexes and cannot
> > sleep, period. __
> 
> But these handlers are *NOT* running in hardirq context;
> so such thought doesn't apply.

But you just told me that this code (which is an IRQ handler) needs to
sleep and take sleeping locks.

Let us start again.  Why does this function:

static irqreturn_t powerbutton_irq(int irq, void *dev_id)
{
	int err;
	u8 value;

#ifdef CONFIG_LOCKDEP
	/* WORKAROUND for lockdep forcing IRQF_DISABLED on us, which
	 * we don't want and can't tolerate.  Although it might be
	 * friendlier not to borrow this thread context...
	 */
	local_irq_enable();
#endif

	err = twl4030_i2c_read_u8(TWL4030_MODULE_PM_MASTER, &value,
				  STS_HW_CONDITIONS);
	if (!err)  {
		input_report_key(powerbutton_dev, KEY_POWER,
				 value & PWR_PWRON_IRQ);
	} else {
		dev_err(dbg_dev, "twl4030: i2c error %d while reading TWL4030"
			" PM_MASTER STS_HW_CONDITIONS register\n", err);
	}

	return IRQ_HANDLED;
}

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?