[PATCH][RT]: handle_IRQ_event() - fix thinko

[PATCH][RT]: handle_IRQ_event() - fix thinko

[Sebastien Dugue]

  handle_IRQ_event() reenables interrupts for threaded IRQ handlers,
provided said handler does not need to run with IRQF_DISABLED.

  Therefore, unless I'm missing something, in the following:

if (!hardirq_count() || !(action->flags & IRQF_DISABLED))
	local_irq_enable();

  the logical OR should be a logical AND:

if (!hardirq_count() && !(action->flags & IRQF_DISABLED))
	local_irq_enable();

  Or maybe I've not yet reached my daily cafeine dose. (Damned addiction)

Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
---
 kernel/irq/handle.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 9ea9f73..f1579b9 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -144,7 +144,7 @@ irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction *action)
 	 * Unconditionally enable interrupts for threaded
 	 * IRQ handlers:
 	 */
-	if (!hardirq_count() || !(action->flags & IRQF_DISABLED))
+	if (!hardirq_count() && !(action->flags & IRQF_DISABLED))
 		local_irq_enable();
 
 	do {
-- 
1.6.0.1.308.gede4c

Re: [PATCH][RT]: handle_IRQ_event() - fix thinko

[Thomas Gleixner]

On Tue, 23 Sep 2008, Sebastien Dugue wrote:

> 
>   handle_IRQ_event() reenables interrupts for threaded IRQ handlers,
> provided said handler does not need to run with IRQF_DISABLED.
> 
>   Therefore, unless I'm missing something, in the following:
> 
> if (!hardirq_count() || !(action->flags & IRQF_DISABLED))
> 	local_irq_enable();
> 
>   the logical OR should be a logical AND:
> 
> if (!hardirq_count() && !(action->flags & IRQF_DISABLED))
> 	local_irq_enable();

No. We don't want to run the threaded handler with interrupts disabled.

!hardirq_count() tells us that we are in thread context.
 
>   Or maybe I've not yet reached my daily cafeine dose. (Damned addiction)

Looks like. :)

Thanks,

	tglx

Re: [PATCH][RT]: handle_IRQ_event() - fix thinko

[Sebastien Dugue]

On Wed, 24 Sep 2008 17:45:36 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote:

> On Tue, 23 Sep 2008, Sebastien Dugue wrote:
> 
> > 
> >   handle_IRQ_event() reenables interrupts for threaded IRQ handlers,
> > provided said handler does not need to run with IRQF_DISABLED.
> > 
> >   Therefore, unless I'm missing something, in the following:
> > 
> > if (!hardirq_count() || !(action->flags & IRQF_DISABLED))
> > 	local_irq_enable();
> > 
> >   the logical OR should be a logical AND:
> > 
> > if (!hardirq_count() && !(action->flags & IRQF_DISABLED))
> > 	local_irq_enable();
> 
> No. We don't want to run the threaded handler with interrupts disabled.
> 
> !hardirq_count() tells us that we are in thread context.

  OK, but then if a driver asked to run with IRQ disabled it will not
be honored with threaded hardirq.


  Sebastien.

Re: [PATCH][RT]: handle_IRQ_event() - fix thinko

[Thomas Gleixner]

On Thu, 25 Sep 2008, Sebastien Dugue wrote:
> On Wed, 24 Sep 2008 17:45:36 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote:
> 
> > On Tue, 23 Sep 2008, Sebastien Dugue wrote:
> > 
> > > 
> > >   handle_IRQ_event() reenables interrupts for threaded IRQ handlers,
> > > provided said handler does not need to run with IRQF_DISABLED.
> > > 
> > >   Therefore, unless I'm missing something, in the following:
> > > 
> > > if (!hardirq_count() || !(action->flags & IRQF_DISABLED))
> > > 	local_irq_enable();
> > > 
> > >   the logical OR should be a logical AND:
> > > 
> > > if (!hardirq_count() && !(action->flags & IRQF_DISABLED))
> > > 	local_irq_enable();
> > 
> > No. We don't want to run the threaded handler with interrupts disabled.
> > 
> > !hardirq_count() tells us that we are in thread context.
> 
>   OK, but then if a driver asked to run with IRQ disabled it will not
> be honored with threaded hardirq.

Err, no. We switch all interrupts (except timer) to threaded with
PREEMPT_RT=y. And we simply keep interrupts enabled across the thread
the same way as we ignore the interrupt disable/enable of the
spinlock_irqXX functions when we switch them to sleeping rt_mutex
based locks.

Thanks,

	tglx