Re: [PATCH v3 13/13] irq: Optimize reschedule check in local_interrupt_enable()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Data race in `__preempt_count_add_return` causes lost wakeups on ARM64
--

commit c3bbe9e483f96a8dad6049a0192dc87491aed263
Author: Boqun Feng <boqun@kernel.org>

irq: Optimize reschedule check in local_interrupt_enable()

This patch optimizes local_interrupt_enable() by avoiding a re-read of
the preempt count. It modifies __preempt_count_{add,sub}_return() to
return unsigned long, allowing it to capture and return the
PREEMPT_NEED_RESCHED bit for reschedule checks.

> diff --git a/arch/arm64/include/asm/preempt.h b/arch/arm64/include/asm/preempt.h
> index 0dd8221d1bef7..e9f597d874134 100644
> --- a/arch/arm64/include/asm/preempt.h
> +++ b/arch/arm64/include/asm/preempt.h

[ ... ]

> @@ -55,20 +55,20 @@ static inline void __preempt_count_sub(int val)
>  	WRITE_ONCE(current_thread_info()->preempt.count, pc);
>  }
>  
> -static inline int __preempt_count_add_return(int val)
> +static inline unsigned long __preempt_count_add_return(int val)
>  {
> -	u32 pc = READ_ONCE(current_thread_info()->preempt.count);
> +	u64 pc = READ_ONCE(current_thread_info()->preempt_count);
>  	pc += val;
> -	WRITE_ONCE(current_thread_info()->preempt.count, pc);

[Severity: High]
Does this code introduce a regression where the need_resched flag can be
clobbered, resulting in a lost wakeup?

On ARM64, the 64-bit preempt_count union contains both the 32-bit count and
the 32-bit need_resched flag. Changing this to a non-atomic 64-bit read and
write seems like it might race with hardware interrupts.

For instance, when local_interrupt_disable() runs, it calls into
__preempt_count_add_return() via hardirq_disable_enter() before actually
disabling hardware interrupts:

include/linux/interrupt_rc.h:local_interrupt_disable() {
    ...
    new_count = hardirq_disable_enter();
    if (!new_count) {
        ...
        raw_local_irq_save(flags);
}

Because hardware interrupts are still fully enabled during this operation, if
an interrupt fires exactly between the 64-bit READ_ONCE() and WRITE_ONCE(),
and the handler wakes up a task (updating need_resched), wouldn't this
WRITE_ONCE() restore the stale need_resched value and lose the wakeup?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605054128.5925-1-boqun@kernel.org?part=13