On Mon, May 13, 2024 at 10:02:48AM GMT, Puranjay Mohan wrote:
> The Linux Kernel Memory Model [1][2] requires RMW operations that have a
> return value to be fully ordered.
>
> BPF atomic operations with BPF_FETCH (including BPF_XCHG and
> BPF_CMPXCHG) return a value back so they need to be JITed to fully
> ordered operations. POWERPC currently emits relaxed operations for
> these.
>
> We can show this by running the following litmus-test:
>
> PPC SB+atomic_add+fetch
>
> {
> 0:r0=x; (* dst reg assuming offset is 0 *)
> 0:r1=2; (* src reg *)
> 0:r2=1;
> 0:r4=y; (* P0 writes to this, P1 reads this *)
> 0:r5=z; (* P1 writes to this, P0 reads this *)
> 0:r6=0;
>
> 1:r2=1;
> 1:r4=y;
> 1:r5=z;
> }
>
> P0 | P1 ;
> stw r2, 0(r4) | stw r2,0(r5) ;
> | ;
> loop:lwarx r3, r6, r0 | ;
> mr r8, r3 | ;
> add r3, r3, r1 | sync ;
> stwcx. r3, r6, r0 | ;
> bne loop | ;
> mr r1, r8 | ;
> | ;
> lwa r7, 0(r5) | lwa r7,0(r4) ;
>
> ~exists(0:r7=0 /\ 1:r7=0)
>
> Witnesses
> Positive: 9 Negative: 3
> Condition ~exists (0:r7=0 /\ 1:r7=0)
> Observation SB+atomic_add+fetch Sometimes 3 9
>
> This test shows that the older store in P0 is reordered with a newer
> load to a different address. Although there is a RMW operation with
> fetch between them. Adding a sync before and after RMW fixes the issue:
>
> Witnesses
> Positive: 9 Negative: 0
> Condition ~exists (0:r7=0 /\ 1:r7=0)
> Observation SB+atomic_add+fetch Never 0 9
>
> [1] https://www.kernel.org/doc/Documentation/memory-barriers.txt
> [2] https://www.kernel.org/doc/Documentation/atomic_t.txt
>
> Fixes: 65112709115f ("powerpc/bpf/64: add support for BPF_ATOMIC bitwise
> operations")
As I noted in v2, I think that is the wrong commit. This fixes the below
four commits in mainline:
Fixes: aea7ef8a82c0 ("powerpc/bpf/32: add support for BPF_ATOMIC bitwise
operations")
Fixes: 2d9206b22743 ("powerpc/bpf/32: Add instructions for atomic_[cmp]xchg")
Fixes: dbe6e2456fb0 ("powerpc/bpf/64: add support for atomic fetch operations")
Fixes: 1e82dfaa7819 ("powerpc/bpf/64: Add instructions for atomic_[cmp]xchg")
> Signed-off-by: Puranjay Mohan <puran...@kernel.org>
> Acked-by: Paul E. McKenney <paul...@kernel.org>
Cc: sta...@vger.kernel.org # v6.0+
I have tested this with test_bpf and test_progs.
Reviewed-by: Naveen N Rao <nav...@kernel.org>
- Naveen
