Re: [PATCH bpf-next 5/8] selftests/bpf: Use CAN_USE_LOAD_ACQ_STORE_REL when appropriate

From: Pu Lehui <pulehui@huawei.com>
To: Peilin Ye <yepeilin@google.com>, <bpf@vger.kernel.org>
Cc: linux-riscv@lists.infradead.org,
	"Andrea Parri" <parri.andrea@gmail.com>,
	"Björn Töpel" <bjorn@kernel.org>,
	"Puranjay Mohan" <puranjay@kernel.org>,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Martin KaFai Lau" <martin.lau@linux.dev>,
	"Eduard Zingerman" <eddyz87@gmail.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	"Song Liu" <song@kernel.org>,
	"Yonghong Song" <yonghong.song@linux.dev>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"KP Singh" <kpsingh@kernel.org>,
	"Stanislav Fomichev" <sdf@fomichev.me>,
	"Hao Luo" <haoluo@google.com>, "Jiri Olsa" <jolsa@kernel.org>,
	"Luke Nelson" <luke.r.nels@gmail.com>,
	"Xi Wang" <xi.wang@gmail.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Mykola Lysenko" <mykolal@fb.com>,
	"Shuah Khan" <shuah@kernel.org>, "Josh Don" <joshdon@google.com>,
	"Barret Rhoden" <brho@google.com>,
	"Neel Natu" <neelnatu@google.com>,
	"Benjamin Segall" <bsegall@google.com>
Subject: Re: [PATCH bpf-next 5/8] selftests/bpf: Use CAN_USE_LOAD_ACQ_STORE_REL when appropriate
Date: Tue, 6 May 2025 22:22:49 +0800	[thread overview]
Message-ID: <6d9f8333-a14a-468a-83ef-4562e54eb60e@huawei.com> (raw)
In-Reply-To: <ca441a13f5ad4a34ddb622cb5b2616b309d59694.1745970908.git.yepeilin@google.com>


On 2025/4/30 8:51, Peilin Ye wrote:
> Instead of open-coding the conditions, use
> '#ifdef CAN_USE_LOAD_ACQ_STORE_REL' to guard the following tests:
> 
>    verifier_precision/bpf_load_acquire
>    verifier_precision/bpf_store_release
>    verifier_store_release/*
> 
> Note that, for the first two tests in verifier_precision.c, switching to
> '#ifdef CAN_USE_LOAD_ACQ_STORE_REL' means also checking if
> '__clang_major__ >= 18', which has already been guaranteed by the outer
> '#if' check.
> 
> Signed-off-by: Peilin Ye <yepeilin@google.com>
> ---
>   tools/testing/selftests/bpf/progs/verifier_precision.c     | 5 ++---
>   tools/testing/selftests/bpf/progs/verifier_store_release.c | 7 +++----
>   2 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/verifier_precision.c b/tools/testing/selftests/bpf/progs/verifier_precision.c
> index 6662d4b39969..2dd0d15c2678 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_precision.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_precision.c
> @@ -91,8 +91,7 @@ __naked int bpf_end_bswap(void)
>   		::: __clobber_all);
>   }
>   
> -#if defined(ENABLE_ATOMICS_TESTS) && \
> -	(defined(__TARGET_ARCH_arm64) || defined(__TARGET_ARCH_x86))
> +#ifdef CAN_USE_LOAD_ACQ_STORE_REL
>   
>   SEC("?raw_tp")
>   __success __log_level(2)
> @@ -138,7 +137,7 @@ __naked int bpf_store_release(void)
>   	: __clobber_all);
>   }
>   
> -#endif /* load-acquire, store-release */
> +#endif /* CAN_USE_LOAD_ACQ_STORE_REL */
>   #endif /* v4 instruction */
>   
>   SEC("?raw_tp")
> diff --git a/tools/testing/selftests/bpf/progs/verifier_store_release.c b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> index c0442d5bb049..7e456e2861b4 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_store_release.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_store_release.c
> @@ -6,8 +6,7 @@
>   #include "../../../include/linux/filter.h"
>   #include "bpf_misc.h"
>   
> -#if __clang_major__ >= 18 && defined(ENABLE_ATOMICS_TESTS) && \
> -	(defined(__TARGET_ARCH_arm64) || defined(__TARGET_ARCH_x86))
> +#ifdef CAN_USE_LOAD_ACQ_STORE_REL
>   
>   SEC("socket")
>   __description("store-release, 8-bit")
> @@ -271,7 +270,7 @@ __naked void store_release_with_invalid_reg(void)
>   	: __clobber_all);
>   }
>   
> -#else
> +#else /* CAN_USE_LOAD_ACQ_STORE_REL */
>   
>   SEC("socket")
>   __description("Clang version < 18, ENABLE_ATOMICS_TESTS not defined, and/or JIT doesn't support store-release, use a dummy test")
> @@ -281,6 +280,6 @@ int dummy_test(void)
>   	return 0;
>   }
>   
> -#endif
> +#endif /* CAN_USE_LOAD_ACQ_STORE_REL */
>   
>   char _license[] SEC("license") = "GPL";
Reviewed-by: Pu Lehui <pulehui@huawei.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv