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

[Thomas Gleixner <tglx@linutronix.de>]

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