2009-03-05 19:59:27

by Sven-Thorsten Dietrich

[permalink] [raw]
Subject: Re: [patch 3/4] genirq: add a quick check handler

On Sat, 2009-02-28 at 17:24 -0500, Christoph Hellwig wrote:
> I really disagree with the notation of the pre-handler. Instead of
> adding an additional pre handler method you should add a new threadfn
> method. The handler could just as now handle/not handle the interrupt,
> or as a third option defer it to the thread. That makes the different
> semantics a lot clearer, and means ->handler and ->threadfn both have
> very well defined contexts, instead of sometimes calling ->handler
> sometimes from irq and sometimes from thread context. This also
> makes it much easier for complex hardware that might have simple and
> fast interrupts that it may want to handle directly from hardirq context
> in just a couple of cycles or complex interrupts that might be deferred
> to process context.
>

Most of the IRQ handler, whether run in a thread or IRQ context, will be
the same code - so what you are proposing would have to eliminate code
duplication as well as heavy runtime branching overhead.

Ultimately, no matter how its done, the concept of disabling IRQ assert
at the device level, rather than the apic level, is the optimal
"correct" implementation.

Formulating that into the code, as Thomas proposed with the quickcheck,
supplies structural demarcation for semi as well as software design.

Things just get quite ugly - as-is- when you try to add runtime thread
enable and disable.

Sven


2009-03-17 07:54:32

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [patch 3/4] genirq: add a quick check handler

On Thu, Mar 05, 2009 at 11:59:09AM -0800, Sven-Thorsten Dietrich wrote:
> Most of the IRQ handler, whether run in a thread or IRQ context, will be
> the same code - so what you are proposing would have to eliminate code
> duplication as well as heavy runtime branching overhead.
>
> Ultimately, no matter how its done, the concept of disabling IRQ assert
> at the device level, rather than the apic level, is the optimal
> "correct" implementation.
>
> Formulating that into the code, as Thomas proposed with the quickcheck,
> supplies structural demarcation for semi as well as software design.


Umm, the code will be look more or less the same either way. I just
think overloading the current handler to mean two different things is
a bad idea. For a driver using a quick disable handler and a long slow
threaded one the only difference is naming the two functions
differently.

I wonder if you're still thinking in the way of a -RT like setup where
threaded interrupts can be enabled and disabled globally? I don't think
we should ever do that for mainline.

2009-03-17 15:30:17

by Steven Rostedt

[permalink] [raw]
Subject: Re: [patch 3/4] genirq: add a quick check handler


On Tue, 17 Mar 2009, Christoph Hellwig wrote:
>
> I wonder if you're still thinking in the way of a -RT like setup where
> threaded interrupts can be enabled and disabled globally? I don't think
> we should ever do that for mainline.

I agree with this statement. You either want the interrupts as threads or
you don't. Honestly, I've never used the enable/disable threaded
interrupts feature in -rt except to test if it worked. But we recommend
against using it.

-- Steve