lkml.org 
[lkml]   [2006]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] IRQ: prevent enabling of previously disabled interrupt
"lgeek@frontiernet.net" <lgeek@frontiernet.net> wrote:
>
> Hi,
> This fix prevents re-disabling and enabling of a previously disabled
> interrupt in 2.6.16-rc5. On an SMP system with irq balancing enabled;
> If an interrupt is disabled from within its own interrupt context with
> disable_irq_nosync and is also earmarked for processor migration, the
> interrupt is blindly moved to the other processor and enabled without
> regard for its current "enabled" state. If there is an interrupt
> pending, it will unexpectedly invoke the irq handler on the new irq
> owning processor (even though the irq was previously disabled)
>
> The more intuitive fix would be to invoke disable_irq_nosync and
> enable_irq, but since we already have the desc->lock from __do_IRQ, we
> cannot call them directly. Instead we can use the same logic to
> disable and enable found in disable_irq_nosync and enable_irq, with
> regards to the desc->depth.
>
> This now prevents a disabled interrupt from being re-disabled, and
> more importantly prevents a disabled interrupt from being incorrectly
> enabled on a different processor.
>
> Signed-off-by: Bryan Holty <lgeek@frontiernet.net>
>
> --- 2.6.16-rc5/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -155,9 +155,13 @@
> * Being paranoid i guess!
> */
> if (unlikely(!cpus_empty(tmp))) {
> - desc->handler->disable(irq);
> + if (likely(!desc->depth++))
> + desc->handler->disable(irq);
> +
> desc->handler->set_affinity(irq,tmp);
> - desc->handler->enable(irq);
> +
> + if (likely(!--desc->depth))
> + desc->handler->enable(irq);
> }
> cpus_clear(pending_irq_cpumask[irq]);
> }

But desc->lock isn't held here. We need that for the update to ->depth (at
least).

And we can't take it here because one of the two ->end callers in __do_IRQ
already holds that lock. Possibly we should require that ->end callers
hold the lock, but that would incur considerable cost for cpu-local
interrupts.

So we'd need to require that ->end gets called outside the lock for
non-CPU-local interrupts. I'm not sure what the implications of that would
be - the ->end handlers don't need to be threaded at present and perhaps we
could put hardware into a bad state?

Or we add a new ->local_end, just for the CPU-local IRQs.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2006-03-07 13:00    [W:0.037 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site