[PATCH v2 02/20] arm64: percpu: Fix this_cpu_and() mask generation
Mark Rutland
mark.rutland at arm.com
Thu Aug 6 03:23:57 PDT 2026
On Thu, Aug 06, 2026 at 09:28:15AM +0100, David Laight wrote:
> On Wed, 5 Aug 2026 14:02:03 +0100
> Mark Rutland <mark.rutland at arm.com> wrote:
> > On Wed, Aug 05, 2026 at 10:14:16AM +0100, David Laight wrote:
> > > On Tue, 4 Aug 2026 18:04:45 +0100
> > > Mark Rutland <mark.rutland at arm.com> wrote:
> > > > #define this_cpu_and_8(pcp, val) \
> > > > - _pcp_protect(__percpu_andnot_case_64, pcp, ~val)
> > > > + _pcp_protect(__percpu_andnot_case_64, pcp, ~(u64)(val))
> > >
> > > This one still isn't right.
> > > If val is a signed int with a negative value then it is sign extended
> > > before being inverted.
> > > val (int)0x80000000
> > > (u64)(val) 0xffffffff80000000
> > > ~(u64)(val) 0x000000007fffffff
> > > Something like ~(u64)((val) + 0u) will DTRT.
> >
> > As above, where have you got that idea from?
> >
> > AFAICT, a smaller signed type *should* be sign extended, and that must
> > happen before bitwise negation, since that bitwise negation is to cancel
> > out the NOT part of the ANDNOT operation.
> >
> > Think:
> >
> > 'pcp' is (u64) 0x0123456789abcdef
> > 'val' is (int) 0x800000000
> > '(u64)(val)' is (u64) 0xffffffff80000000
> > 'pcp & (u64)(val)' is (u64) 0x0123456780000000
> >
> > '~(u64)(val)' is (u64) 0x000000007fffffff
> > 'pcp ANDNOT ~(u64)(val)' is (u64) 0x0123456780000000
>
> The problem tends to arise with (u8)128 << 24 which is signed even
> though that is never intended.
Never intended by whom?
The expression '(u8)128 << 24' has int type. If an int is passed into a
u64 function parameter, it will be sign extended to 64 bits. If that's
passed into a binary expression against a u64, it will be sign extended.
That's the way __this_cpu_and() evaluates its argument too.
This is no different from calling any other function. If that caller
doesn't want the int value sign-extended to 64 bits to match the 64-bit
argument type, the caller needs to cast to an unsigned type.
See:
| void some_callee(u64 arg);
|
| void some_caller(u8 arg)
| {
| some_caller(arg << 24).
| }
|
| u64 some_inline_and(u8 arg)
| {
| u64 lhs = 0x0123456789abcdef;
| return lhs & (arg << 24);
| }
|
| void outline___this_cpu_and_expr(u64 __percpu *p)
| {
| __this_cpu_and(*p, (u8)128 << 24);
| }
Generated code:
| <some_caller>:
| paciasp
| stp x29, x30, [sp, #-16]!
| lsl w0, w0, #24 // left-shift 24
| mov x29, sp
| sxtw x0, w0 // sign-extend from 32-bit to 64-bit
| bl some_callee
| ldp x29, x30, [sp], #16
| autiasp
| ret
|
| <some_inline_and>:
| lsl w0, w0, #24 // left-shift 24
| mov x1, #0xcdef
| movk x1, #0x89ab, lsl #16
| sxtw x0, w0 // sign-extend from 32-bit to 64-bit
| movk x1, #0x4567, lsl #32
| movk x1, #0x123, lsl #48
| and x0, x0, x1
| ret
|
| <outline___this_cpu_and_expr>:
| mrs x2, tpidr_el1
| ldr x1, [x0, x2]
| and x1, x1, #0xffffffff80000000
| str x1, [x0, x2]
| ret
> To my mind sign extension prior to and/or operations is almost
> certainly unexpected.
As above, I disagree. I think you're starting from an incorrect
assumption about how these should work.
Having sign extension here is is entirely consistent with the usual way
type promotions are applied. It's the semantic of the __this_cpu_*()
operations that should match the this_cpu_*() operations.
Applying an arbitrarily different rule here doesn't help, and would be a
bug. I am not going to change this.
Mark.
More information about the linux-arm-kernel
mailing list