Re: [PATCH 02/24] alpha: enter hardirq context before looking up the irq descriptor

[Matt Turner]

On Thu, Sep 3, 2026 at 6:07 PM Magnus Lindholm <linmag7@xxxxxxxxx> wrote:
>
> Hi Matt,
>
> On Tue, Sep 1, 2026 at 5:50 PM Matt Turner <mattst88@xxxxxxxxx> wrote:
> >
> > handle_irq() called irq_to_desc() before irq_enter(), so that lookup ran
> > with the preempt count still saying task context and with RCU not yet
> > watching.  Generic code called from an interrupt handler should see
> > hardirq context, and irq_to_desc() is more than an array index once
> > SPARSE_IRQ is in use.
> >
> > Move irq_enter() to the top of the function and add the matching
> > irq_exit() to the invalid interrupt path.
> >
> > Assisted-by: Claude:claude-opus-5
> > Signed-off-by: Matt Turner <mattst88@xxxxxxxxx>
> > ---
> >  arch/alpha/kernel/irq.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git ./arch/alpha/kernel/irq.c ./arch/alpha/kernel/irq.c
> > index 4a6a8b1d5a8b..5867a1655045 100644
> > --- ./arch/alpha/kernel/irq.c
> > +++ ./arch/alpha/kernel/irq.c
> > @@ -107,18 +107,21 @@ handle_irq(int irq)
> >          * handled by some other CPU. (or is disabled)
> >          */
> >         static unsigned int illegal_count=0;
> > -       struct irq_desc *desc = irq_to_desc(irq);
> > -
> > +       struct irq_desc *desc;
> > +
> > +       irq_enter();
> > +
> > +       desc = irq_to_desc(irq);
> >         if (!desc || ((unsigned) irq > ACTUAL_NR_IRQS &&
> >             illegal_count < MAX_ILLEGAL_IRQS)) {
> >                 irq_err_count++;
> >                 illegal_count++;
> >                 printk(KERN_CRIT "device_interrupt: invalid interrupt %d\n",
> >                        irq);
> > +               irq_exit();
> >                 return;
> >         }
> >
> > -       irq_enter();
> >         generic_handle_irq_desc(desc);
> >         irq_exit();
> >  }
> > --
> > 2.54.0
> >
>
> Going over this code again, I noticed what may be redundant hardirq
> accounting in the existing device-interrupt path.
>
> For type 3 interrupts, do_entInt() already wraps
> alpha_mv.device_interrupt() in irq_enter()/irq_exit(). The platform
> dispatcher then calls handle_irq(), which enters and exits hardirq
> context again. Therefore, irq_to_desc() is already reached in hardirq
> context before this patch. The patch moves it from nesting level one
> to level two rather than from task context to hardirq context.
>
> The RTC path does not have the outer irq_enter(), so the change is
> needed there.
>
> Would it be cleaner for do_entInt() to own the hardirq context for
> both paths and remove the pair from handle_irq()? Or is there another
> caller which requires handle_irq() to manage it itself?
>
> I tested the patch on a two-CPU UP2000 with lockdep, IRQ flag tracing,
> and sparse IRQs enabled. SCSI and network interrupts operated normally,
> I observed no lockdep or IRQ-related warnings, and debug_locks remained
> enabled in /proc/lockdep_stats.

Hi Magnus,

Good question. Yes, do_entInt() should own irq_enter()/irq_exit() for
the RTC and device-interrupt cases too, same as it already does for
IPI, machine-check, and perf. RTC and device-interrupt are the only
two that don't, and only because handle_irq() does it internally.

There's a wrinkle. A bunch of device_interrupt() implementations call
handle_irq() in a loop over several pending vectors: sys_takara.c,
sys_eiger.c, sys_alcor.c, sys_dp264.c, sys_cabriolet.c,
sys_noritake.c, sys_mikasa.c, sys_rx164.c, irq_pyxis.c, irq_i8259.c.
Right now each of those handle_irq() calls enters and exits hardirq
context on its own, so one hardware interrupt that demuxes to N IRQs
does N enter/exit pairs. Moving the pair up to do_entInt() collapses
that to one per hardware interrupt, matching how the
IPI/machine-check/perf cases already work.

I don't see anything in the per-machine device_interrupt() bodies that
needs task context before the first handle_irq() call -- it's all
vector decode. But I've only got one Marvel box to boot-test on, so
I'm not comfortable folding the reshuffle into this patch without at
least reading through the other machine types more carefully.

I'll send it as a follow-up on top of the series rather than reworking
patch 2. Easier to drop if something turns out to care. Let me know if
you think that's wrong.

Thanks,
Matt