Re: [PATCH v2 02/20] arm64: percpu: Fix this_cpu_and() mask generation

[David Laight <david.laight.linux@gmail.com>]

On Wed, 5 Aug 2026 14:02:03 +0100
Mark Rutland <mark.rutland@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@arm.com> wrote:
> >   
> > > The arm64 implementation of this_cpu_and(pcp, val) is built in terms of
> > > ANDNOT operations, which requires the 'val' argument to be bitwise
> > > negated. The bitwise negation is not implemented correctly, with two
> > > bugs described below.
> > > 
> > > (1) The bitwise negation is performed as '~val' rather than '~(val)'.
> > >     This won't always generate the expected value when 'val' is an
> > >     expression.
> > > 
> > >     For example, for this_cpu_and(pcp, 1 - 1):
> > > 
> > >     * 'val'    is  '1 - 1'   ===> (int) 0x00000000
> > >     * '~val'   is '~1 - 1'   ===> (int) 0xfffffffd
> > >     * '~(val)' is '~(1 - 1)' ===> (int) 0xffffffff
> > > 
> > >     ... and thus bit[1] of 'pcp' would be preserved unexpectedly by the
> > >     ANDNOT operation.
> > > 
> > > (2) The bitwise negation is performed on 'val' before it has been cast
> > >     to (at least) the width of 'pcp'. This won't always generate the
> > >     expected value for the upper bits.
> > > 
> > >     For example, for this_cpu_and(pcp, zero), where 'pcp' is a u64 and
> > >     'zero' is a u32:
> > > 
> > >     * 'zero'           ===> (u32) 0x00000000
> > >     * '~(zero)'        ===> (u32) 0xffffffff
> > >     * '(u64)~(zero)'   ===> (u64) 0x00000000ffffffff
> > >     * '~((u64)(zero))' ===> (u64) 0xffffffffffffffff
> > > 
> > >     ... and thus bits[63:32] of 'pcp' would be preserved unexpectedly by
> > >     the ANDNOT operation.
> > > 
> > > Fix these issues by adding brackets around 'val', and by casting 'val'
> > > to an appropriately-sized type before bitwise negation.  
...
> > >  #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.

To my mind sign extension prior to and/or operations is almost
certainly unexpected.

	David