Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] `irq_chip_retrigger_hierarchy()` skips the outermost interrupt chip, breaking retriggering for interrupts where the top-level chip directly implements `.irq_retrigger` (e.g., single-level domains). --
--- Patch [2]: [PATCH v2 2/3] x86/irq: Use irq_chip_retrigger_hierarchy() in fixup_irqs() --- commit 51de6b45bc61b20c750e5760473230c207ce5194 Author: Naman Jain <[email protected]> x86/irq: Use irq_chip_retrigger_hierarchy() in fixup_irqs() This commit updates fixup_irqs() to use a scoped guard for the irq_desc lock and replaces the direct invocation of the outermost chip's irq_retrigger() callback with irq_chip_retrigger_hierarchy(). > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > --- a/arch/x86/kernel/irq.c > +++ b/arch/x86/kernel/irq.c [ ... ] > @@ -489,22 +484,18 @@ void fixup_irqs(void) [ ... ] > if (is_vector_pending(vector)) { > - desc = __this_cpu_read(vector_irq[vector]); > - > - raw_spin_lock(&desc->lock); > - data = irq_desc_get_irq_data(desc); > - chip = irq_data_get_irq_chip(data); > - if (chip->irq_retrigger) { > - chip->irq_retrigger(data); > + guard(raw_spinlock)(&desc->lock); > + if (irq_chip_retrigger_hierarchy(&desc->irq_data)) [Severity: High] Does this skip the outermost interrupt chip for single-level domains? Looking at irq_chip_retrigger_hierarchy() in kernel/irq/chip.c, the loop initialization immediately advances to the parent data: int irq_chip_retrigger_hierarchy(struct irq_data *data) { 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; } For domains where the outermost chip directly implements the retrigger callback (such as legacy interrupts in x86_vector_domain), data->parent_data will be NULL or lack the retrigger logic. Will this cause the hierarchy walk to just return 0 and silently drop the pending interrupt during CPU hot-unplug? The generic try_retrigger() in kernel/irq/resend.c handles this by explicitly checking desc->irq_data.chip->irq_retrigger before falling back to the hierarchy walk. Should fixup_irqs() follow that same pattern to avoid skipping the top-level chip? > __this_cpu_write(vector_irq[vector], > VECTOR_RETRIGGERED); > - } > - raw_spin_unlock(&desc->lock); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
