Re: [PATCH v3 03/13] preempt: Introduce __preempt_count_{sub, add}_return()

On Fri, Jun 05, 2026 at 06:30:14AM +0000, [email protected] wrote:
> >     preempt: Introduce __preempt_count_{sub, add}_return()
> >
> >     In order to use preempt_count() to tracking the interrupt disable
> >     nesting level, __preempt_count_{add,sub}_return() are introduced, as
> >     their name suggest, these primitives return the new value of the
> >     preempt_count() after changing it. The following example shows the usage
> >     of it in local_interrupt_disable():
> >
> >             // increase the HARDIRQ_DISABLE bit
> >             new_count = __preempt_count_add_return(HARDIRQ_DISABLE_OFFSET);
> >
> >             // if it's the first-time increment, then disable the interrupt
> >             // at hardware level.
> >             if (new_count & HARDIRQ_DISABLE_MASK == HARDIRQ_DISABLE_OFFSET) {
>                                                     ^^
> 
> The operator precedence in this example appears incorrect. In C, the equality
> operator == has higher precedence than the bitwise AND operator &, so this
> evaluates as:
> 
>     new_count & (HARDIRQ_DISABLE_MASK == HARDIRQ_DISABLE_OFFSET)
> 
> Since the mask does not equal the offset, the equality evaluates to 0, making
> the entire condition always false.
> 
> Should this be written with explicit parentheses to prevent readers from
> copying flawed logic into downstream implementations?
> 

Will fix this but this code is in commit log rather than real code.

Regards,
Boqun

>     if ((new_count & HARDIRQ_DISABLE_MASK) == HARDIRQ_DISABLE_OFFSET) {
> 
> This was raised by [email protected] in v3 review:
> https://lore.kernel.org/all/[email protected]/
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26998319662