>     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?

    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

Reply via email to