On Tue, 17 Jul 2007 19:09:57 +0900
Fernando Luis V__zquez Cao <fernando at oss.ntt.co.jp> wrote:
> With the advent of kdump it is possible that device drivers receive
> interrupts generated in the context of a previous kernel. Ideally
> quiescing the underlying devices should suffice but not all drivers
> do this, either because it is not possible or because they did not
> contemplate this case. Thus drivers ought to be able to handle
> interrupts coming in as soon as the interrupt handler is registered.
>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando at oss.ntt.co.jp>
> ---
>
> diff -urNp linux-2.6.22-orig/kernel/irq/manage.c linux-2.6.22/kernel/irq/manage.c
> --- linux-2.6.22-orig/kernel/irq/manage.c 2007-07-09 08:32:17.000000000 +0900
> +++ linux-2.6.22/kernel/irq/manage.c 2007-07-17 18:37:24.000000000 +0900
> @@ -537,6 +537,29 @@ int request_irq(unsigned int irq, irq_ha
>
> select_smp_affinity(irq);
>
> +#if defined(CONFIG_DEBUG_PENDING_IRQ) || defined(CONFIG_DEBUG_SHIRQ)
> +#ifndef CONFIG_DEBUG_PENDING_IRQ
> + if (irqflags & IRQF_SHARED) {
> + /*
> + * It's a shared IRQ -- the driver ought to be prepared for it
> + * to happen immediately, so let's make sure....
> + * We do this before actually registering it, to make sure that
> + * a 'real' IRQ doesn't run in parallel with our fake.
> + */
> +#endif /* !CONFIG_DEBUG_PENDING_IRQ */
> + if (irqflags & IRQF_DISABLED) {
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + handler(irq, dev_id);
> + local_irq_restore(flags);
> + } else
> + handler(irq, dev_id);
> +#ifndef CONFIG_DEBUG_PENDING_IRQ
> + }
> +#endif /* !CONFIG_DEBUG_PENDING_IRQ */
> +#endif /* CONFIG_DEBUG_PENDING_IRQ || CONFIG_DEBUG_SHIRQ */
Even if we were going to merge this functionality as-is, I'd ask for some
sort of refactoring to fix up that ifdef maze.
But more substantial issues:
- This is presented as a "debug" feature, but it isn't a debug feature at
all - it is new functionality which is unrelated to kernel development.
Also, it is a "debug" feature which provides no debugging! At the very
least, one would expect to see it emit a printk to tell people that we
have some driver which needs fixing.
Also, this not-really-a-debug-feature is undesirably coupled with a
real debugging feature: CONFIG_DEBUG_PENDING_IRQ.
- Does this new feature really need its own Kconfig setting? Why not enable
it unconditionally? request_irq() isn't exactly performance-critical.
- If poss, we really do want to find some way of emitting a warning when
we detect such a device driver. Like, call the handler and if it
returned IRQ_HANDLED, start shouting.
