* [PATCH bpf-next v2 1/4] bpf: Use this_cpu_{inc|dec|inc_return} for bpf_task_storage_busy
2022-09-01 6:19 [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety Hou Tao
@ 2022-09-01 6:19 ` Hou Tao
2022-09-01 6:19 ` [PATCH bpf-next v2 2/4] bpf: Use this_cpu_{inc_return|dec} for prog->active Hou Tao
` (3 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Hou Tao @ 2022-09-01 6:19 UTC (permalink / raw)
To: bpf
Cc: Song Liu, Sebastian Andrzej Siewior, Hao Sun, Hao Luo,
Andrii Nakryiko, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, KP Singh, David S . Miller,
Jakub Kicinski, Stanislav Fomichev, Jiri Olsa, John Fastabend,
Lorenz Bauer, houtao1
From: Hou Tao <houtao1@huawei.com>
Now migrate_disable() does not disable preemption and under some
architectures (e.g. arm64) __this_cpu_{inc|dec|inc_return} are neither
preemption-safe nor IRQ-safe, so for fully preemptible kernel concurrent
lookups or updates on the same task local storage and on the same CPU
may make bpf_task_storage_busy be imbalanced, and
bpf_task_storage_trylock() on the specific cpu will always fail.
Fixing it by using this_cpu_{inc|dec|inc_return} when manipulating
bpf_task_storage_busy.
Fixes: bc235cdb423a ("bpf: Prevent deadlock from recursive bpf_task_storage_[get|delete]")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
kernel/bpf/bpf_local_storage.c | 4 ++--
kernel/bpf/bpf_task_storage.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
index 4ee2e7286c23..802fc15b0d73 100644
--- a/kernel/bpf/bpf_local_storage.c
+++ b/kernel/bpf/bpf_local_storage.c
@@ -555,11 +555,11 @@ void bpf_local_storage_map_free(struct bpf_local_storage_map *smap,
struct bpf_local_storage_elem, map_node))) {
if (busy_counter) {
migrate_disable();
- __this_cpu_inc(*busy_counter);
+ this_cpu_inc(*busy_counter);
}
bpf_selem_unlink(selem, false);
if (busy_counter) {
- __this_cpu_dec(*busy_counter);
+ this_cpu_dec(*busy_counter);
migrate_enable();
}
cond_resched_rcu();
diff --git a/kernel/bpf/bpf_task_storage.c b/kernel/bpf/bpf_task_storage.c
index e9014dc62682..6f290623347e 100644
--- a/kernel/bpf/bpf_task_storage.c
+++ b/kernel/bpf/bpf_task_storage.c
@@ -26,20 +26,20 @@ static DEFINE_PER_CPU(int, bpf_task_storage_busy);
static void bpf_task_storage_lock(void)
{
migrate_disable();
- __this_cpu_inc(bpf_task_storage_busy);
+ this_cpu_inc(bpf_task_storage_busy);
}
static void bpf_task_storage_unlock(void)
{
- __this_cpu_dec(bpf_task_storage_busy);
+ this_cpu_dec(bpf_task_storage_busy);
migrate_enable();
}
static bool bpf_task_storage_trylock(void)
{
migrate_disable();
- if (unlikely(__this_cpu_inc_return(bpf_task_storage_busy) != 1)) {
- __this_cpu_dec(bpf_task_storage_busy);
+ if (unlikely(this_cpu_inc_return(bpf_task_storage_busy) != 1)) {
+ this_cpu_dec(bpf_task_storage_busy);
migrate_enable();
return false;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next v2 2/4] bpf: Use this_cpu_{inc_return|dec} for prog->active
2022-09-01 6:19 [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety Hou Tao
2022-09-01 6:19 ` [PATCH bpf-next v2 1/4] bpf: Use this_cpu_{inc|dec|inc_return} for bpf_task_storage_busy Hou Tao
@ 2022-09-01 6:19 ` Hou Tao
2022-09-01 6:19 ` [PATCH bpf-next v2 3/4] selftests/bpf: Move sys_pidfd_open() into task_local_storage_helpers.h Hou Tao
` (2 subsequent siblings)
4 siblings, 0 replies; 8+ messages in thread
From: Hou Tao @ 2022-09-01 6:19 UTC (permalink / raw)
To: bpf
Cc: Song Liu, Sebastian Andrzej Siewior, Hao Sun, Hao Luo,
Andrii Nakryiko, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, KP Singh, David S . Miller,
Jakub Kicinski, Stanislav Fomichev, Jiri Olsa, John Fastabend,
Lorenz Bauer, houtao1
From: Hou Tao <houtao1@huawei.com>
Both __this_cpu_inc_return() and __this_cpu_dec() are not preemption
safe and now migrate_disable() doesn't disable preemption, so the update
of prog-active is not atomic and in theory under fully preemptible kernel
recurisve prevention may do not work.
Fixing by using the preemption-safe and IRQ-safe variants.
Fixes: ca06f55b9002 ("bpf: Add per-program recursion prevention mechanism")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
kernel/bpf/trampoline.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index ff87e38af8a7..ad76940b02cc 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -895,7 +895,7 @@ u64 notrace __bpf_prog_enter(struct bpf_prog *prog, struct bpf_tramp_run_ctx *ru
run_ctx->saved_run_ctx = bpf_set_run_ctx(&run_ctx->run_ctx);
- if (unlikely(__this_cpu_inc_return(*(prog->active)) != 1)) {
+ if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
inc_misses_counter(prog);
return 0;
}
@@ -930,7 +930,7 @@ void notrace __bpf_prog_exit(struct bpf_prog *prog, u64 start, struct bpf_tramp_
bpf_reset_run_ctx(run_ctx->saved_run_ctx);
update_prog_stats(prog, start);
- __this_cpu_dec(*(prog->active));
+ this_cpu_dec(*(prog->active));
migrate_enable();
rcu_read_unlock();
}
@@ -966,7 +966,7 @@ u64 notrace __bpf_prog_enter_sleepable(struct bpf_prog *prog, struct bpf_tramp_r
migrate_disable();
might_fault();
- if (unlikely(__this_cpu_inc_return(*(prog->active)) != 1)) {
+ if (unlikely(this_cpu_inc_return(*(prog->active)) != 1)) {
inc_misses_counter(prog);
return 0;
}
@@ -982,7 +982,7 @@ void notrace __bpf_prog_exit_sleepable(struct bpf_prog *prog, u64 start,
bpf_reset_run_ctx(run_ctx->saved_run_ctx);
update_prog_stats(prog, start);
- __this_cpu_dec(*(prog->active));
+ this_cpu_dec(*(prog->active));
migrate_enable();
rcu_read_unlock_trace();
}
--
2.29.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next v2 3/4] selftests/bpf: Move sys_pidfd_open() into task_local_storage_helpers.h
2022-09-01 6:19 [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety Hou Tao
2022-09-01 6:19 ` [PATCH bpf-next v2 1/4] bpf: Use this_cpu_{inc|dec|inc_return} for bpf_task_storage_busy Hou Tao
2022-09-01 6:19 ` [PATCH bpf-next v2 2/4] bpf: Use this_cpu_{inc_return|dec} for prog->active Hou Tao
@ 2022-09-01 6:19 ` Hou Tao
2022-09-01 6:19 ` [PATCH bpf-next v2 4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy Hou Tao
2022-09-01 19:30 ` [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety patchwork-bot+netdevbpf
4 siblings, 0 replies; 8+ messages in thread
From: Hou Tao @ 2022-09-01 6:19 UTC (permalink / raw)
To: bpf
Cc: Song Liu, Sebastian Andrzej Siewior, Hao Sun, Hao Luo,
Andrii Nakryiko, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, KP Singh, David S . Miller,
Jakub Kicinski, Stanislav Fomichev, Jiri Olsa, John Fastabend,
Lorenz Bauer, houtao1
From: Hou Tao <houtao1@huawei.com>
sys_pidfd_open() is defined twice in both test_bprm_opts.c and
test_local_storage.c, so move it to a common header file. And it will be
used in map_tests as well.
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
.../selftests/bpf/prog_tests/test_bprm_opts.c | 10 +---------
.../bpf/prog_tests/test_local_storage.c | 10 +---------
.../selftests/bpf/task_local_storage_helpers.h | 18 ++++++++++++++++++
3 files changed, 20 insertions(+), 18 deletions(-)
create mode 100644 tools/testing/selftests/bpf/task_local_storage_helpers.h
diff --git a/tools/testing/selftests/bpf/prog_tests/test_bprm_opts.c b/tools/testing/selftests/bpf/prog_tests/test_bprm_opts.c
index 2559bb775762..a0054019e677 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_bprm_opts.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_bprm_opts.c
@@ -9,18 +9,10 @@
#include "bprm_opts.skel.h"
#include "network_helpers.h"
-
-#ifndef __NR_pidfd_open
-#define __NR_pidfd_open 434
-#endif
+#include "task_local_storage_helpers.h"
static const char * const bash_envp[] = { "TMPDIR=shouldnotbeset", NULL };
-static inline int sys_pidfd_open(pid_t pid, unsigned int flags)
-{
- return syscall(__NR_pidfd_open, pid, flags);
-}
-
static int update_storage(int map_fd, int secureexec)
{
int task_fd, ret = 0;
diff --git a/tools/testing/selftests/bpf/prog_tests/test_local_storage.c b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c
index 26ac26a88026..9c77cd6b1eaf 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_local_storage.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_local_storage.c
@@ -11,15 +11,7 @@
#include "local_storage.skel.h"
#include "network_helpers.h"
-
-#ifndef __NR_pidfd_open
-#define __NR_pidfd_open 434
-#endif
-
-static inline int sys_pidfd_open(pid_t pid, unsigned int flags)
-{
- return syscall(__NR_pidfd_open, pid, flags);
-}
+#include "task_local_storage_helpers.h"
static unsigned int duration;
diff --git a/tools/testing/selftests/bpf/task_local_storage_helpers.h b/tools/testing/selftests/bpf/task_local_storage_helpers.h
new file mode 100644
index 000000000000..711d5abb7d51
--- /dev/null
+++ b/tools/testing/selftests/bpf/task_local_storage_helpers.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TASK_LOCAL_STORAGE_HELPER_H
+#define __TASK_LOCAL_STORAGE_HELPER_H
+
+#include <unistd.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+
+#ifndef __NR_pidfd_open
+#define __NR_pidfd_open 434
+#endif
+
+static inline int sys_pidfd_open(pid_t pid, unsigned int flags)
+{
+ return syscall(__NR_pidfd_open, pid, flags);
+}
+
+#endif
--
2.29.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next v2 4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy
2022-09-01 6:19 [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety Hou Tao
` (2 preceding siblings ...)
2022-09-01 6:19 ` [PATCH bpf-next v2 3/4] selftests/bpf: Move sys_pidfd_open() into task_local_storage_helpers.h Hou Tao
@ 2022-09-01 6:19 ` Hou Tao
2022-09-01 19:37 ` Martin KaFai Lau
2022-09-01 19:30 ` [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety patchwork-bot+netdevbpf
4 siblings, 1 reply; 8+ messages in thread
From: Hou Tao @ 2022-09-01 6:19 UTC (permalink / raw)
To: bpf
Cc: Song Liu, Sebastian Andrzej Siewior, Hao Sun, Hao Luo,
Andrii Nakryiko, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, KP Singh, David S . Miller,
Jakub Kicinski, Stanislav Fomichev, Jiri Olsa, John Fastabend,
Lorenz Bauer, houtao1
From: Hou Tao <houtao1@huawei.com>
Under full preemptible kernel, task local storage lookup operations on
the same CPU may update per-cpu bpf_task_storage_busy concurrently. If
the update of bpf_task_storage_busy is not preemption safe, the final
value of bpf_task_storage_busy may become not-zero forever and
bpf_task_storage_trylock() will always fail. So add a test case to
ensure the update of bpf_task_storage_busy is preemption safe.
Will skip the test case when CONFIG_PREEMPT is disabled, and it can only
reproduce the problem probabilistically. By increasing
TASK_STORAGE_MAP_NR_LOOP and running it under ARM64 VM with 4-cpus, it
takes about four rounds to reproduce:
> test_maps is modified to only run test_task_storage_map_stress_lookup()
$ export TASK_STORAGE_MAP_NR_THREAD=256
$ export TASK_STORAGE_MAP_NR_LOOP=81920
$ export TASK_STORAGE_MAP_PIN_CPU=1
$ time ./test_maps
test_task_storage_map_stress_lookup(135):FAIL:bad bpf_task_storage_busy got -2
real 0m24.743s
user 0m6.772s
sys 0m17.966s
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
.../bpf/map_tests/task_storage_map.c | 122 ++++++++++++++++++
.../bpf/progs/read_bpf_task_storage_busy.c | 39 ++++++
2 files changed, 161 insertions(+)
create mode 100644 tools/testing/selftests/bpf/map_tests/task_storage_map.c
create mode 100644 tools/testing/selftests/bpf/progs/read_bpf_task_storage_busy.c
diff --git a/tools/testing/selftests/bpf/map_tests/task_storage_map.c b/tools/testing/selftests/bpf/map_tests/task_storage_map.c
new file mode 100644
index 000000000000..1adc9c292eb2
--- /dev/null
+++ b/tools/testing/selftests/bpf/map_tests/task_storage_map.c
@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022. Huawei Technologies Co., Ltd */
+#define _GNU_SOURCE
+#include <sched.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <errno.h>
+#include <string.h>
+#include <pthread.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#include "test_maps.h"
+#include "task_local_storage_helpers.h"
+#include "read_bpf_task_storage_busy.skel.h"
+
+struct lookup_ctx {
+ bool start;
+ bool stop;
+ int pid_fd;
+ int map_fd;
+ int loop;
+};
+
+static void *lookup_fn(void *arg)
+{
+ struct lookup_ctx *ctx = arg;
+ long value;
+ int i = 0;
+
+ while (!ctx->start)
+ usleep(1);
+
+ while (!ctx->stop && i++ < ctx->loop)
+ bpf_map_lookup_elem(ctx->map_fd, &ctx->pid_fd, &value);
+ return NULL;
+}
+
+static void abort_lookup(struct lookup_ctx *ctx, pthread_t *tids, unsigned int nr)
+{
+ unsigned int i;
+
+ ctx->stop = true;
+ ctx->start = true;
+ for (i = 0; i < nr; i++)
+ pthread_join(tids[i], NULL);
+}
+
+void test_task_storage_map_stress_lookup(void)
+{
+#define MAX_NR_THREAD 4096
+ unsigned int i, nr = 256, loop = 8192, cpu = 0;
+ struct read_bpf_task_storage_busy *skel;
+ pthread_t tids[MAX_NR_THREAD];
+ struct lookup_ctx ctx;
+ cpu_set_t old, new;
+ const char *cfg;
+ int err;
+
+ cfg = getenv("TASK_STORAGE_MAP_NR_THREAD");
+ if (cfg) {
+ nr = atoi(cfg);
+ if (nr > MAX_NR_THREAD)
+ nr = MAX_NR_THREAD;
+ }
+ cfg = getenv("TASK_STORAGE_MAP_NR_LOOP");
+ if (cfg)
+ loop = atoi(cfg);
+ cfg = getenv("TASK_STORAGE_MAP_PIN_CPU");
+ if (cfg)
+ cpu = atoi(cfg);
+
+ skel = read_bpf_task_storage_busy__open_and_load();
+ err = libbpf_get_error(skel);
+ CHECK(err, "open_and_load", "error %d\n", err);
+
+ /* Only for a fully preemptible kernel */
+ if (!skel->kconfig->CONFIG_PREEMPT)
+ return;
+
+ /* Save the old affinity setting */
+ sched_getaffinity(getpid(), sizeof(old), &old);
+
+ /* Pinned on a specific CPU */
+ CPU_ZERO(&new);
+ CPU_SET(cpu, &new);
+ sched_setaffinity(getpid(), sizeof(new), &new);
+
+ ctx.start = false;
+ ctx.stop = false;
+ ctx.pid_fd = sys_pidfd_open(getpid(), 0);
+ ctx.map_fd = bpf_map__fd(skel->maps.task);
+ ctx.loop = loop;
+ for (i = 0; i < nr; i++) {
+ err = pthread_create(&tids[i], NULL, lookup_fn, &ctx);
+ if (err) {
+ abort_lookup(&ctx, tids, i);
+ CHECK(err, "pthread_create", "error %d\n", err);
+ goto out;
+ }
+ }
+
+ ctx.start = true;
+ for (i = 0; i < nr; i++)
+ pthread_join(tids[i], NULL);
+
+ skel->bss->pid = getpid();
+ err = read_bpf_task_storage_busy__attach(skel);
+ CHECK(err, "attach", "error %d\n", err);
+
+ /* Trigger program */
+ syscall(SYS_gettid);
+ skel->bss->pid = 0;
+
+ CHECK(skel->bss->busy != 0, "bad bpf_task_storage_busy", "got %d\n", skel->bss->busy);
+out:
+ read_bpf_task_storage_busy__destroy(skel);
+ /* Restore affinity setting */
+ sched_setaffinity(getpid(), sizeof(old), &old);
+}
diff --git a/tools/testing/selftests/bpf/progs/read_bpf_task_storage_busy.c b/tools/testing/selftests/bpf/progs/read_bpf_task_storage_busy.c
new file mode 100644
index 000000000000..a47bb0120719
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/read_bpf_task_storage_busy.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) 2022. Huawei Technologies Co., Ltd */
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+extern bool CONFIG_PREEMPT __kconfig __weak;
+extern const int bpf_task_storage_busy __ksym;
+
+char _license[] SEC("license") = "GPL";
+
+int pid = 0;
+int busy = 0;
+
+struct {
+ __uint(type, BPF_MAP_TYPE_TASK_STORAGE);
+ __uint(map_flags, BPF_F_NO_PREALLOC);
+ __type(key, int);
+ __type(value, long);
+} task SEC(".maps");
+
+SEC("raw_tp/sys_enter")
+int BPF_PROG(read_bpf_task_storage_busy)
+{
+ int *value;
+ int key;
+
+ if (!CONFIG_PREEMPT)
+ return 0;
+
+ if (bpf_get_current_pid_tgid() >> 32 != pid)
+ return 0;
+
+ value = bpf_this_cpu_ptr(&bpf_task_storage_busy);
+ if (value)
+ busy = *value;
+
+ return 0;
+}
--
2.29.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy
2022-09-01 6:19 ` [PATCH bpf-next v2 4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy Hou Tao
@ 2022-09-01 19:37 ` Martin KaFai Lau
2022-09-02 3:16 ` Hou Tao
0 siblings, 1 reply; 8+ messages in thread
From: Martin KaFai Lau @ 2022-09-01 19:37 UTC (permalink / raw)
To: Hou Tao
Cc: bpf, Song Liu, Sebastian Andrzej Siewior, Hao Sun, Hao Luo,
Andrii Nakryiko, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, KP Singh, David S . Miller, Jakub Kicinski,
Stanislav Fomichev, Jiri Olsa, John Fastabend, Lorenz Bauer,
houtao1
On Thu, Sep 01, 2022 at 02:19:38PM +0800, Hou Tao wrote:
> +void test_task_storage_map_stress_lookup(void)
> +{
> +#define MAX_NR_THREAD 4096
> + unsigned int i, nr = 256, loop = 8192, cpu = 0;
> + struct read_bpf_task_storage_busy *skel;
> + pthread_t tids[MAX_NR_THREAD];
> + struct lookup_ctx ctx;
> + cpu_set_t old, new;
> + const char *cfg;
> + int err;
> +
> + cfg = getenv("TASK_STORAGE_MAP_NR_THREAD");
> + if (cfg) {
> + nr = atoi(cfg);
> + if (nr > MAX_NR_THREAD)
> + nr = MAX_NR_THREAD;
> + }
> + cfg = getenv("TASK_STORAGE_MAP_NR_LOOP");
> + if (cfg)
> + loop = atoi(cfg);
> + cfg = getenv("TASK_STORAGE_MAP_PIN_CPU");
> + if (cfg)
> + cpu = atoi(cfg);
> +
> + skel = read_bpf_task_storage_busy__open_and_load();
> + err = libbpf_get_error(skel);
> + CHECK(err, "open_and_load", "error %d\n", err);
> +
> + /* Only for a fully preemptible kernel */
> + if (!skel->kconfig->CONFIG_PREEMPT)
> + return;
> +
> + /* Save the old affinity setting */
> + sched_getaffinity(getpid(), sizeof(old), &old);
> +
> + /* Pinned on a specific CPU */
> + CPU_ZERO(&new);
> + CPU_SET(cpu, &new);
> + sched_setaffinity(getpid(), sizeof(new), &new);
> +
> + ctx.start = false;
> + ctx.stop = false;
> + ctx.pid_fd = sys_pidfd_open(getpid(), 0);
> + ctx.map_fd = bpf_map__fd(skel->maps.task);
> + ctx.loop = loop;
> + for (i = 0; i < nr; i++) {
> + err = pthread_create(&tids[i], NULL, lookup_fn, &ctx);
> + if (err) {
> + abort_lookup(&ctx, tids, i);
> + CHECK(err, "pthread_create", "error %d\n", err);
> + goto out;
> + }
> + }
> +
> + ctx.start = true;
> + for (i = 0; i < nr; i++)
> + pthread_join(tids[i], NULL);
> +
> + skel->bss->pid = getpid();
> + err = read_bpf_task_storage_busy__attach(skel);
> + CHECK(err, "attach", "error %d\n", err);
> +
> + /* Trigger program */
> + syscall(SYS_gettid);
> + skel->bss->pid = 0;
> +
> + CHECK(skel->bss->busy != 0, "bad bpf_task_storage_busy", "got %d\n", skel->bss->busy);
Applied. One nit.
Please follow up with a test PASS or SKIP printf.
There is a 'skips' counter in test_maps.c that
is good to bump also.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH bpf-next v2 4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy
2022-09-01 19:37 ` Martin KaFai Lau
@ 2022-09-02 3:16 ` Hou Tao
0 siblings, 0 replies; 8+ messages in thread
From: Hou Tao @ 2022-09-02 3:16 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, Song Liu, Sebastian Andrzej Siewior, Hao Sun, Hao Luo,
Andrii Nakryiko, Yonghong Song, Alexei Starovoitov,
Daniel Borkmann, KP Singh, David S . Miller, Jakub Kicinski,
Stanislav Fomichev, Jiri Olsa, John Fastabend, Lorenz Bauer,
houtao1
Hi,
On 9/2/2022 3:37 AM, Martin KaFai Lau wrote:
> On Thu, Sep 01, 2022 at 02:19:38PM +0800, Hou Tao wrote:
>> +void test_task_storage_map_stress_lookup(void)
>> +{
SNIP
>> + ctx.start = true;
>> + for (i = 0; i < nr; i++)
>> + pthread_join(tids[i], NULL);
>> +
>> + skel->bss->pid = getpid();
>> + err = read_bpf_task_storage_busy__attach(skel);
>> + CHECK(err, "attach", "error %d\n", err);
>> +
>> + /* Trigger program */
>> + syscall(SYS_gettid);
>> + skel->bss->pid = 0;
>> +
>> + CHECK(skel->bss->busy != 0, "bad bpf_task_storage_busy", "got %d\n", skel->bss->busy);
> Applied. One nit.
> Please follow up with a test PASS or SKIP printf.
> There is a 'skips' counter in test_maps.c that
> is good to bump also.
Will send a follow-up patch to do it. Thanks.
>
> .
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety
2022-09-01 6:19 [PATCH bpf-next v2 0/4] Use this_cpu_xxx for preemption-safety Hou Tao
` (3 preceding siblings ...)
2022-09-01 6:19 ` [PATCH bpf-next v2 4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy Hou Tao
@ 2022-09-01 19:30 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-09-01 19:30 UTC (permalink / raw)
To: Hou Tao
Cc: bpf, songliubraving, bigeasy, sunhao.th, haoluo, andrii, yhs, ast,
daniel, kafai, kpsingh, davem, kuba, sdf, jolsa, john.fastabend,
oss, houtao1
Hello:
This series was applied to bpf/bpf-next.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:
On Thu, 1 Sep 2022 14:19:34 +0800 you wrote:
> From: Hou Tao <houtao1@huawei.com>
>
> Hi,
>
> The patchset aims to make the update of per-cpu prog->active and per-cpu
> bpf_task_storage_busy being preemption-safe. The problem is on same
> architectures (e.g. arm64), __this_cpu_{inc|dec|inc_return} are neither
> preemption-safe nor IRQ-safe, so under fully preemptible kernel the
> concurrent updates on these per-cpu variables may be interleaved and the
> final values of these variables may be not zero.
>
> [...]
Here is the summary with links:
- [bpf-next,v2,1/4] bpf: Use this_cpu_{inc|dec|inc_return} for bpf_task_storage_busy
https://git.kernel.org/bpf/bpf-next/c/197827a05e13
- [bpf-next,v2,2/4] bpf: Use this_cpu_{inc_return|dec} for prog->active
https://git.kernel.org/bpf/bpf-next/c/c89e843a11f1
- [bpf-next,v2,3/4] selftests/bpf: Move sys_pidfd_open() into task_local_storage_helpers.h
https://git.kernel.org/bpf/bpf-next/c/c710136e8774
- [bpf-next,v2,4/4] selftests/bpf: Test concurrent updates on bpf_task_storage_busy
https://git.kernel.org/bpf/bpf-next/c/73b97bc78b32
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 8+ messages in thread