From: Hou Tao <houtao1@xxxxxxxxxx>
When removing the inner map from the outer map, the inner map will be
freed after one RCU grace period and one RCU tasks trace grace
period, so it is certain that the bpf program which may access the inner
map, has exited before the inner map is freed.
However there is unnecessary to wait for one RCU grace period or one RCU
tasks trace grace period if the outer map is only accessed by sleepable
program or non-sleepable program. So recording the context of the owned
bpf programs when adding map into env->used_maps and using the recorded
access context to decide which, and how many, RCU grace periods are
needed when freeing the inner map.
Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
---
include/linux/bpf.h | 9 ++++++++-
kernel/bpf/map_in_map.c | 18 +++++++++++++-----
kernel/bpf/syscall.c | 15 +++++++++++++--
kernel/bpf/verifier.c | 5 +++++
4 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index ec3c90202ffe6..8faa1af4b39df 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -245,6 +245,12 @@ struct bpf_list_node_kern {
void *owner;
} __attribute__((aligned(8)));
+enum {
+ BPF_MAP_ACC_NORMAL_PROG_CTX = 1,
+ BPF_MAP_ACC_SLEEPABLE_PROG_CTX = 2,
+ BPF_MAP_ACC_PROG_CTX_MASK = BPF_MAP_ACC_NORMAL_PROG_CTX | BPF_MAP_ACC_SLEEPABLE_PROG_CTX,
+};
+
struct bpf_map {
/* The first two cachelines with read-mostly members of which some
* are also accessed in fast-path (e.g. ops, max_entries).
@@ -292,7 +298,8 @@ struct bpf_map {
} owner;
bool bypass_spec_v1;
bool frozen; /* write-once; write-protected by freeze_mutex */
- bool free_after_mult_rcu_gp;
+ atomic_t owned_prog_ctx;
+ atomic_t may_be_accessed_prog_ctx;
s64 __percpu *elem_count;
};
diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c
index cf33630655661..e3d26a89ac5b6 100644
--- a/kernel/bpf/map_in_map.c
+++ b/kernel/bpf/map_in_map.c
@@ -131,12 +131,20 @@ void bpf_map_fd_put_ptr(struct bpf_map *map, void *ptr, bool deferred)
{
struct bpf_map *inner_map = ptr;
- /* The inner map may still be used by both non-sleepable and sleepable
- * bpf program, so free it after one RCU grace period and one tasks
- * trace RCU grace period.
+ /* Defer the freeing of inner map according to the owner program
+ * context of outer maps, so unnecessary multiple RCU GP waitings
+ * can be avoided.
*/
- if (deferred)
- WRITE_ONCE(inner_map->free_after_mult_rcu_gp, true);
+ if (deferred) {
+ /* owned_prog_ctx may be updated concurrently by new bpf program
+ * so add smp_mb() below to ensure that reading owned_prog_ctx
+ * will return the newly-set bit when the new bpf program finds
+ * the inner map before it is removed from outer map.
+ */
+ smp_mb();
+ atomic_or(atomic_read(&map->owned_prog_ctx),
+ &inner_map->may_be_accessed_prog_ctx);
+ }
bpf_map_put(inner_map);
}
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index e2d2701ce2c45..5a7906f2b027e 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -694,12 +694,20 @@ static void bpf_map_free_deferred(struct work_struct *work)
{
struct bpf_map *map = container_of(work, struct bpf_map, work);
struct btf_record *rec = map->record;
+ int acc_ctx;
security_bpf_map_free(map);
bpf_map_release_memcg(map);
- if (READ_ONCE(map->free_after_mult_rcu_gp))
- synchronize_rcu_mult(call_rcu, call_rcu_tasks_trace);
+ acc_ctx = atomic_read(&map->may_be_accessed_prog_ctx) & BPF_MAP_ACC_PROG_CTX_MASK;
+ if (acc_ctx) {
+ if (acc_ctx == BPF_MAP_ACC_NORMAL_PROG_CTX)
+ synchronize_rcu();
+ else if (acc_ctx == BPF_MAP_ACC_SLEEPABLE_PROG_CTX)
+ synchronize_rcu_tasks_trace();
+ else
+ synchronize_rcu_mult(call_rcu, call_rcu_tasks_trace);
+ }
/* implementation dependent freeing */
map->ops->map_free(map);
@@ -5326,6 +5334,9 @@ static int bpf_prog_bind_map(union bpf_attr *attr)
goto out_unlock;
}
+ /* No need to update owned_prog_ctx, because the bpf program doesn't
+ * access the map.
+ */
memcpy(used_maps_new, used_maps_old,
sizeof(used_maps_old[0]) * prog->aux->used_map_cnt);
used_maps_new[prog->aux->used_map_cnt] = map;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bd1c42eb540f1..d8d5432b240dc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18012,12 +18012,17 @@ static int resolve_pseudo_ldimm64(struct bpf_verifier_env *env)
return -E2BIG;
}
+ atomic_or(env->prog->aux->sleepable ? BPF_MAP_ACC_SLEEPABLE_PROG_CTX :
+ BPF_MAP_ACC_NORMAL_PROG_CTX,
+ &map->owned_prog_ctx);
/* hold the map. If the program is rejected by verifier,
* the map will be released by release_maps() or it
* will be used by the valid program until it's unloaded
* and all maps are released in free_used_maps()
*/
bpf_map_inc(map);
+ /* Paired with smp_mb() in bpf_map_fd_put_ptr() */
+ smp_mb__after_atomic();
aux->map_index = env->used_map_cnt;
env->used_maps[env->used_map_cnt++] = map;
--
2.29.2
