Re: [GIT PULL] irq/core changes for v3.5

[Thomas Gleixner <tglx@linutronix.de>]

On Tue, 12 Jun 2012, Linus Torvalds wrote:

> On Tue, Jun 12, 2012 at 5:56 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > Thinking more about it, it's probably the best thing to simply force
> > the IRQF_ONESHOT flag if it's missing.
> 
> No, that's just crazy.
> 
> Now you make broken shit code work, and then you break the *correct*
> code that didn't want threading and didn't set IRQF_ONESHOT.

That correct code is totally unaffected. We only force set it for the
case which *requests* a threaded irq w/o a primary handler and w/o
supplying the ONESHOT flag.

> Just face it: if somebody doesn't have an interrupt-time function
> pointer, they need to rethink their code. It's a mistake. It's broken
> shit.

We explicitely made it that way to prevent people from implementing
the same function which just returns IRQ_WAKE_THREAD over and over.

This was done in the first place for irqs where the device which
raised the interrupt cannot be accessed from hard interrupt context
(stuff behind serial busses, i2c, spi ....)

The only choice those people had before we provided threaded
interrupts in the core was having a primary handler which called
disable_irq_nosync() and then woke an handling thread, which then
reenabled the irq line with enable_irq().

That allowed us to remove a lot of crappy, broken and racy kthread
code from drivers that way and consolidated the handling into the core
code.

Yes, in hindsight I should have enforced IRQF_ONESHOT for those
callers which have a NULL primary handler right from the beginning,
but that doesn't help much now.

> Why pander to crap? What is the advantage of allowing people to think
> that they don't need an interrupt-time function? It's a fundamentaly
> broken model, since it *cannot* work tgether with another driver that
> wants to have the normal semantics and happens to share the irq.

The vast majority of that stuff is embedded. Shared interupt lines are
almost non existent in that space. It's just x86 which is full of that
shared nonsense.

If it's shared between a normal semantics device and one which
requires the oneshot semantics, the core has already a sanity check
for those cases and always had.

If you really need a threaded handler which shares the interrupt line
with a non threaded one, then you definitely need a primary handler
which can disable the irq at the device level, but that was always
a requirement.

I just checked all the drivers in question with coccinelle.

Only a total of 8 drivers set IRQF_SHARED. 5 of them are for the same
ARM platform on which none of the irqs can be shared. So IRQF_SHARED
there is bogus. One other is for ARM stuff as well, which does not
have shared interrupts either. The two remaining are "general purpose"
drivers which also originated from embedded and are unlikely to show
up on a PC platform.

I did not bother to check the few ones which use platform data, but
those are definitely embedded ones as well, which won't have shared
interrupts either.

So now we have the choice of:

   - Leaving the current check and regress 90+ drivers

   - Leaving the current check and fix 90+ broken drivers

   - Reverting it and end up with no protection at all

   - Forcing the flag and risking the wreckage of two oddball drivers
     _IF_ they ever show up on a PC platform.

I definitely prefer option #4 as it makes the most sense.

Thanks,

	tglx