Skip to content

Commit 387b532

Browse files
Martin KaFai LauAlexei Starovoitov
authored andcommitted
selftests/bpf: Tracing prog can still do lookup under busy lock
This patch modifies the task_ls_recursion test to check that the first bpf_task_storage_get(&map_a, ...) in BPF_PROG(on_update) can still do the lockless lookup even it cannot acquire the percpu busy lock. If the lookup succeeds, it will increment the value by 1 and the value in the task storage map_a will become 200+1=201. After that, BPF_PROG(on_update) tries to delete from map_a and should get -EBUSY because it cannot acquire the percpu busy lock after finding the data. Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org> Link: https://lore.kernel.org/r/20221025184524.3526117-10-martin.lau@linux.dev Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent 0334b4d commit 387b532

File tree

2 files changed

+86
-5
lines changed

2 files changed

+86
-5
lines changed

tools/testing/selftests/bpf/prog_tests/task_local_storage.c

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <sys/syscall.h> /* For SYS_xxx definitions */
99
#include <sys/types.h>
1010
#include <test_progs.h>
11+
#include "task_local_storage_helpers.h"
1112
#include "task_local_storage.skel.h"
1213
#include "task_local_storage_exit_creds.skel.h"
1314
#include "task_ls_recursion.skel.h"
@@ -78,21 +79,64 @@ static void test_exit_creds(void)
7879

7980
static void test_recursion(void)
8081
{
82+
int err, map_fd, prog_fd, task_fd;
8183
struct task_ls_recursion *skel;
82-
int err;
84+
struct bpf_prog_info info;
85+
__u32 info_len = sizeof(info);
86+
long value;
87+
88+
task_fd = sys_pidfd_open(getpid(), 0);
89+
if (!ASSERT_NEQ(task_fd, -1, "sys_pidfd_open"))
90+
return;
8391

8492
skel = task_ls_recursion__open_and_load();
8593
if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
86-
return;
94+
goto out;
8795

8896
err = task_ls_recursion__attach(skel);
8997
if (!ASSERT_OK(err, "skel_attach"))
9098
goto out;
9199

92100
/* trigger sys_enter, make sure it does not cause deadlock */
101+
skel->bss->test_pid = getpid();
93102
syscall(SYS_gettid);
103+
skel->bss->test_pid = 0;
104+
task_ls_recursion__detach(skel);
105+
106+
/* Refer to the comment in BPF_PROG(on_update) for
107+
* the explanation on the value 201 and 100.
108+
*/
109+
map_fd = bpf_map__fd(skel->maps.map_a);
110+
err = bpf_map_lookup_elem(map_fd, &task_fd, &value);
111+
ASSERT_OK(err, "lookup map_a");
112+
ASSERT_EQ(value, 201, "map_a value");
113+
ASSERT_EQ(skel->bss->nr_del_errs, 1, "bpf_task_storage_delete busy");
114+
115+
map_fd = bpf_map__fd(skel->maps.map_b);
116+
err = bpf_map_lookup_elem(map_fd, &task_fd, &value);
117+
ASSERT_OK(err, "lookup map_b");
118+
ASSERT_EQ(value, 100, "map_b value");
119+
120+
prog_fd = bpf_program__fd(skel->progs.on_lookup);
121+
memset(&info, 0, sizeof(info));
122+
err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
123+
ASSERT_OK(err, "get prog info");
124+
ASSERT_GT(info.recursion_misses, 0, "on_lookup prog recursion");
125+
126+
prog_fd = bpf_program__fd(skel->progs.on_update);
127+
memset(&info, 0, sizeof(info));
128+
err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
129+
ASSERT_OK(err, "get prog info");
130+
ASSERT_EQ(info.recursion_misses, 0, "on_update prog recursion");
131+
132+
prog_fd = bpf_program__fd(skel->progs.on_enter);
133+
memset(&info, 0, sizeof(info));
134+
err = bpf_obj_get_info_by_fd(prog_fd, &info, &info_len);
135+
ASSERT_OK(err, "get prog info");
136+
ASSERT_EQ(info.recursion_misses, 0, "on_enter prog recursion");
94137

95138
out:
139+
close(task_fd);
96140
task_ls_recursion__destroy(skel);
97141
}
98142

tools/testing/selftests/bpf/progs/task_ls_recursion.c

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@
55
#include <bpf/bpf_helpers.h>
66
#include <bpf/bpf_tracing.h>
77

8+
#ifndef EBUSY
9+
#define EBUSY 16
10+
#endif
11+
812
char _license[] SEC("license") = "GPL";
13+
int nr_del_errs = 0;
14+
int test_pid = 0;
915

1016
struct {
1117
__uint(type, BPF_MAP_TYPE_TASK_STORAGE);
@@ -26,6 +32,13 @@ int BPF_PROG(on_lookup)
2632
{
2733
struct task_struct *task = bpf_get_current_task_btf();
2834

35+
if (!test_pid || task->pid != test_pid)
36+
return 0;
37+
38+
/* The bpf_task_storage_delete will call
39+
* bpf_local_storage_lookup. The prog->active will
40+
* stop the recursion.
41+
*/
2942
bpf_task_storage_delete(&map_a, task);
3043
bpf_task_storage_delete(&map_b, task);
3144
return 0;
@@ -37,11 +50,32 @@ int BPF_PROG(on_update)
3750
struct task_struct *task = bpf_get_current_task_btf();
3851
long *ptr;
3952

53+
if (!test_pid || task->pid != test_pid)
54+
return 0;
55+
4056
ptr = bpf_task_storage_get(&map_a, task, 0,
4157
BPF_LOCAL_STORAGE_GET_F_CREATE);
42-
if (ptr)
58+
/* ptr will not be NULL when it is called from
59+
* the bpf_task_storage_get(&map_b,...F_CREATE) in
60+
* the BPF_PROG(on_enter) below. It is because
61+
* the value can be found in map_a and the kernel
62+
* does not need to acquire any spin_lock.
63+
*/
64+
if (ptr) {
65+
int err;
66+
4367
*ptr += 1;
68+
err = bpf_task_storage_delete(&map_a, task);
69+
if (err == -EBUSY)
70+
nr_del_errs++;
71+
}
4472

73+
/* This will still fail because map_b is empty and
74+
* this BPF_PROG(on_update) has failed to acquire
75+
* the percpu busy lock => meaning potential
76+
* deadlock is detected and it will fail to create
77+
* new storage.
78+
*/
4579
ptr = bpf_task_storage_get(&map_b, task, 0,
4680
BPF_LOCAL_STORAGE_GET_F_CREATE);
4781
if (ptr)
@@ -57,14 +91,17 @@ int BPF_PROG(on_enter, struct pt_regs *regs, long id)
5791
long *ptr;
5892

5993
task = bpf_get_current_task_btf();
94+
if (!test_pid || task->pid != test_pid)
95+
return 0;
96+
6097
ptr = bpf_task_storage_get(&map_a, task, 0,
6198
BPF_LOCAL_STORAGE_GET_F_CREATE);
62-
if (ptr)
99+
if (ptr && !*ptr)
63100
*ptr = 200;
64101

65102
ptr = bpf_task_storage_get(&map_b, task, 0,
66103
BPF_LOCAL_STORAGE_GET_F_CREATE);
67-
if (ptr)
104+
if (ptr && !*ptr)
68105
*ptr = 100;
69106
return 0;
70107
}

0 commit comments

Comments
 (0)