On 10/30/23 12:28 PM, thinker.li@xxxxxxxxx wrote:
From: Kui-Feng Lee <thinker.li@xxxxxxxxx>
Giving a BTF, the bpf_struct_ops knows the right place to look up type info
associated with a type ID. This enables a user space program to load a
struct_ops object linked to a struct_ops type defined by a module, by
providing the module BTF (fd).
This describes about the struct_ops map creation change (by adding
value_type_btf_obj_fd)? It could be described more clearly in the commit
message, like specify the value_type_btf_obj_fd addition and how it is used in
the struct_ops map creation.
The bpf_prog includes attach_btf in aux which is passed along with the
bpf_attr when loading the program. The purpose of attach_btf is to
determine the btf type of attach_btf_id. The attach_btf_id is then used to
identify the traced function for a trace program. In the case of struct_ops
programs, it is used to identify the struct_ops type of the struct_ops
object that a program is attached to.
Does attach_btf_obj_fd also work?
[ ... ]
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 256516aba632..db2bbba50e38 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -694,6 +694,7 @@ static void __bpf_struct_ops_map_free(struct bpf_map *map)
bpf_jit_uncharge_modmem(PAGE_SIZE);
}
bpf_map_area_free(st_map->uvalue);
+ btf_put(st_map->st_ops_desc->btf);
bpf_map_area_free(st_map);
}
@@ -735,16 +736,31 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
const struct btf_type *t, *vt;
struct module *mod = NULL;
struct bpf_map *map;
+ struct btf *btf;
int ret;
- st_ops_desc = bpf_struct_ops_find_value(btf_vmlinux, attr->btf_vmlinux_value_type_id);
- if (!st_ops_desc)
- return ERR_PTR(-ENOTSUPP);
+ if (attr->value_type_btf_obj_fd) {
+ /* The map holds btf for its whole life time. */
It took me a while to parse this comment and connect it with the
btf_put(st_map->st_ops_desc->btf) in the __bpf_struct_ops_map_free() above.
It is now like "btf" owns "struct_ops_desc" which also stores a pointer pointing
back to itself, like "btf->struct_ops_desc->btf". The struct_ops_desc->btf was
not initialized by st_map but st_map will increment its refcount much later.
Can btf be directly stored in the st_map->btf instead and map_alloc holds the
refcnt of st_map->btf and btf_put(st_map->btf) in map_free?
+ btf = btf_get_by_fd(attr->value_type_btf_obj_fd);
+ if (IS_ERR(btf))
+ return ERR_PTR(PTR_ERR(btf));
+
+ if (btf != btf_vmlinux) {
+ mod = btf_try_get_module(btf);
+ if (!mod) {
+ ret = -EINVAL;
+ goto errout;
+ }
+ }
+ } else {
+ btf = btf_vmlinux;
+ btf_get(btf);
+ }
- if (st_ops_desc->btf != btf_vmlinux) {
- mod = btf_try_get_module(st_ops_desc->btf);
- if (!mod)
- return ERR_PTR(-EINVAL);
+ st_ops_desc = bpf_struct_ops_find_value(btf, attr->btf_vmlinux_value_type_id);
+ if (!st_ops_desc) {
+ ret = -ENOTSUPP;
+ goto errout;
}
vt = st_ops_desc->value_type;
