From: Yonghong Song <yonghong.song@linux.dev>
To: bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
kernel-team@fb.com, Martin KaFai Lau <martin.lau@kernel.org>
Subject: [PATCH bpf-next v3 10/13] selftests/bpf: Remove unnecessary direct read of local percpu kptr
Date: Sun, 27 Aug 2023 08:28:21 -0700 [thread overview]
Message-ID: <20230827152821.2001129-1-yonghong.song@linux.dev> (raw)
In-Reply-To: <20230827152729.1995219-1-yonghong.song@linux.dev>
For the second argument of bpf_kptr_xchg(), if the reg type contains
MEM_ALLOC and MEM_PERCPU, which means a percpu allocation,
after bpf_kptr_xchg(), the argument is marked as MEM_RCU and MEM_PERCPU
if in rcu critical section. This way, re-reading from the map value
is not needed. Remove it from the percpu_alloc_array.c selftest.
Without previous kernel change, the test will fail like below:
0: R1=ctx(off=0,imm=0) R10=fp0
; int BPF_PROG(test_array_map_10, int a)
0: (b4) w1 = 0 ; R1_w=0
; int i, index = 0;
1: (63) *(u32 *)(r10 -4) = r1 ; R1_w=0 R10=fp0 fp-8=0000????
2: (bf) r2 = r10 ; R2_w=fp0 R10=fp0
;
3: (07) r2 += -4 ; R2_w=fp-4
; e = bpf_map_lookup_elem(&array, &index);
4: (18) r1 = 0xffff88810e771800 ; R1_w=map_ptr(off=0,ks=4,vs=16,imm=0)
6: (85) call bpf_map_lookup_elem#1 ; R0_w=map_value_or_null(id=1,off=0,ks=4,vs=16,imm=0)
7: (bf) r6 = r0 ; R0_w=map_value_or_null(id=1,off=0,ks=4,vs=16,imm=0) R6_w=map_value_or_null(id=1,off=0,ks=4,vs=16,imm=0)
; if (!e)
8: (15) if r6 == 0x0 goto pc+81 ; R6_w=map_value(off=0,ks=4,vs=16,imm=0)
; bpf_rcu_read_lock();
9: (85) call bpf_rcu_read_lock#87892 ;
; p = e->pc;
10: (bf) r7 = r6 ; R6=map_value(off=0,ks=4,vs=16,imm=0) R7_w=map_value(off=0,ks=4,vs=16,imm=0)
11: (07) r7 += 8 ; R7_w=map_value(off=8,ks=4,vs=16,imm=0)
12: (79) r6 = *(u64 *)(r6 +8) ; R6_w=percpu_rcu_ptr_or_null_val_t(id=2,off=0,imm=0)
; if (!p) {
13: (55) if r6 != 0x0 goto pc+13 ; R6_w=0
; p = bpf_percpu_obj_new(struct val_t);
14: (18) r1 = 0x12 ; R1_w=18
16: (b7) r2 = 0 ; R2_w=0
17: (85) call bpf_percpu_obj_new_impl#87883 ; R0_w=percpu_ptr_or_null_val_t(id=4,ref_obj_id=4,off=0,imm=0) refs=4
18: (bf) r6 = r0 ; R0=percpu_ptr_or_null_val_t(id=4,ref_obj_id=4,off=0,imm=0) R6=percpu_ptr_or_null_val_t(id=4,ref_obj_id=4,off=0,imm=0) refs=4
; if (!p)
19: (15) if r6 == 0x0 goto pc+69 ; R6=percpu_ptr_val_t(ref_obj_id=4,off=0,imm=0) refs=4
; p1 = bpf_kptr_xchg(&e->pc, p);
20: (bf) r1 = r7 ; R1_w=map_value(off=8,ks=4,vs=16,imm=0) R7=map_value(off=8,ks=4,vs=16,imm=0) refs=4
21: (bf) r2 = r6 ; R2_w=percpu_ptr_val_t(ref_obj_id=4,off=0,imm=0) R6=percpu_ptr_val_t(ref_obj_id=4,off=0,imm=0) refs=4
22: (85) call bpf_kptr_xchg#194 ; R0_w=percpu_ptr_or_null_val_t(id=6,ref_obj_id=6,off=0,imm=0) refs=6
; if (p1) {
23: (15) if r0 == 0x0 goto pc+3 ; R0_w=percpu_ptr_val_t(ref_obj_id=6,off=0,imm=0) refs=6
; bpf_percpu_obj_drop(p1);
24: (bf) r1 = r0 ; R0_w=percpu_ptr_val_t(ref_obj_id=6,off=0,imm=0) R1_w=percpu_ptr_val_t(ref_obj_id=6,off=0,imm=0) refs=6
25: (b7) r2 = 0 ; R2_w=0 refs=6
26: (85) call bpf_percpu_obj_drop_impl#87882 ;
; v = bpf_this_cpu_ptr(p);
27: (bf) r1 = r6 ; R1_w=scalar(id=7) R6=scalar(id=7)
28: (85) call bpf_this_cpu_ptr#154
R1 type=scalar expected=percpu_ptr_, percpu_rcu_ptr_, percpu_trusted_ptr_
The R1 which gets its value from R6 is a scalar. But before insn 22, R6 is
R6=percpu_ptr_val_t(ref_obj_id=4,off=0,imm=0)
Its type is changed to a scalar at insn 22 without previous patch.
Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
tools/testing/selftests/bpf/progs/percpu_alloc_array.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/percpu_alloc_array.c b/tools/testing/selftests/bpf/progs/percpu_alloc_array.c
index 3bd7d47870a9..bbc45346e006 100644
--- a/tools/testing/selftests/bpf/progs/percpu_alloc_array.c
+++ b/tools/testing/selftests/bpf/progs/percpu_alloc_array.c
@@ -146,10 +146,6 @@ int BPF_PROG(test_array_map_10)
/* race condition */
bpf_percpu_obj_drop(p1);
}
-
- p = e->pc;
- if (!p)
- goto out;
}
v = bpf_this_cpu_ptr(p);
--
2.34.1
next prev parent reply other threads:[~2023-08-27 15:28 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-27 15:27 [PATCH bpf-next v3 00/13] bpf: Add support for local percpu kptr Yonghong Song
2023-08-27 15:27 ` [PATCH bpf-next v3 01/13] bpf: Add support for non-fix-size percpu mem allocation Yonghong Song
2023-09-13 14:10 ` Hou Tao
2023-11-15 15:31 ` Heiko Carstens
2023-11-15 15:54 ` Alexei Starovoitov
2023-11-16 1:15 ` Hou Tao
2023-11-16 5:52 ` Yonghong Song
2023-11-16 13:54 ` Heiko Carstens
2023-08-27 15:27 ` [PATCH bpf-next v3 02/13] bpf: Add BPF_KPTR_PERCPU as a field type Yonghong Song
2023-08-27 15:27 ` [PATCH bpf-next v3 03/13] bpf: Add alloc/xchg/direct_access support for local percpu kptr Yonghong Song
2023-09-06 0:40 ` Alexei Starovoitov
2023-08-27 15:27 ` [PATCH bpf-next v3 04/13] bpf: Add bpf_this_cpu_ptr/bpf_per_cpu_ptr support for allocated percpu obj Yonghong Song
2023-08-27 15:27 ` [PATCH bpf-next v3 05/13] selftests/bpf: Update error message in negative linked_list test Yonghong Song
2023-08-27 15:28 ` [PATCH bpf-next v3 06/13] libbpf: Add __percpu_kptr macro definition Yonghong Song
2023-08-27 15:28 ` [PATCH bpf-next v3 07/13] selftests/bpf: Add bpf_percpu_obj_{new,drop}() macro in bpf_experimental.h Yonghong Song
2023-08-27 15:28 ` [PATCH bpf-next v3 08/13] selftests/bpf: Add tests for array map with local percpu kptr Yonghong Song
2023-08-27 15:28 ` [PATCH bpf-next v3 09/13] bpf: Mark OBJ_RELEASE argument as MEM_RCU when possible Yonghong Song
2023-09-06 0:37 ` Alexei Starovoitov
2023-08-27 15:28 ` Yonghong Song [this message]
2023-08-27 15:28 ` [PATCH bpf-next v3 11/13] selftests/bpf: Add tests for cgrp_local_storage with local percpu kptr Yonghong Song
2023-08-27 15:28 ` [PATCH bpf-next v3 12/13] selftests/bpf: Add some negative tests Yonghong Song
2023-08-27 15:28 ` [PATCH bpf-next v3 13/13] bpf: Mark BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE deprecated Yonghong Song
2023-09-06 0:50 ` [PATCH bpf-next v3 00/13] bpf: Add support for local percpu kptr patchwork-bot+netdevbpf
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=20230827152821.2001129-1-yonghong.song@linux.dev \
--to=yonghong.song@linux.dev \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
/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