* [PATCH bpf-next v2 1/5] bpf: Free struct bpf_cpumask in call_rcu handler
2023-03-16 5:40 [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe David Vernet
@ 2023-03-16 5:40 ` David Vernet
2023-03-16 5:40 ` [PATCH bpf-next v2 2/5] bpf: Mark struct bpf_cpumask as rcu protected David Vernet
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: David Vernet @ 2023-03-16 5:40 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, linux-kernel, kernel-team
The struct bpf_cpumask type uses the bpf_mem_cache_{alloc,free}() APIs
to allocate and free its cpumasks. The bpf_mem allocator may currently
immediately reuse some memory when its freed, without waiting for an RCU
read cycle to elapse. We want to be able to treat struct bpf_cpumask
objects as completely RCU safe.
This is necessary for two reasons:
1. bpf_cpumask_kptr_get() currently does an RCU-protected
refcnt_inc_not_zero(). This of course assumes that the underlying
memory is not reused, and is therefore unsafe in its current form.
2. We want to be able to get rid of bpf_cpumask_kptr_get() entirely, and
intead use the superior kptr RCU semantics now afforded by the
verifier.
This patch fixes (1), and enables (2), by making struct bpf_cpumask RCU
safe. A subsequent patch will update the verifier to allow struct
bpf_cpumask * pointers to be passed to KF_RCU kfuncs, and then a latter
patch will remove bpf_cpumask_kptr_get().
Fixes: 516f4d3397c9 ("bpf: Enable cpumasks to be queried and used as kptrs")
Signed-off-by: David Vernet <void@manifault.com>
---
kernel/bpf/cpumask.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index b6587ec40f1b..98eea62b6b7b 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -9,6 +9,7 @@
/**
* struct bpf_cpumask - refcounted BPF cpumask wrapper structure
* @cpumask: The actual cpumask embedded in the struct.
+ * @rcu: The RCU head used to free the cpumask with RCU safety.
* @usage: Object reference counter. When the refcount goes to 0, the
* memory is released back to the BPF allocator, which provides
* RCU safety.
@@ -24,6 +25,7 @@
*/
struct bpf_cpumask {
cpumask_t cpumask;
+ struct rcu_head rcu;
refcount_t usage;
};
@@ -108,6 +110,16 @@ __bpf_kfunc struct bpf_cpumask *bpf_cpumask_kptr_get(struct bpf_cpumask **cpumas
return cpumask;
}
+static void cpumask_free_cb(struct rcu_head *head)
+{
+ struct bpf_cpumask *cpumask;
+
+ cpumask = container_of(head, struct bpf_cpumask, rcu);
+ migrate_disable();
+ bpf_mem_cache_free(&bpf_cpumask_ma, cpumask);
+ migrate_enable();
+}
+
/**
* bpf_cpumask_release() - Release a previously acquired BPF cpumask.
* @cpumask: The cpumask being released.
@@ -121,11 +133,8 @@ __bpf_kfunc void bpf_cpumask_release(struct bpf_cpumask *cpumask)
if (!cpumask)
return;
- if (refcount_dec_and_test(&cpumask->usage)) {
- migrate_disable();
- bpf_mem_cache_free(&bpf_cpumask_ma, cpumask);
- migrate_enable();
- }
+ if (refcount_dec_and_test(&cpumask->usage))
+ call_rcu(&cpumask->rcu, cpumask_free_cb);
}
/**
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 2/5] bpf: Mark struct bpf_cpumask as rcu protected
2023-03-16 5:40 [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe David Vernet
2023-03-16 5:40 ` [PATCH bpf-next v2 1/5] bpf: Free struct bpf_cpumask in call_rcu handler David Vernet
@ 2023-03-16 5:40 ` David Vernet
2023-03-16 5:40 ` [PATCH bpf-next v2 3/5] bpf/selftests: Test using global cpumask kptr with RCU David Vernet
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: David Vernet @ 2023-03-16 5:40 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, linux-kernel, kernel-team
struct bpf_cpumask is a BPF-wrapper around the struct cpumask type which
can be instantiated by a BPF program, and then queried as a cpumask in
similar fashion to normal kernel code. The previous patch in this series
makes the type fully RCU safe, so the type can be included in the
rcu_protected_type BTF ID list.
A subsequent patch will remove bpf_cpumask_kptr_get(), as it's no longer
useful now that we can just treat the type as RCU safe by default and do
our own if check.
Signed-off-by: David Vernet <void@manifault.com>
---
kernel/bpf/verifier.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 60793f793ca6..15b5c5c729f9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4599,6 +4599,7 @@ static bool in_rcu_cs(struct bpf_verifier_env *env)
BTF_SET_START(rcu_protected_types)
BTF_ID(struct, prog_test_ref_kfunc)
BTF_ID(struct, cgroup)
+BTF_ID(struct, bpf_cpumask)
BTF_SET_END(rcu_protected_types)
static bool rcu_protected_object(const struct btf *btf, u32 btf_id)
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 3/5] bpf/selftests: Test using global cpumask kptr with RCU
2023-03-16 5:40 [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe David Vernet
2023-03-16 5:40 ` [PATCH bpf-next v2 1/5] bpf: Free struct bpf_cpumask in call_rcu handler David Vernet
2023-03-16 5:40 ` [PATCH bpf-next v2 2/5] bpf: Mark struct bpf_cpumask as rcu protected David Vernet
@ 2023-03-16 5:40 ` David Vernet
2023-03-16 5:40 ` [PATCH bpf-next v2 4/5] bpf: Remove bpf_cpumask_kptr_get() kfunc David Vernet
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: David Vernet @ 2023-03-16 5:40 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, linux-kernel, kernel-team
Now that struct bpf_cpumask * is considered an RCU-safe type according
to the verifier, we should add tests that validate its common usages.
This patch adds those tests to the cpumask test suite. A subsequent
changes will remove bpf_cpumask_kptr_get(), and will adjust the selftest
and BPF documentation accordingly.
Signed-off-by: David Vernet <void@manifault.com>
---
.../selftests/bpf/prog_tests/cpumask.c | 1 +
.../selftests/bpf/progs/cpumask_common.h | 6 ++
.../selftests/bpf/progs/cpumask_failure.c | 62 +++++++++++++++++++
.../selftests/bpf/progs/cpumask_success.c | 33 ++++++++++
4 files changed, 102 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
index 5fbe457c4ebe..6c0fe23498c7 100644
--- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
+++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
@@ -17,6 +17,7 @@ static const char * const cpumask_success_testcases[] = {
"test_insert_leave",
"test_insert_remove_release",
"test_insert_kptr_get_release",
+ "test_global_mask_rcu",
};
static void verify_success(const char *prog_name)
diff --git a/tools/testing/selftests/bpf/progs/cpumask_common.h b/tools/testing/selftests/bpf/progs/cpumask_common.h
index 65e5496ca1b2..7623782fbd62 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_common.h
+++ b/tools/testing/selftests/bpf/progs/cpumask_common.h
@@ -9,6 +9,9 @@
int err;
+#define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8)))
+private(MASK) static struct bpf_cpumask __kptr * global_mask;
+
struct __cpumask_map_value {
struct bpf_cpumask __kptr * cpumask;
};
@@ -51,6 +54,9 @@ void bpf_cpumask_copy(struct bpf_cpumask *dst, const struct cpumask *src) __ksym
u32 bpf_cpumask_any(const struct cpumask *src) __ksym;
u32 bpf_cpumask_any_and(const struct cpumask *src1, const struct cpumask *src2) __ksym;
+void bpf_rcu_read_lock(void) __ksym;
+void bpf_rcu_read_unlock(void) __ksym;
+
static inline const struct cpumask *cast(struct bpf_cpumask *cpumask)
{
return (const struct cpumask *)cpumask;
diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c
index cfe83f0ef9e2..9f726d55f747 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_failure.c
+++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c
@@ -127,3 +127,65 @@ int BPF_PROG(test_cpumask_null, struct task_struct *task, u64 clone_flags)
return 0;
}
+
+SEC("tp_btf/task_newtask")
+__failure __msg("R2 must be a rcu pointer")
+int BPF_PROG(test_global_mask_out_of_rcu, struct task_struct *task, u64 clone_flags)
+{
+ struct bpf_cpumask *local, *prev;
+
+ local = create_cpumask();
+ if (!local)
+ return 0;
+
+ prev = bpf_kptr_xchg(&global_mask, local);
+ if (prev) {
+ bpf_cpumask_release(prev);
+ err = 3;
+ return 0;
+ }
+
+ bpf_rcu_read_lock();
+ local = global_mask;
+ if (!local) {
+ err = 4;
+ bpf_rcu_read_unlock();
+ return 0;
+ }
+
+ bpf_rcu_read_unlock();
+
+ /* RCU region is exited before calling KF_RCU kfunc. */
+
+ bpf_cpumask_test_cpu(0, (const struct cpumask *)local);
+
+ return 0;
+}
+
+SEC("tp_btf/task_newtask")
+__failure __msg("NULL pointer passed to trusted arg1")
+int BPF_PROG(test_global_mask_no_null_check, struct task_struct *task, u64 clone_flags)
+{
+ struct bpf_cpumask *local, *prev;
+
+ local = create_cpumask();
+ if (!local)
+ return 0;
+
+ prev = bpf_kptr_xchg(&global_mask, local);
+ if (prev) {
+ bpf_cpumask_release(prev);
+ err = 3;
+ return 0;
+ }
+
+ bpf_rcu_read_lock();
+ local = global_mask;
+
+ /* No NULL check is performed on global cpumask kptr. */
+ bpf_cpumask_test_cpu(0, (const struct cpumask *)local);
+
+ bpf_rcu_read_unlock();
+
+ return 0;
+}
diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
index 97ed08c4ff03..fe928ff72a06 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_success.c
+++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
@@ -423,3 +423,36 @@ int BPF_PROG(test_insert_kptr_get_release, struct task_struct *task, u64 clone_f
return 0;
}
+
+SEC("tp_btf/task_newtask")
+int BPF_PROG(test_global_mask_rcu, struct task_struct *task, u64 clone_flags)
+{
+ struct bpf_cpumask *local, *prev;
+
+ if (!is_test_task())
+ return 0;
+
+ local = create_cpumask();
+ if (!local)
+ return 0;
+
+ prev = bpf_kptr_xchg(&global_mask, local);
+ if (prev) {
+ bpf_cpumask_release(prev);
+ err = 3;
+ return 0;
+ }
+
+ bpf_rcu_read_lock();
+ local = global_mask;
+ if (!local) {
+ err = 4;
+ bpf_rcu_read_unlock();
+ return 0;
+ }
+
+ bpf_cpumask_test_cpu(0, (const struct cpumask *)local);
+ bpf_rcu_read_unlock();
+
+ return 0;
+}
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 4/5] bpf: Remove bpf_cpumask_kptr_get() kfunc
2023-03-16 5:40 [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe David Vernet
` (2 preceding siblings ...)
2023-03-16 5:40 ` [PATCH bpf-next v2 3/5] bpf/selftests: Test using global cpumask kptr with RCU David Vernet
@ 2023-03-16 5:40 ` David Vernet
2023-03-16 5:40 ` [PATCH bpf-next v2 5/5] bpf,docs: Remove bpf_cpumask_kptr_get() from documentation David Vernet
2023-03-16 19:40 ` [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe patchwork-bot+netdevbpf
5 siblings, 0 replies; 7+ messages in thread
From: David Vernet @ 2023-03-16 5:40 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, linux-kernel, kernel-team
Now that struct bpf_cpumask is RCU safe, there's no need for this kfunc.
Rather than doing the following:
private(MASK) static struct bpf_cpumask __kptr *global;
int BPF_PROG(prog, s32 cpu, ...)
{
struct bpf_cpumask *cpumask;
bpf_rcu_read_lock();
cpumask = bpf_cpumask_kptr_get(&global);
if (!cpumask) {
bpf_rcu_read_unlock();
return -1;
}
bpf_cpumask_setall(cpumask);
...
bpf_cpumask_release(cpumask);
bpf_rcu_read_unlock();
}
Programs can instead simply do (assume same global cpumask):
int BPF_PROG(prog, ...)
{
struct bpf_cpumask *cpumask;
bpf_rcu_read_lock();
cpumask = global;
if (!cpumask) {
bpf_rcu_read_unlock();
return -1;
}
bpf_cpumask_setall(cpumask);
...
bpf_rcu_read_unlock();
}
In other words, no extra atomic acquire / release, and less boilerplate
code.
This patch removes both the kfunc, as well as its selftests and
documentation.
Signed-off-by: David Vernet <void@manifault.com>
---
kernel/bpf/cpumask.c | 29 ------------------
.../selftests/bpf/prog_tests/cpumask.c | 1 -
.../selftests/bpf/progs/cpumask_common.h | 1 -
.../selftests/bpf/progs/cpumask_failure.c | 24 ---------------
.../selftests/bpf/progs/cpumask_success.c | 30 -------------------
5 files changed, 85 deletions(-)
diff --git a/kernel/bpf/cpumask.c b/kernel/bpf/cpumask.c
index 98eea62b6b7b..db9da2194c1a 100644
--- a/kernel/bpf/cpumask.c
+++ b/kernel/bpf/cpumask.c
@@ -82,34 +82,6 @@ __bpf_kfunc struct bpf_cpumask *bpf_cpumask_acquire(struct bpf_cpumask *cpumask)
return cpumask;
}
-/**
- * bpf_cpumask_kptr_get() - Attempt to acquire a reference to a BPF cpumask
- * stored in a map.
- * @cpumaskp: A pointer to a BPF cpumask map value.
- *
- * Attempts to acquire a reference to a BPF cpumask stored in a map value. The
- * cpumask returned by this function must either be embedded in a map as a
- * kptr, or freed with bpf_cpumask_release(). This function may return NULL if
- * no BPF cpumask was found in the specified map value.
- */
-__bpf_kfunc struct bpf_cpumask *bpf_cpumask_kptr_get(struct bpf_cpumask **cpumaskp)
-{
- struct bpf_cpumask *cpumask;
-
- /* The BPF memory allocator frees memory backing its caches in an RCU
- * callback. Thus, we can safely use RCU to ensure that the cpumask is
- * safe to read.
- */
- rcu_read_lock();
-
- cpumask = READ_ONCE(*cpumaskp);
- if (cpumask && !refcount_inc_not_zero(&cpumask->usage))
- cpumask = NULL;
-
- rcu_read_unlock();
- return cpumask;
-}
-
static void cpumask_free_cb(struct rcu_head *head)
{
struct bpf_cpumask *cpumask;
@@ -435,7 +407,6 @@ BTF_SET8_START(cpumask_kfunc_btf_ids)
BTF_ID_FLAGS(func, bpf_cpumask_create, KF_ACQUIRE | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_cpumask_release, KF_RELEASE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_cpumask_acquire, KF_ACQUIRE | KF_TRUSTED_ARGS)
-BTF_ID_FLAGS(func, bpf_cpumask_kptr_get, KF_ACQUIRE | KF_KPTR_GET | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_cpumask_first, KF_RCU)
BTF_ID_FLAGS(func, bpf_cpumask_first_zero, KF_RCU)
BTF_ID_FLAGS(func, bpf_cpumask_set_cpu, KF_RCU)
diff --git a/tools/testing/selftests/bpf/prog_tests/cpumask.c b/tools/testing/selftests/bpf/prog_tests/cpumask.c
index 6c0fe23498c7..cdf4acc18e4c 100644
--- a/tools/testing/selftests/bpf/prog_tests/cpumask.c
+++ b/tools/testing/selftests/bpf/prog_tests/cpumask.c
@@ -16,7 +16,6 @@ static const char * const cpumask_success_testcases[] = {
"test_copy_any_anyand",
"test_insert_leave",
"test_insert_remove_release",
- "test_insert_kptr_get_release",
"test_global_mask_rcu",
};
diff --git a/tools/testing/selftests/bpf/progs/cpumask_common.h b/tools/testing/selftests/bpf/progs/cpumask_common.h
index 7623782fbd62..0c5b785a93e4 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_common.h
+++ b/tools/testing/selftests/bpf/progs/cpumask_common.h
@@ -26,7 +26,6 @@ struct array_map {
struct bpf_cpumask *bpf_cpumask_create(void) __ksym;
void bpf_cpumask_release(struct bpf_cpumask *cpumask) __ksym;
struct bpf_cpumask *bpf_cpumask_acquire(struct bpf_cpumask *cpumask) __ksym;
-struct bpf_cpumask *bpf_cpumask_kptr_get(struct bpf_cpumask **cpumask) __ksym;
u32 bpf_cpumask_first(const struct cpumask *cpumask) __ksym;
u32 bpf_cpumask_first_zero(const struct cpumask *cpumask) __ksym;
void bpf_cpumask_set_cpu(u32 cpu, struct bpf_cpumask *cpumask) __ksym;
diff --git a/tools/testing/selftests/bpf/progs/cpumask_failure.c b/tools/testing/selftests/bpf/progs/cpumask_failure.c
index 9f726d55f747..db4f94e72b61 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_failure.c
+++ b/tools/testing/selftests/bpf/progs/cpumask_failure.c
@@ -94,30 +94,6 @@ int BPF_PROG(test_insert_remove_no_release, struct task_struct *task, u64 clone_
return 0;
}
-SEC("tp_btf/task_newtask")
-__failure __msg("Unreleased reference")
-int BPF_PROG(test_kptr_get_no_release, struct task_struct *task, u64 clone_flags)
-{
- struct bpf_cpumask *cpumask;
- struct __cpumask_map_value *v;
-
- cpumask = create_cpumask();
- if (!cpumask)
- return 0;
-
- if (cpumask_map_insert(cpumask))
- return 0;
-
- v = cpumask_map_value_lookup();
- if (!v)
- return 0;
-
- cpumask = bpf_cpumask_kptr_get(&v->cpumask);
-
- /* cpumask is never released. */
- return 0;
-}
-
SEC("tp_btf/task_newtask")
__failure __msg("NULL pointer passed to trusted arg0")
int BPF_PROG(test_cpumask_null, struct task_struct *task, u64 clone_flags)
diff --git a/tools/testing/selftests/bpf/progs/cpumask_success.c b/tools/testing/selftests/bpf/progs/cpumask_success.c
index fe928ff72a06..2fcdd7f68ac7 100644
--- a/tools/testing/selftests/bpf/progs/cpumask_success.c
+++ b/tools/testing/selftests/bpf/progs/cpumask_success.c
@@ -394,36 +394,6 @@ int BPF_PROG(test_insert_remove_release, struct task_struct *task, u64 clone_fla
return 0;
}
-SEC("tp_btf/task_newtask")
-int BPF_PROG(test_insert_kptr_get_release, struct task_struct *task, u64 clone_flags)
-{
- struct bpf_cpumask *cpumask;
- struct __cpumask_map_value *v;
-
- cpumask = create_cpumask();
- if (!cpumask)
- return 0;
-
- if (cpumask_map_insert(cpumask)) {
- err = 3;
- return 0;
- }
-
- v = cpumask_map_value_lookup();
- if (!v) {
- err = 4;
- return 0;
- }
-
- cpumask = bpf_cpumask_kptr_get(&v->cpumask);
- if (cpumask)
- bpf_cpumask_release(cpumask);
- else
- err = 5;
-
- return 0;
-}
-
SEC("tp_btf/task_newtask")
int BPF_PROG(test_global_mask_rcu, struct task_struct *task, u64 clone_flags)
{
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 5/5] bpf,docs: Remove bpf_cpumask_kptr_get() from documentation
2023-03-16 5:40 [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe David Vernet
` (3 preceding siblings ...)
2023-03-16 5:40 ` [PATCH bpf-next v2 4/5] bpf: Remove bpf_cpumask_kptr_get() kfunc David Vernet
@ 2023-03-16 5:40 ` David Vernet
2023-03-16 19:40 ` [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe patchwork-bot+netdevbpf
5 siblings, 0 replies; 7+ messages in thread
From: David Vernet @ 2023-03-16 5:40 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, linux-kernel, kernel-team
Now that the kfunc no longer exists, we can remove it and instead
describe how RCU can be used to get a struct bpf_cpumask from a map
value. This patch updates the BPF documentation accordingly.
Signed-off-by: David Vernet <void@manifault.com>
---
Documentation/bpf/cpumasks.rst | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/Documentation/bpf/cpumasks.rst b/Documentation/bpf/cpumasks.rst
index 75344cd230e5..41efd8874eeb 100644
--- a/Documentation/bpf/cpumasks.rst
+++ b/Documentation/bpf/cpumasks.rst
@@ -117,12 +117,7 @@ For example:
As mentioned and illustrated above, these ``struct bpf_cpumask *`` objects can
also be stored in a map and used as kptrs. If a ``struct bpf_cpumask *`` is in
a map, the reference can be removed from the map with bpf_kptr_xchg(), or
-opportunistically acquired with bpf_cpumask_kptr_get():
-
-.. kernel-doc:: kernel/bpf/cpumask.c
- :identifiers: bpf_cpumask_kptr_get
-
-Here is an example of a ``struct bpf_cpumask *`` being retrieved from a map:
+opportunistically acquired using RCU:
.. code-block:: c
@@ -144,7 +139,7 @@ Here is an example of a ``struct bpf_cpumask *`` being retrieved from a map:
/**
* A simple example tracepoint program showing how a
* struct bpf_cpumask * kptr that is stored in a map can
- * be acquired using the bpf_cpumask_kptr_get() kfunc.
+ * be passed to kfuncs using RCU protection.
*/
SEC("tp_btf/cgroup_mkdir")
int BPF_PROG(cgrp_ancestor_example, struct cgroup *cgrp, const char *path)
@@ -158,26 +153,21 @@ Here is an example of a ``struct bpf_cpumask *`` being retrieved from a map:
if (!v)
return -ENOENT;
+ bpf_rcu_read_lock();
/* Acquire a reference to the bpf_cpumask * kptr that's already stored in the map. */
- kptr = bpf_cpumask_kptr_get(&v->cpumask);
- if (!kptr)
+ kptr = v->cpumask;
+ if (!kptr) {
/* If no bpf_cpumask was present in the map, it's because
* we're racing with another CPU that removed it with
* bpf_kptr_xchg() between the bpf_map_lookup_elem()
- * above, and our call to bpf_cpumask_kptr_get().
- * bpf_cpumask_kptr_get() internally safely handles this
- * race, and will return NULL if the cpumask is no longer
- * present in the map by the time we invoke the kfunc.
+ * above, and our load of the pointer from the map.
*/
+ bpf_rcu_read_unlock();
return -EBUSY;
+ }
- /* Free the reference we just took above. Note that the
- * original struct bpf_cpumask * kptr is still in the map. It will
- * be freed either at a later time if another context deletes
- * it from the map, or automatically by the BPF subsystem if
- * it's still present when the map is destroyed.
- */
- bpf_cpumask_release(kptr);
+ bpf_cpumask_setall(kptr);
+ bpf_rcu_read_unlock();
return 0;
}
--
2.39.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe
2023-03-16 5:40 [PATCH bpf-next v2 0/5] Make struct bpf_cpumask RCU safe David Vernet
` (4 preceding siblings ...)
2023-03-16 5:40 ` [PATCH bpf-next v2 5/5] bpf,docs: Remove bpf_cpumask_kptr_get() from documentation David Vernet
@ 2023-03-16 19:40 ` patchwork-bot+netdevbpf
5 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-16 19:40 UTC (permalink / raw)
To: David Vernet
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, linux-kernel, kernel-team
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Thu, 16 Mar 2023 00:40:23 -0500 you wrote:
> The struct bpf_cpumask type is currently not RCU safe. It uses the
> bpf_mem_cache_{alloc,free}() APIs to allocate and release cpumasks, and
> those allocations may be reused before an RCU grace period has elapsed.
> We want to be able to enable using this pattern in BPF programs:
>
> private(MASK) static struct bpf_cpumask __kptr *global;
>
> [...]
Here is the summary with links:
- [bpf-next,v2,1/5] bpf: Free struct bpf_cpumask in call_rcu handler
https://git.kernel.org/bpf/bpf-next/c/77473d1a962f
- [bpf-next,v2,2/5] bpf: Mark struct bpf_cpumask as rcu protected
https://git.kernel.org/bpf/bpf-next/c/63d2d83d21a6
- [bpf-next,v2,3/5] bpf/selftests: Test using global cpumask kptr with RCU
https://git.kernel.org/bpf/bpf-next/c/a5a197df58c4
- [bpf-next,v2,4/5] bpf: Remove bpf_cpumask_kptr_get() kfunc
https://git.kernel.org/bpf/bpf-next/c/1b403ce77dfb
- [bpf-next,v2,5/5] bpf,docs: Remove bpf_cpumask_kptr_get() from documentation
https://git.kernel.org/bpf/bpf-next/c/fec2c6d14fd5
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] 7+ messages in thread