BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Maxim Khmelevskii" <max@linux.ibm.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next 3/3] s390/bpf: Enable atomics tests for s390
Date: Thu, 23 Jul 2026 14:12:25 +0000	[thread overview]
Message-ID: <20260723141226.4CB081F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260723140648.583055-8-max@linux.ibm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Meaningful code and commit message mismatch: failure to reuse CAN_USE_LOAD_ACQ_STORE_REL in arena_atomics.c.
--

--- Patch [3]: [PATCH bpf-next 3/3] s390/bpf: Enable atomics tests for s390 ---
commit c1e0ed2dde3c2d83049962c67b540464ea7a24d3
Author: Maxim Khmelevskii <max@linux.ibm.com>

s390/bpf: Enable atomics tests for s390

This commit adds s390 to the list of supported architectures for atomics
tests and updates CAN_USE_LOAD_ACQ_STORE_REL in bpf_misc.h.

> Reuse CAN_USE_LOAD_ACQ_STORE_REL in arena_atomics selftest,
> to remove code duplication.

[Severity: Medium]
Is this description accurate for the changes actually made in arena_atomics.c?

> diff --git a/tools/testing/selftests/bpf/progs/arena_atomics.c b/tools/testing/selftests/bpf/progs/arena_atomics.c
> index 2e7751a853999..73bc2b835f3fe 100644
> --- a/tools/testing/selftests/bpf/progs/arena_atomics.c
> +++ b/tools/testing/selftests/bpf/progs/arena_atomics.c
> @@ -28,8 +28,10 @@ bool skip_all_tests = true;
>  
>  #if defined(ENABLE_ATOMICS_TESTS) &&		  \
>  	defined(__BPF_FEATURE_ADDR_SPACE_CAST) && \
> -	(defined(__TARGET_ARCH_arm64) || defined(__TARGET_ARCH_x86) || \
> -	 (defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64))
> +	(defined(__TARGET_ARCH_arm64) || \
> +	 defined(__TARGET_ARCH_x86) || \
> +	 (defined(__TARGET_ARCH_riscv) && __riscv_xlen == 64) || \
> +	 defined(__TARGET_ARCH_s390))

[Severity: Medium]
Could this #if block use CAN_USE_LOAD_ACQ_STORE_REL as mentioned in the
commit message, instead of manually expanding the architecture list?

By not using the macro, it appears that powerpc and loongarch support
defined in bpf_misc.h are omitted in these checks. This manual expansion
also seems to be duplicated in load_acquire() and store_release() later
in the patch.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723140648.583055-5-max@linux.ibm.com?part=3

      reply	other threads:[~2026-07-23 14:12 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-23 14:03 [PATCH bpf-next 0/3] s390/bpf: Support load-acquire and store-release instructions Maxim Khmelevskii
2026-07-23 14:03 ` [PATCH bpf-next 1/3] s390/bpf: Add emit_ldx and emit_stx functions Maxim Khmelevskii
2026-07-23 15:08   ` bot+bpf-ci
2026-07-23 14:03 ` [PATCH bpf-next 2/3] s390/bpf: Support load-acquire and store-release instructions Maxim Khmelevskii
2026-07-23 14:03 ` [PATCH bpf-next 3/3] s390/bpf: Enable atomics tests for s390 Maxim Khmelevskii
2026-07-23 14:12   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260723141226.4CB081F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=max@linux.ibm.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox