From: Naman Jain <[email protected]> Sent: Wednesday, August 19, 2026 9:48 PM > > 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.
Indeed, I was just looking at this myself before I saw your reply. I think you are right. Michael > > Hi Thomas, > Can you please comment if you are OK with me adding this fallback > mechanism in next version. > > Regards, > Naman
