Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] x86/irq: lost interrupts in fixup_irqs() due to bypassed irq_retrigger --
commit a27ffeac3562f1a871ba38be5a47e5097ea4e9f4 Author: Naman Jain <[email protected]> x86/irq: Use irq_chip_retrigger_hierarchy() in fixup_irqs() fixup_irqs() re-injects a pending interrupt on its new target CPU by looking at the outermost domain chip and invoking its irq_retrigger() callback directly. This commit changes the implementation to use irq_chip_retrigger_hierarchy() instead, which walks up the interrupt hierarchy until it finds a chip that implements irq_retrigger(). It also changes desc->lock to use a scoped guard. > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > index 30122f0b3af96..acdc1145cce6d 100644 > --- a/arch/x86/kernel/irq.c > +++ b/arch/x86/kernel/irq.c > @@ -490,21 +490,19 @@ void fixup_irqs(void) > * nothing else will touch it. > */ > for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) { > - if (IS_ERR_OR_NULL(__this_cpu_read(vector_irq[vector]))) > + desc = __this_cpu_read(vector_irq[vector]); > + > + if (IS_ERR_OR_NULL(desc)) > continue; > > if (is_vector_pending(vector)) { > - desc = __this_cpu_read(vector_irq[vector]); > - > - raw_spin_lock(&desc->lock); > + guard(raw_spinlock)(&desc->lock); > data = irq_desc_get_irq_data(desc); > - chip = irq_data_get_irq_chip(data); > - if (chip->irq_retrigger) { > - chip->irq_retrigger(data); > + ret = irq_chip_retrigger_hierarchy(data); [Severity: High] Does this skip the outermost chip and potentially drop the interrupt? In fixup_irqs(), data is passed to irq_chip_retrigger_hierarchy(), which begins by immediately advancing to data->parent_data: kernel/irq/chip.c:irq_chip_retrigger_hierarchy() { for (data = data->parent_data; data; data = data->parent_data) if (data->chip && data->chip->irq_retrigger) return data->chip->irq_retrigger(data); return 0; } If the outermost chip (such as lapic_controller for legacy IRQs) implements irq_retrigger itself, the original irq_data is skipped and the loop will terminate immediately if data->parent_data is NULL. This can lead to hardware interrupts pending during CPU hot-unplug being silently dropped. It appears this logic error is not fixed by subsequent patches in the series either. > + if (ret) > __this_cpu_write(vector_irq[vector], > VECTOR_RETRIGGERED); > - } > - raw_spin_unlock(&desc->lock); > } > + -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
