Re: Get rid of IRQF_DISABLED - (was [PATCH] genirq: warn aboutIRQF_SHARED|IRQF_DISABLED)

From: Peter Zijlstra
Date: Mon Nov 30 2009 - 09:03:49 EST


On Mon, 2009-11-30 at 14:54 +0100, Thomas Gleixner wrote:
> On Mon, 30 Nov 2009, Uwe Kleine-KÃnig wrote:
> > For shared irqs IRQF_DISABLED is only guaranteed for the first handler.
> > So only warn starting at the second registration.
> >
> > The warning is moved to __setup_irq having the additional benefit of
> > catching actions registered using setup_irq not only register_irq.
> >
> > This doesn't fix the cases where setup order is wrong but it should
> > report the broken cases more reliably.
>
> The whole IRQF_DISABLED trickery is questionable and I'm pretty
> unhappy about the warning in general.
>
> While it is true that there is no guarantee of IRQF_DISABLED on shared
> interrupts (at least not for the secondary handlers) we really need to
> think about the reason why we want to run interrupt handlers with
> interrupts enabled at all.
>
> The separation of interrupt handlers which run with interrupts
> disabled/enabled goes all the way back to Linux 1.0, which had two
> interrupt handling modes:
>
> 1) handlers installed with SA_INTERRUPT ran atomically with interrupts
> disabled.
>
> 2) handlers installed without SA_INTERRUPT ran with interrupts enabled
> as they did more complex stuff like signal handling in the kernel.
>
> The interrupt which was always run with interrupts disabled was the
> timer interrupt because some of the "slower" interrupt handlers were
> relying on jiffies being updated, which is only possible when they run
> with interrupts enabled and no such handler can interrupt the timer
> interrupt.
>
> In the 2.1.x timeframe the discussion about shared interrupt handlers
> and the treatment of SA_INTERRUPT (today IRQF_DISABLED) was resolved
> by changing the code to what we have right now. If you read back in
> the archives you will find the same arguments as we have seen in this
> thread and a boatload of different solutions to that.
>
> The real question is why we want to run an interrupt handler with
> interrupts enabled at all. There are two reaons AFAICT:
>
> 1) interrupt handler relies on jiffies being updated:
>
> I don't think that this is the case anymore and if we still have
> code which does it is probably historic crap which is unused for
> quite a time.
>
> 2) interrupt handler runs a long time:
>
> I'm sure we still have some of those especially in the
> archaelogical corners of drivers/* and in the creative space of the
> embedded "oh, I don't know why but it works" departement. That's
> code which needs to be fixed anyway.
>
> 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.
>
> Thoughts ?

I'm all for removing that brain damage:

http://lkml.org/lkml/2009/3/2/33

We should convert the broken hardware PIO and 3com interrupt things to
threaded interrupts, and simply mandate all IRQ handlers run short and
with IRQs disabled.

Except I guess that will upset some of the IRQ priority folks, like
power, where they (iirc) have a stack per irq prio level.

But its not like the core kernel knows about these nesting rules and can
actually track any of that muck.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/