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

[sashiko-bot@kernel.org]

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?

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