Re: [PATCH 15/25] genirq: Allow per-cpu interrupt sharing for non-overlapping affinities

[Marc Zyngier <maz@kernel.org>]

On Mon, 08 Sep 2025 17:31:17 +0100,
Marc Zyngier <maz@kernel.org> wrote:
> 
> Interrupt sharing for percpu-devid interrupts is forbidden, and
> for good reasons. These are interrupts generated *from* a CPU and
> handled by itself (timer, for example). Nobody in their right mind
> would put two devices on the same pin (and if they have, they get to
> keep the pieces...).
> 
> But this also prevents more benign cases, where devices are connected
> to groups of CPUs, and for which the affinities are not overlapping.
> Effectively, the only thing they share is the interrupt number, and
> nothing else.
> 
> Let's tweak the definition of IRQF_SHARED applied to percpu_devid
> interrupts to allow this particular case. This results in extra
> validation at the point of the interrupt being setup and freed,
> as well as a tiny bit of extra complexity for interrupts at handling
> time (to pick the correct irqaction).
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  kernel/irq/chip.c   |  8 ++++--
>  kernel/irq/manage.c | 67 +++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 61 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 0d0276378c707..af90dd440d5ee 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -897,8 +897,9 @@ void handle_percpu_irq(struct irq_desc *desc)
>  void handle_percpu_devid_irq(struct irq_desc *desc)
>  {
>  	struct irq_chip *chip = irq_desc_get_chip(desc);
> -	struct irqaction *action = desc->action;
>  	unsigned int irq = irq_desc_get_irq(desc);
> +	unsigned int cpu = smp_processor_id();
> +	struct irqaction *action;
>  	irqreturn_t res;
>  
>  	/*
> @@ -910,12 +911,15 @@ void handle_percpu_devid_irq(struct irq_desc *desc)
>  	if (chip->irq_ack)
>  		chip->irq_ack(&desc->irq_data);
>  
> +	for (action = desc->action; action; action = action->next)
> +		if (cpumask_test_cpu(cpu, action->affinity))
> +			break;
> +
>  	if (likely(action)) {
>  		trace_irq_handler_entry(irq, action);
>  		res = action->handler(irq, raw_cpu_ptr(action->percpu_dev_id));
>  		trace_irq_handler_exit(irq, action, res);
>  	} else {
> -		unsigned int cpu = smp_processor_id();
>  		bool enabled = cpumask_test_cpu(cpu, desc->percpu_enabled);
>  
>  		if (enabled)

As Will points out off the list, the above lacks the a similar
handling for percpu_devid NMIs, leading to NMIs that are only handled
on the first affinity group.

It's easy enough to move the above to common code and share it with
handle_percpu_devid_fasteoi_nmi(), but at this point there is hardly
any difference with handle_percpu_devid_irq().

Any objection to simply killing the NMI version?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.