linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Saket Kumar Bhaskar <skb99@linux.ibm.com>
To: puranjay@kernel.org
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	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>, 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>,
	Hari Bathini <hbathini@linux.ibm.com>,
	Naveen N Rao <naveen@kernel.org>, Mykola Lysenko <mykolal@fb.com>,
	Peilin Ye <yepeilin@google.com>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, "Paul E . McKenney" <paulmck@kernel.org>,
	lkmm@lists.linux.dev
Subject: Re: [PATCH RESEND bpf-next 1/1] powerpc64/bpf: Add jit support for load_acquire and store_release
Date: Thu, 24 Jul 2025 15:57:47 +0530	[thread overview]
Message-ID: <aIIKo39dK22ew1T5@linux.ibm.com> (raw)
In-Reply-To: <mb61pfreuy1rm.fsf@kernel.org>

On Thu, Jul 17, 2025 at 08:56:45PM +0000, puranjay@kernel.org wrote:
> Puranjay Mohan <puranjay@kernel.org> writes:
> 
> Somehow the cover letter for this patch was missed, adding it here:
> 
> To test the functionality of these special instructions, a tool called
> blitmus[0] was used to convert the following baseline litmus test[1] to bpf
> programs:
> 
>  C MP+poonceonces
> 
>  (*
>   * Result: Sometimes
>   *
>   * Can the counter-intuitive message-passing outcome be prevented with
>   * no ordering at all?
>   *)
> 
>  {}
> 
>  P0(int *buf, int *flag)
>  {
>          WRITE_ONCE(*buf, 1);
>          WRITE_ONCE(*flag, 1);
>  }
> 
>  P1(int *buf, int *flag)
>  {
>          int r0;
>          int r1;
> 
>          r0 = READ_ONCE(*flag);
>          r1 = READ_ONCE(*buf);
>  }
> 
>  exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> 
> Running the generated bpf program shows that the bad outcome is possible on
> powerpc:
> 
>  [fedora@linux-kernel blitmus]$ sudo ./mp_poonceonces
>  Starting litmus test with configuration:
>    Test: MP+poonceonces
>    Iterations: 4100
> 
>  Test MP+poonceonces Allowed
>  Histogram (4 states)
>  21548375 :>1:r0=0; 1:r1=0;
>  301187   :>1:r0=0; 1:r1=1;
>  337147   *>1:r0=1; 1:r1=0;
>  18813291 :>1:r0=1; 1:r1=1;
>  Ok
> 
>  Witnesses
>  Positive: 337147, Negative: 40662853
>  Condition exists (1:r0=1 /\ 1:r1=0) is validated
>  Observation MP+poonceonces Sometimes 337147 40662853
>  Time MP+poonceonces 13.48
> 
>  Thu Jul 17 18:12:51 UTC
> 
> Now the second write and the first read is converted to store_release and
> load_acquire and it gives us the following litmus test[2]
> 
>  C MP+pooncerelease+poacquireonce
> 
>  (*
>   * Result: Never
>   *
>   * This litmus test demonstrates that smp_store_release() and
>   * smp_load_acquire() provide sufficient ordering for the message-passing
>   * pattern.
>   *)
> 
>  {}
> 
>  P0(int *buf, int *flag)
>  {
>          WRITE_ONCE(*buf, 1);
>          smp_store_release(flag, 1);
>  }
> 
>  P1(int *buf, int *flag)
>  {
>          int r0;
>          int r1;
> 
>          r0 = smp_load_acquire(flag);
>          r1 = READ_ONCE(*buf);
>  }
> 
>  exists (1:r0=1 /\ 1:r1=0) (* Bad outcome. *)
> 
> 
> Running the generated bpf program shows that the bad outcome is *not* possible
> on powerpc with the implementation in this patch:
> 
>  [fedora@linux-kernel blitmus]$ sudo ./mp_pooncerelease_poacquireonce
>  Starting litmus test with configuration:
>    Test: MP+pooncerelease+poacquireonce
>    Iterations: 4100
> 
>  Test MP+pooncerelease+poacquireonce Allowed
>  Histogram (3 states)
>  21036021 :>1:r0=0; 1:r1=0;
>  14488694 :>1:r0=0; 1:r1=1;
>  5475285  :>1:r0=1; 1:r1=1;
>  No
> 
>  Witnesses
>  Positive: 0, Negative: 41000000
>  Condition exists (1:r0=1 /\ 1:r1=0) is NOT validated
>  Observation MP+pooncerelease+poacquireonce Never 0 41000000
>  Time MP+pooncerelease+poacquireonce 13.74
> 
>  Thu Jul 17 18:13:40 UTC
> 
> [0] https://github.com/puranjaymohan/blitmus
> [1] https://github.com/puranjaymohan/blitmus/blob/main/litmus_tests/MP%2Bpoonceonces.litmus
> [2] https://github.com/puranjaymohan/blitmus/blob/main/litmus_tests/MP%2Bpooncerelease%2Bpoacquireonce.litmus

Hi Puranjay,

Thanks for the patch. I applied the patch and tested it.

Before this patch:

# ./test_progs -a \
  verifier_load_acquire,verifier_store_release,atomics
#11/1    atomics/add:OK
#11/2    atomics/sub:OK
#11/3    atomics/and:OK
#11/4    atomics/or:OK
#11/5    atomics/xor:OK
#11/6    atomics/cmpxchg:OK
#11/7    atomics/xchg:OK
#11      atomics:OK
#528/1   verifier_load_acquire/Clang version < 18, ENABLE_ATOMICS_TESTS not defined, and/or JIT doesn't support load-acquire, use a dummy test:OK
#528     verifier_load_acquire:OK
#565/1   verifier_store_release/Clang version < 18, ENABLE_ATOMICS_TESTS not defined, and/or JIT doesn't support store-release, use a dummy test:OK
#565     verifier_store_release:OK
Summary: 3/9 PASSED, 0 SKIPPED, 0 FAILED

After this patch:

# ./test_progs -a \
  verifier_load_acquire,verifier_store_release,atomics
#11/1    atomics/add:OK
#11/2    atomics/sub:OK
#11/3    atomics/and:OK
#11/4    atomics/or:OK
#11/5    atomics/xor:OK
#11/6    atomics/cmpxchg:OK
#11/7    atomics/xchg:OK
#11      atomics:OK
#529/1   verifier_load_acquire/load-acquire, 8-bit:OK
#529/2   verifier_load_acquire/load-acquire, 8-bit @unpriv:OK
#529/3   verifier_load_acquire/load-acquire, 16-bit:OK
#529/4   verifier_load_acquire/load-acquire, 16-bit @unpriv:OK
#529/5   verifier_load_acquire/load-acquire, 32-bit:OK
#529/6   verifier_load_acquire/load-acquire, 32-bit @unpriv:OK
#529/7   verifier_load_acquire/load-acquire, 64-bit:OK
#529/8   verifier_load_acquire/load-acquire, 64-bit @unpriv:OK
#529/9   verifier_load_acquire/load-acquire with uninitialized src_reg:OK
#529/10  verifier_load_acquire/load-acquire with uninitialized src_reg @unpriv:OK
#529/11  verifier_load_acquire/load-acquire with non-pointer src_reg:OK
#529/12  verifier_load_acquire/load-acquire with non-pointer src_reg @unpriv:OK
#529/13  verifier_load_acquire/misaligned load-acquire:OK
#529/14  verifier_load_acquire/misaligned load-acquire @unpriv:OK
#529/15  verifier_load_acquire/load-acquire from ctx pointer:OK
#529/16  verifier_load_acquire/load-acquire from ctx pointer @unpriv:OK
#529/17  verifier_load_acquire/load-acquire with invalid register R15:OK
#529/18  verifier_load_acquire/load-acquire with invalid register R15 @unpriv:OK
#529/19  verifier_load_acquire/load-acquire from pkt pointer:OK
#529/20  verifier_load_acquire/load-acquire from flow_keys pointer:OK
#529/21  verifier_load_acquire/load-acquire from sock pointer:OK
#529     verifier_load_acquire:OK
#566/1   verifier_store_release/store-release, 8-bit:OK
#566/2   verifier_store_release/store-release, 8-bit @unpriv:OK
#566/3   verifier_store_release/store-release, 16-bit:OK
#566/4   verifier_store_release/store-release, 16-bit @unpriv:OK
#566/5   verifier_store_release/store-release, 32-bit:OK
#566/6   verifier_store_release/store-release, 32-bit @unpriv:OK
#566/7   verifier_store_release/store-release, 64-bit:OK
#566/8   verifier_store_release/store-release, 64-bit @unpriv:OK
#566/9   verifier_store_release/store-release with uninitialized src_reg:OK
#566/10  verifier_store_release/store-release with uninitialized src_reg @unpriv:OK
#566/11  verifier_store_release/store-release with uninitialized dst_reg:OK
#566/12  verifier_store_release/store-release with uninitialized dst_reg @unpriv:OK
#566/13  verifier_store_release/store-release with non-pointer dst_reg:OK
#566/14  verifier_store_release/store-release with non-pointer dst_reg @unpriv:OK
#566/15  verifier_store_release/misaligned store-release:OK
#566/16  verifier_store_release/misaligned store-release @unpriv:OK
#566/17  verifier_store_release/store-release to ctx pointer:OK
#566/18  verifier_store_release/store-release to ctx pointer @unpriv:OK
#566/19  verifier_store_release/store-release, leak pointer to stack:OK
#566/20  verifier_store_release/store-release, leak pointer to stack @unpriv:OK
#566/21  verifier_store_release/store-release, leak pointer to map:OK
#566/22  verifier_store_release/store-release, leak pointer to map @unpriv:OK
#566/23  verifier_store_release/store-release with invalid register R15:OK
#566/24  verifier_store_release/store-release with invalid register R15 @unpriv:OK
#566/25  verifier_store_release/store-release to pkt pointer:OK
#566/26  verifier_store_release/store-release to flow_keys pointer:OK
#566/27  verifier_store_release/store-release to sock pointer:OK
#566     verifier_store_release:OK
Summary: 3/55 PASSED, 0 SKIPPED, 0 FAILED

Tested-by: Saket Kumar Bhaskar <skb99@linux.ibm.com>

Regards,
Saket


  reply	other threads:[~2025-07-24 10:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20250717202935.29018-1-puranjay@kernel.org>
2025-07-17 20:29 ` [PATCH RESEND bpf-next 1/1] powerpc64/bpf: Add jit support for load_acquire and store_release Puranjay Mohan
2025-07-17 20:56   ` puranjay
2025-07-24 10:27     ` Saket Kumar Bhaskar [this message]
2025-07-27 17:29       ` Daniel Borkmann
2025-07-28  2:29         ` Madhavan Srinivasan
2025-07-24  8:50   ` Hari Bathini

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=aIIKo39dK22ew1T5@linux.ibm.com \
    --to=skb99@linux.ibm.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=hbathini@linux.ibm.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=lkmm@lists.linux.dev \
    --cc=maddy@linux.ibm.com \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=mpe@ellerman.id.au \
    --cc=mykolal@fb.com \
    --cc=naveen@kernel.org \
    --cc=npiggin@gmail.com \
    --cc=paulmck@kernel.org \
    --cc=puranjay@kernel.org \
    --cc=sdf@fomichev.me \
    --cc=song@kernel.org \
    --cc=yepeilin@google.com \
    --cc=yonghong.song@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;
as well as URLs for NNTP newsgroup(s).