From: David Brownell on
On Saturday 28 November 2009, Uwe Kleine-K�nig wrote:
> �I just thought that a
> warning that is triggered with request_irq and request_threaded_irq
> shouldn't be skipped when using setup_irq.

Seems fair to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: David Brownell on
On Saturday 28 November 2009, Thomas Gleixner wrote:
> What about analysing the code and verifying that the setup order is
> correct ?

Better to not care too much about setup order. Sometimes it's
unavoidable -- X initializes before Y "or else..." -- but the
patch I saw was just reporting goofage better.


> Adding save/restore_irq just because you have no clue what the code
> does is utter nonsense.

Absolutely. If that helps, find and fix the real bug.




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jamie Lokier on
Thomas Gleixner wrote:
> What about analysing the code and verifying that the setup order is
> correct ?
>
> Adding save/restore_irq just because you have no clue what the code
> does is utter nonsense.

Wouldn't it be quite a lot nicer if generic setup moved the
IRQF_DISABLED handler to be first in the list, if that actually works
in a useful way rather than simply being a quirk that irqs are
disabled for the first one?

-- Jamie
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jamie Lokier on
Uwe Kleine-K�nig wrote:
> Hello,
>
> On Sun, Nov 29, 2009 at 02:31:18AM +0000, Jamie Lokier wrote:
> > Thomas Gleixner wrote:
> > > What about analysing the code and verifying that the setup order is
> > > correct ?
> > >
> > > Adding save/restore_irq just because you have no clue what the code
> > > does is utter nonsense.
> >
> > Wouldn't it be quite a lot nicer if generic setup moved the
> > IRQF_DISABLED handler to be first in the list, if that actually works
> > in a useful way rather than simply being a quirk that irqs are
> > disabled for the first one?
> Hmm, what happens if an ISR runs with irqs disabled even though it
> doesn't expect it? I wouldn't bet that nothing breaks.

Moving the IRQF_DISABLED handler to be first will run an ISR with
interrupts disabled which *does* expect it, so that's good.

According to this thread, at the moment when you have multiple
IRQF_DISABLED|IRQF_SHARED ISRs, only the first one is run with
interrupts disabled.

In fact I don't see why the kernel cannot put _all_ of the
IRQ_DISABLED handlers at the beginning of the list, traverse those
with interrupts disabled, then enable interrupts them for the
remaining handlers.

> IMHO the best is if a warning is printed or registering fails if shared
> irq actions don't agree about wanting IRQF_DISABLED.

On the hardware in question, the debug and timer interrupts share a
line, and I'm guessing only the timer interrupt should have IRQF_DISABLED.

Or we could do away with this silliness and just switch everything to
threaded interrupts with RT-priorities ;-)

-- Jamie
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jamie Lokier on
Russell King - ARM Linux wrote:
> On Sun, Nov 29, 2009 at 03:18:40PM +0000, Jamie Lokier wrote:
> > Or we could do away with this silliness and just switch everything to
> > threaded interrupts with RT-priorities ;-)
>
> ... thereby needlessly increasing the latency of all interrupt handling
> and probably breaking some devices.

It's not that cut and dried. RT gives you priorities where you might
not have had them before, so in some cases can reduce worst-case
latency for critical devices. IRQF_DISABLED is a cheap two-level
priority scheme; RT threaded interrupts extend it.

The extra processing increases latency, yes, but that is in a sense a
scheduling fast-path problem; it's not _intrinsic_ to threaded
interrupts that they have to have higher latency (you don't have to
use the normal calculation or task switch to get equivalent
behaviour), but undoubtedly trying to optimise that leads to very
twisty code and state representations, and I don't see it happening in
Linux any time soon.

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