* Re: [PATCH] alpha: Fix arch_irqs_disabled_flags() to handle intermediate IPL levels
2026-04-03 15:01 [PATCH] alpha: Fix arch_irqs_disabled_flags() to handle intermediate IPL levels Matt Turner
@ 2026-04-04 8:26 ` Magnus Lindholm
0 siblings, 0 replies; 2+ messages in thread
From: Magnus Lindholm @ 2026-04-04 8:26 UTC (permalink / raw)
To: Matt Turner; +Cc: linux-alpha, linux-kernel
On Fri, Apr 3, 2026 at 5:01 PM Matt Turner <mattst88@gmail.com> wrote:
>
> arch_irqs_disabled_flags() only considered IRQs disabled when IPL was
> at IPL_MAX (7). However, PALcode enters interrupt handlers at
> intermediate IPL levels (e.g., IPL_DEV1=4, IPL_TIMER=5), which also
> have interrupts disabled at that level and below.
>
> This caused a lockdep warning when TRACE_IRQFLAGS_SUPPORT was enabled:
> lockdep_hardirqs_off() calls irqs_disabled() inside an interrupt
> handler and expects it to return true, but the intermediate IPL didn't
> match the IPL_MAX check.
>
> Fix by treating any IPL above IPL_MIN as interrupts disabled.
>
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Matt Turner <mattst88@gmail.com>
> ---
> arch/alpha/include/asm/irqflags.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git ./arch/alpha/include/asm/irqflags.h ./arch/alpha/include/asm/irqflags.h
> index 9f25d4e0d37e..80a01702b03a 100644
> --- ./arch/alpha/include/asm/irqflags.h
> +++ ./arch/alpha/include/asm/irqflags.h
> @@ -57,7 +57,7 @@ static inline void arch_local_irq_restore(unsigned long flags)
>
> static inline bool arch_irqs_disabled_flags(unsigned long flags)
> {
> - return flags == IPL_MAX;
> + return (flags & 7) > IPL_MIN;
> }
Hi, Matt
I'm not sure this change is correct.
On Alpha, the irqflags helpers are intended to operate on the
IPL field in the processor status (PS) register, not on the full
PS value. The low 3 bits of PS encode the current IPL, which is
why masking with `& 7` is appropriate when saving, restoring,
and checking interrupt flags.
However, changing arch_irqs_disabled_flags() from checking for
IPL_MAX to checking for any IPL > IPL_MIN changes the meaning
of the helper.
arch_irqs_disabled_flags() is used to answer whether local IRQs
are disabled in the Linux sense, i.e. whether all interrupt
levels are masked. On Alpha, that corresponds to IPL_MAX.
Intermediate IPL levels such as IPL_DEV1 or IPL_TIMER only mask
interrupts at and below that priority; higher-priority
interrupts can still be taken, so IRQs are not fully disabled.
So while masking flags down to the IPL bits is the right fix
when the saved flags may contain the full PS register,
broadening the predicate to `(flags & 7) > IPL_MIN` appears to
misreport partially masked interrupt state as "IRQs disabled".
I think the correct fix is to keep the PS/IPL cleanup, but
preserve the existing IPL_MAX semantics for
arch_irqs_disabled_flags().
Maybe we should consider something like this, (see below)
Regards Magnus
diff --git a/arch/alpha/include/asm/irqflags.h \
b/arch/alpha/include/asm/irqflags.h
index 9f25d4e0d37e..f207544f52de 100644
--- a/arch/alpha/include/asm/irqflags.h
+++ b/arch/alpha/include/asm/irqflags.h
@@ -26,7 +26,7 @@ extern int __min_ipl;
static inline unsigned long arch_local_save_flags(void)
{
- return rdps();
+ return getipl();
}
static inline void arch_local_irq_disable(void)
@@ -51,13 +51,13 @@ static inline void arch_local_irq_enable(void)
static inline void arch_local_irq_restore(unsigned long flags)
{
barrier();
- setipl(flags);
+ setipl(flags & 7);
barrier();
}
static inline bool arch_irqs_disabled_flags(unsigned long flags)
{
- return flags == IPL_MAX;
+ return (flags & 7) == IPL_MAX;
}
^ permalink raw reply related [flat|nested] 2+ messages in thread