Re: [PATCH v3 05/13] irq & spin_lock: Add counted interrupt disabling/enabling

[Boqun Feng <boqun@kernel.org>]

On Fri, Jun 05, 2026 at 06:01:55AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] local_interrupt_enable() fails to invoke deferred softirqs and re-arm hrtimers, leading to indefinite stalls.
> --
> 
> commit de449b5be29a59d42df6ee590a2de395da7bbacc
> Author: Boqun Feng <boqun@kernel.org>
> 
> irq & spin_lock: Add counted interrupt disabling/enabling
> 
> This commit introduces a new set of interrupt disabling APIs:
> local_interrupt_disable() and local_interrupt_enable(). These save the
> interrupt state to a per-cpu variable and use a per-cpu counter to track
> nesting levels, preventing accidental enablement.
> 
> Link: https://patch.msgid.link/20260121223933.1568682-6-lyude@redhat.com
> 
> > diff --git a/include/linux/interrupt_rc.h b/include/linux/interrupt_rc.h
> > new file mode 100644
> > index 0000000000000..dd4444c613307
> > --- /dev/null
> > +++ b/include/linux/interrupt_rc.h
> 
> [ ... ]
> 
> > +static inline void local_interrupt_enable(void)
> > +{
> > +	int new_count;
> > +
> > +	new_count = hardirq_disable_exit();
> > +
> > +	if ((new_count & HARDIRQ_DISABLE_MASK) == 0) {
> > +		unsigned long flags;
> > +
> > +		flags = raw_cpu_read(local_interrupt_disable_state.flags);
> > +		local_irq_restore(flags);
> > +		/*
> > +		 * TODO: re-read preempt count can be avoided, but it needs
> > +		 * should_resched() taking another parameter as the current
> > +		 * preempt count
> > +		 */
> > +#ifdef CONFIG_PREEMPTION
> > +		if (should_resched(0))
> > +			__preempt_schedule();
> > +#endif
> > +	}
> > +}
> 
> [Severity: High]
> When the hardirq disable count drops to 0 here, does this miss checking for
> deferred softirqs?
> 

Yes, but it doesn't have any behavioral difference than a
local_irq_enable(), which won't trigger deferred softirqs either.

Regards,
Boqun

> Because __irq_exit_rcu() is updated later in this patch to skip invoking
> softirqs and re-arming hrtimers if hardirq_disable_count() is non-zero, the
> responsibility appears to shift to this function. Without a check for
> local_softirq_pending() when the count reaches 0, could softirqs and hrtimers
> deferred during the vulnerable window be indefinitely stalled until the next
> hardware interrupt fires?
> 
> > diff --git a/kernel/softirq.c b/kernel/softirq.c
> > index 10af5ed859e73..d1ab1799794ca 100644
> > --- a/kernel/softirq.c
> > +++ b/kernel/softirq.c
> 
> [ ... ]
> 
> > @@ -728,7 +731,16 @@ static inline void __irq_exit_rcu(void)
> >  #endif
> >  	account_hardirq_exit(current);
> >  	preempt_count_sub(HARDIRQ_OFFSET);
> > -	if (!in_interrupt() && local_softirq_pending()) {
> > +	/*
> > +	 * Interrupts may happen between hardirq_disable_enter() and
> > +	 * local_irq_save() in local_interrupt_disable(), if irq_exit() invokes
> > +	 * softirq here, we may have a softirq handler calling
> > +	 * local_interrupt_disable() but it won't disable the irq because
> > +	 * hardirq disabling count is already 1, hence we need to prevent
> > +	 * invoking softirq when a local_interrupt_disable() is ongoing.
> > +	 */
> > +	if (!in_interrupt() && !hardirq_disable_count() &&
> > +	    local_softirq_pending()) {
> >  		/*
> >  		 * If we left hrtimers unarmed, make sure to arm them now,
> >  		 * before enabling interrupts to run SoftIRQ.
> >  		 */
> > +		hrtimer_rearm_deferred();
> > +		invoke_softirq();
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260605054128.5925-1-boqun@kernel.org?part=5