Re: [RFC PATCH 1/4] genirq: Provide basic NMI management for interrupt lines

From: Thomas Gleixner
Date: Tue Jul 17 2018 - 05:22:30 EST


On Mon, 9 Jul 2018, Julien Thierry wrote:
> +/**
> + * disable_nmi_nosync - disable an nmi without waiting
> + * @irq: Interrupt to disable
> + *
> + * Disable the selected interrupt line. Disables and enables are
> + * nested.
> + * The interrupt to disable must have been requested through request_nmi.
> + * Unlike disable_nmi(), this function does not ensure existing
> + * instances of the IRQ handler have completed before returning.
> + */
> +void disable_nmi_nosync(unsigned int irq)
> +{
> + disable_irq_nosync(irq);
> +}
> +
> +/**
> + * disable_nmi - disable an nmi and wait for completion
> + * @irq: Interrupt to disable
> + *
> + * Disable the selected interrupt line. Enables and disables are
> + * nested.
> + * The interrupt to disable must have been requested through request_nmi.
> + * This function waits for any pending IRQ handlers for this interrupt
> + * to complete before returning. If you use this function while
> + * holding a resource the IRQ handler may need you will deadlock.

I don't see how that wait would work with NMIs.

> +/* This function must be called with desc->lock held */
> +static const void *__cleanup_nmi(unsigned int irq, struct irq_desc *desc)
> +{
> + const char *devname = NULL;
> +
> + desc->istate &= ~IRQS_NMI;
> +
> +#ifdef CONFIG_SMP
> + if (WARN_ON(desc->affinity_notify))
> + desc->affinity_notify = NULL;
> +#endif

We should not support that affinity notify horror for NMIs at all.

> + if (!WARN_ON(desc->action == NULL)) {
> + irq_pm_remove_action(desc, desc->action);
> + devname = desc->action->name;
> + unregister_handler_proc(irq, desc->action);
> +
> + kfree(desc->action);
> + desc->action = NULL;
> + }
> +
> + irq_settings_clr_disable_unlazy(desc);
> + irq_shutdown(desc);
> +
> +#ifdef CONFIG_SMP
> + /* make sure affinity_hint is cleaned up */
> + if (WARN_ON_ONCE(desc->affinity_hint))
> + desc->affinity_hint = NULL;
> +#endif

Especially not twice :)

Thanks,

tglx