On 8/19/2026 9:29 PM, Michael Kelley wrote:
From: Naman Jain <[email protected]> Sent: Tuesday, August 18, 2026 
12:31 AM

On 8/18/2026 10:40 AM, Michael Kelley wrote:
From: Naman Jain <[email protected]> Sent: Monday, August 10, 2026 
2:07 AM

[snip]


Sashiko pointed out that irq_chip_retrigger_hierarchy() doesn't check
the outermost domain -- it immediately goes the parent. In v1 of this patch
series, you categorized this as a false positive. But I don't understand
your explanation. Even if there aren't currently any outermost domains
with a custom retrigger function, there could be at some point in the
future. So it seems wrong to skip it. But maybe I'm missing something.
Could you elaborate on your reasoning?

Michael


irq_chip_retrigger_hierarchy() deliberately skips the chip passed to it
and begins with its parent, i.e. apic_retrigger_irq(). The new code was
doing the same. However, if someone adds a custom .irq_retrigger, hoping
that it would get executed, it would not.

Sashiko's example was valid in principle, but it is not relevant to
these device interrupts because desc->irq_data is the outer
MSI/IOAPIC/Hyper-V chip. The LAPIC data is its parent, not the starting
data. I found no device IRQ in this path whose descriptor starts
directly at lapic_controller.

I am all in for adding this as a fallback -
+    if (chip->irq_retrigger)
+        ret = chip->irq_retrigger(data);
+    else
+        ret = irq_chip_retrigger_hierarchy(data);

This should solve this problem while still covering the chips which can
forget to add a retrigger function.

Please let me know if this looks good to you.

In principle, I think what you have is correct.  But there's a
problem in that irq_chip_retrigger_hierarchy() isn't defined
unless CONFIG_IRQ_DOMAIN_HIERARCHY=y.  try_retrigger()
is what you want, but it's static. You could add the #ifdef's here
like in try_retrigger(), but the same problem will occur in Patch 3
with msi_set_affinity(). Really need a non-static version of
try_retrigger(). Or maybe just doing the #ifdef's here and in
msi_set_affinity() is the simplest approach. I don't have a
strong opinion either way.  Sorry this is getting so messy ....

Michael

Hi Michael,
Config dependencies make sure that CONFIG_IRQ_DOMAIN_HIERARCHY is defined wherever irq_chip_retrigger_hierarchy() is getting called.

x86 SMP or x86-64 guarantees X86_LOCAL_APIC → IRQ_DOMAIN_HIERARCHY, PCI_MSI guarantees it via GENERIC_MSI_IRQ. So the #ifdef may not be required.

Hi Thomas,
Can you please comment if you are OK with me adding this fallback mechanism in next version.

Regards,
Naman

Reply via email to