"[email protected]" <[email protected]> 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 <[email protected]>
>
> --- 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
