Re: [PATCH v4 15/21] arm64: percpu: Implement preemptible read/write ops

[Vladimir Murzin <vladimir.murzin@arm.com>]

On 9/8/26 16:17, Mark Rutland wrote:
> Use the PCPU GPR infrastructure to implement preemptible this_cpu_read()
> and this_cpu_write().
> 
> This change means that this_cpu_read() will always use a plain LDR, even
> in LTO configurations where READ_ONCE() will use LDA[P]R. Using plain
> LDR is preferable, given that the rationale for using LDA[P]R in
> READ_ONCE() was to retain address dependencies against values written by
> other CPUs, which isn't expected usage for this_cpu_read(). Using plain
> LDR will enforce fewer ordering constraints.
> 
> Test case:
> 
> | void outline_this_cpu_write_u64(u64 __percpu *p, u64 v)
> | {
> | 	this_cpu_write(*p, v);
> | }
> 
> Generated code before this patch (v7.2-rc4):
> 
> | <outline_this_cpu_write_u64>:
> |        paciasp
> |        stp     x29, x30, [sp, #-16]!
> |        mrs     x2, sp_el0
> |        mov     x29, sp
> |        ldr     w3, [x2, #8]
> |        add     w3, w3, #0x1
> |        str     w3, [x2, #8]
> |        mrs     x3, tpidr_el1
> |        str     x1, [x0, x3]
> |        ldr     x0, [x2, #8]
> |        sub     x0, x0, #0x1
> |        str     w0, [x2, #8]
> |        cbz     x0, 1f
> |        ldr     x0, [x2, #8]
> |        cbnz    x0, 2f
> | 1:     bl      preempt_schedule_notrace
> | 2:     ldp     x29, x30, [sp], #16
> |        autiasp
> |        ret
> 
> Generated code after this patch:
> 
> | <outline_this_cpu_write_u64>:
> |        mrs     x2, sp_el0
> |        mov     w3, #0x7c60
> |        strh    w3, [x2, #20]
> |        mrs     x3, tpidr_el1
> |        str     x1, [x0, x3]
> |        strh    wzr, [x2, #20]
> |        ret
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Tested-by: Muhammad Usama Anjum <usama.anjum@arm.com>
> Cc: Ada Couprie Diaz <ada.coupriediaz@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Jinjie Ruan <ruanjinjie@huawei.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Vladimir Murzin <vladimir.murzin@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Yang Shi <yang@os.amperecomputing.com>
> ---
>  arch/arm64/include/asm/percpu.h | 79 ++++++++++++++++++++++++---------
>  1 file changed, 58 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/percpu.h b/arch/arm64/include/asm/percpu.h
> index 03fa916030fd1..984f69846cc09 100644
> --- a/arch/arm64/include/asm/percpu.h
> +++ b/arch/arm64/include/asm/percpu.h
> @@ -128,15 +128,42 @@ static inline unsigned long __kern_my_cpu_offset(void)
>  #define __my_cpu_offset __kern_my_cpu_offset()
>  #endif
>  
> -#define PERCPU_RW_OPS(sz)						\
> -static inline unsigned long __percpu_read_##sz(void *ptr)		\
> -{									\
> -	return READ_ONCE(*(u##sz *)ptr);				\
> -}									\
> -									\
> -static inline void __percpu_write_##sz(void *ptr, unsigned long val)	\
> -{									\
> -	WRITE_ONCE(*(u##sz *)ptr, (u##sz)val);				\
> +#define PERCPU_RW_OPS(w, sfx, sz)						\
> +static inline unsigned long __percpu_read_##sz(void __percpu *pcp)		\
> +{										\
> +	u16 *gprs = &current_thread_info()->pcpu_gprs;				\
> +	unsigned long off;							\
> +	unsigned long val;							\
> +										\
> +	asm volatile(								\
> +	__PCPU_GPRS_BEGIN_OFFSET("%[gprs]", "%[pcp]", "%[off]")			\
> +	"	ldr" #sfx "\t%" #w "[val], [%[pcp], %[off]]\n"			\
> +	__PCPU_GPRS_END("%[gprs]")						\
> +	: [gprs] "=Qo" (*gprs),							\
> +	  [off] "=&r" (off),							\
> +	  [val] "=&r" (val)							\
> +	: [pcp] "r" (pcp)							\
> +	: "memory"								\
> +	);									\
> +										\
> +	return val;								\
> +}										\
> +										\
> +static inline void __percpu_write_##sz(void __percpu *pcp, unsigned long val)	\
> +{										\
> +	u16 *gprs = &current_thread_info()->pcpu_gprs;				\
> +	unsigned long off;							\
> +										\
> +	asm volatile(								\
> +	__PCPU_GPRS_BEGIN_OFFSET("%[gprs]", "%[pcp]", "%[off]")			\
> +	"	str" #sfx "\t%" #w "[val], [%[pcp], %[off]]\n"			\
> +	__PCPU_GPRS_END("%[gprs]")						\
> +	: [gprs] "=Qo" (*gprs),							\
> +	  [off] "=&r" (off)							\
> +	: [pcp] "r" (pcp),							\
> +	  [val] "r" ((u##sz)val)						\
> +	: "memory"								\
> +	);									\
>  }
>  
>  #define __PERCPU_OP_CASE(w, sfx, name, sz, op_llsc, op_lse)		\
> @@ -196,10 +223,10 @@ __percpu_##name##_return_case_##sz(void *ptr, unsigned long val)	\
>  	__PERCPU_RET_OP_CASE(w,  , name, 32, op_llsc, op_lse)		\
>  	__PERCPU_RET_OP_CASE( ,  , name, 64, op_llsc, op_lse)
>  
> -PERCPU_RW_OPS(8)
> -PERCPU_RW_OPS(16)
> -PERCPU_RW_OPS(32)
> -PERCPU_RW_OPS(64)
> +PERCPU_RW_OPS(w, b, 8)
> +PERCPU_RW_OPS(w, h, 16)
> +PERCPU_RW_OPS(w,  , 32)
> +PERCPU_RW_OPS( ,  , 64)
>  
>  /*
>   * Use value-returning atomics for CPU-local ops as they are more likely
> @@ -245,23 +272,33 @@ PERCPU_RET_OP(add, add, ldadd)
>  	__retval;							\
>  })
>  
> +#define _pcp_wrap(op, pcp, ...)						\
> +({									\
> +	op(&(pcp), __VA_ARGS__);					\
> +})
> +
> +#define _pcp_wrap_return(op, pcp, args...)				\
> +({									\
> +	(typeof(pcp))op(&(pcp), ##args);				\
> +})
> +
>  #define this_cpu_read_1(pcp)		\
> -	_pcp_protect_return(__percpu_read_8, pcp)
> +	_pcp_wrap_return(__percpu_read_8, pcp)
>  #define this_cpu_read_2(pcp)		\
> -	_pcp_protect_return(__percpu_read_16, pcp)
> +	_pcp_wrap_return(__percpu_read_16, pcp)
>  #define this_cpu_read_4(pcp)		\
> -	_pcp_protect_return(__percpu_read_32, pcp)
> +	_pcp_wrap_return(__percpu_read_32, pcp)
>  #define this_cpu_read_8(pcp)		\
> -	_pcp_protect_return(__percpu_read_64, pcp)
> +	_pcp_wrap_return(__percpu_read_64, pcp)
>  
>  #define this_cpu_write_1(pcp, val)	\
> -	_pcp_protect(__percpu_write_8, pcp, (unsigned long)(val))
> +	_pcp_wrap(__percpu_write_8, pcp, (unsigned long)(val))
>  #define this_cpu_write_2(pcp, val)	\
> -	_pcp_protect(__percpu_write_16, pcp, (unsigned long)(val))
> +	_pcp_wrap(__percpu_write_16, pcp, (unsigned long)(val))
>  #define this_cpu_write_4(pcp, val)	\
> -	_pcp_protect(__percpu_write_32, pcp, (unsigned long)(val))
> +	_pcp_wrap(__percpu_write_32, pcp, (unsigned long)(val))
>  #define this_cpu_write_8(pcp, val)	\
> -	_pcp_protect(__percpu_write_64, pcp, (unsigned long)(val))
> +	_pcp_wrap(__percpu_write_64, pcp, (unsigned long)(val))
>  
>  #define this_cpu_add_1(pcp, val)	\
>  	_pcp_protect(__percpu_add_case_8, pcp, val)
> -- 2.30.2
> 

FWIW,

Reviewed-by: Vladimir Murzin <vladimir.murzin@arm.com>