Re: [PATCH v8 2/3] riscv: introduce percpu.h into include/asm

On Fri, 03 Jul 2026, Yunhui Cui wrote:

>Add RISC-V specific this_cpu helpers so common percpu operations can use
>short architecture sequences instead of the generic implementation.
>Native-width operations use AMOs, while 8/16-bit operations use Zabha when
>available and a local 32-bit LR/SC fallback otherwise.

The subject and changelog could use some improvement. How about:

riscv: implement this_cpu operations

riscv has no asm/percpu.h, so every this_cpu_*() read-modify-write
takes the generic fallback, which disables local irqs.

A single AMO is atomic with respect to interrupts on the local hart,
so this can be replaced by one relaxed AMO under preemption disabled.
Native-width operations use AMOs, while 8/16-bit operations use Zabha when
available, or a local LR/SC sequence otherwise.

Some comments below, but I ran this on my sifive p550, feel free to add my:

Tested-by: Davidlohr Bueso <[email protected]>

>
>Signed-off-by: Yunhui Cui <[email protected]>
>---
> arch/riscv/include/asm/percpu.h | 296 ++++++++++++++++++++++++++++++++
> 1 file changed, 296 insertions(+)
> create mode 100644 arch/riscv/include/asm/percpu.h
>
>diff --git a/arch/riscv/include/asm/percpu.h b/arch/riscv/include/asm/percpu.h
>new file mode 100644
>index 0000000000000..76b1b8c1fb953
>--- /dev/null
>+++ b/arch/riscv/include/asm/percpu.h
>@@ -0,0 +1,296 @@
>+/* SPDX-License-Identifier: GPL-2.0-or-later */
>+
>+#ifndef __ASM_PERCPU_H
>+#define __ASM_PERCPU_H
>+
>+#include <linux/bits.h>
>+#include <linux/preempt.h>
>+
>+#include <asm/alternative-macros.h>
>+#include <asm/cmpxchg.h>
>+#include <asm/cpufeature-macros.h>
>+#include <asm/hwcap.h>
>+
>+#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);				\
>+}
>+
>+PERCPU_RW_OPS(8)
>+PERCPU_RW_OPS(16)
>+PERCPU_RW_OPS(32)
>+
>+#ifdef CONFIG_64BIT
>+PERCPU_RW_OPS(64)
>+#endif
>+
>+#define __PERCPU_AMO_OP_CASE(sfx, name, sz, amo_insn)			\
>+static inline void							\
>+__percpu_##name##_amo_case_##sz(void *ptr, unsigned long val)		\
>+{									\
>+	asm volatile (							\
>+		"amo" #amo_insn #sfx " zero, %[val], %[ptr]"		\

So afaict that rd=zero could steer the AMO away from the hart, no? From the
Zaamo chapter, it notes that complex implementations "might also implement
AMOs at memory controllers, and can optimize away fetching the original value
when the destination is x0". Similar for the others (PERCPU_8_16_OP).

Maybe use a tmp register instead and force the old value from L1 ("near"):

	u##sz tmp;
	asm volatile (
		"amo" #amo_insn #sfx " %[tmp], %[val], %[ptr]"
		: [ptr] "+A" (*(u##sz *)ptr), [tmp] "=r" (tmp)
		: [val] "r" ((u##sz)(val)));


... which arm64 now does for its pcpu ops:

	/*
	 * Use value-returning atomics for CPU-local ops as they are
	 * more likely to execute "near" to the CPU (e.g. in L1$).
	 *
	 * https://lore.kernel.org/r/e7d539ed-ced0-4b96-8ecd-048a5b803b85@paulmck-laptop
	 */

>+		: [ptr] "+A" (*(u##sz *)ptr)				\
>+		: [val] "r" ((u##sz)(val))				\
>+		: "memory");						\

Should not need this clobber either - these don't imply barriers (but you also get
them with the preemption disable/enable).

>+}
>+

...

>+#define this_cpu_cmpxchg_1(pcp, o, n)	_pcp_protect_return(cmpxchg_relaxed, pcp, o, n)

This triggers that cmpxchg variable shadowing issue you mentioned in patch 1 with
that __cpu_fallback_try_cmpxchg also using '__old'.

>+#define this_cpu_cmpxchg_2(pcp, o, n)	_pcp_protect_return(cmpxchg_relaxed, pcp, o, n)
>+#define this_cpu_cmpxchg_4(pcp, o, n)	_pcp_protect_return(cmpxchg_relaxed, pcp, o, n)
>+
>+#ifdef CONFIG_64BIT
>+#define this_cpu_cmpxchg_8(pcp, o, n)	_pcp_protect_return(cmpxchg_relaxed, pcp, o, n)
>+
>+#define this_cpu_cmpxchg64(pcp, o, n)	this_cpu_cmpxchg_8(pcp, o, n)
>+#endif
>+
>+#ifdef system_has_cmpxchg128
>+#define this_cpu_cmpxchg128(pcp, o, n)					\
>+({									\
>+	u128 ret__;							\
>+	typeof(pcp) *ptr__;						\
>+									\
>+	preempt_disable_notrace();					\
>+	ptr__ = raw_cpu_ptr(&(pcp));					\
>+	if (system_has_cmpxchg128())					\
>+		ret__ = cmpxchg128_local(ptr__, (o), (n));		\
>+	else								\
>+		ret__ = this_cpu_generic_cmpxchg(pcp, (o), (n));	\
>+	preempt_enable_notrace();					\
>+	ret__;								\
>+})

With those above this_cpu_cmpxchg() you could now enable HAVE_CMPXCHG_LOCAL.

Thanks,
Davidlohr

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv