Re: [PATCH bpf-next] libarena: Use compiler load-acquire/store-release in bpf_atomic.h

[Puranjay Mohan <puranjay@kernel.org>]

Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:

> Teach libarena's BPF atomic primitives to use compiler builtins for
> load-acquire and store-release when Clang advertises
> __BPF_FEATURE_LOAD_ACQ_STORE_REL. Older compilers continue to use the
> existing barrier-based fallback.
>
> Notably, as BPF programs begin running on arm64, it is better to use the
> more appropriate variants since we can no longer rely on x86 TSO ordering.
>
> Commit 880442305a39 ("bpf: Introduce load-acquire and store-release instructions")
> introduced support, hence kernels from 6.15 onwards are needed when
> compiling with compilers supporting these instructions. We have
> relatively recent kernel version requirements in libarena anyway, and
> have not cut first release, hence declare such a dependency.
>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  .../selftests/bpf/libarena/include/bpf_atomic.h  | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/libarena/include/bpf_atomic.h b/tools/testing/selftests/bpf/libarena/include/bpf_atomic.h
> index b7b230431929..ec8d606e7ce3 100644
> --- a/tools/testing/selftests/bpf/libarena/include/bpf_atomic.h
> +++ b/tools/testing/selftests/bpf/libarena/include/bpf_atomic.h
> @@ -86,6 +86,21 @@ extern bool CONFIG_X86_64 __kconfig __weak;
>  /* Control dependency provides LOAD->STORE, provide LOAD->LOAD */
>  #define smp_acquire__after_ctrl_dep() ({ smp_rmb(); })
>
> +#if defined(__BPF_FEATURE_LOAD_ACQ_STORE_REL)
> +#define __BPF_HAVE_LOAD_ACQUIRE_STORE_RELEASE 1
> +#endif
> +
> +#ifdef __BPF_HAVE_LOAD_ACQUIRE_STORE_RELEASE
> +/*
> + * Clang advertises this feature when it can lower acquire/release atomic
> + * builtins to BPF_LOAD_ACQ/BPF_STORE_REL. Older compilers keep using the
> + * barrier-based fallback below. The generated instructions require kernel
> + * verifier/JIT support added in Linux 6.15; compile for an older BPF CPU to
> + * keep using the fallback when targeting older kernels.
> + */
> +#define smp_load_acquire(p) __atomic_load_n((p), __ATOMIC_ACQUIRE)
> +#define smp_store_release(p, val) __atomic_store_n((p), (val), __ATOMIC_RELEASE)

__atomic_load_n(&some_struct, …) is a compile error. we should do the
more generic version:

#define smp_load_acquire(p)					\
	({							\
		__unqual_scalar_typeof(*p) __val;		\
		__atomic_load(p, &__val, __ATOMIC_ACQUIRE);	\
		__val;						\
	})

#define smp_store_release(p, v)					\
	({							\
		__unqual_scalar_typeof(*p) __val = (v);		\
		__atomic_store(p, &__val, __ATOMIC_RELEASE);	\
	})

Thanks,
Puranjay