| Message ID | 20221206231000.3180914-5-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | BPF rbtree next-gen datastructure | expand |
On Tue, Dec 06, 2022 at 03:09:51PM -0800, Dave Marchevsky wrote: > Many of the structs recently added to track field info for linked-list > head are useful as-is for rbtree root. So let's do a mechanical renaming > of list_head-related types and fields: > > include/linux/bpf.h: > struct btf_field_list_head -> struct btf_field_datastructure_head > list_head -> datastructure_head in struct btf_field union > kernel/bpf/btf.c: > list_head -> datastructure_head in struct btf_field_info Looking through this patch and others it eventually becomes confusing with 'datastructure head' name. I'm not sure what is 'head' of the data structure. There is head in the link list, but 'head of tree' is odd. The attemp here is to find a common name that represents programming concept where there is a 'root' and there are 'nodes' that added to that 'root'. The 'data structure' name is too broad in that sense. Especially later it becomes 'datastructure_api' which is even broader. I was thinking to propose: struct btf_field_list_head -> struct btf_field_tree_root list_head -> tree_root in struct btf_field union and is_kfunc_tree_api later... since link list is a tree too. But reading 'tree' next to other names like 'field', 'kfunc' it might be mistaken that 'tree' applies to the former. So I think using 'graph' as more general concept to describe both link list and rb-tree would be the best. So the proposal: struct btf_field_list_head -> struct btf_field_graph_root list_head -> graph_root in struct btf_field union and is_kfunc_graph_api later... 'graph' is short enough and rarely used in names, so it stands on its own next to 'field' and in combination with other names. wdyt? > > This is a nonfunctional change, functionality to actually use these > fields for rbtree will be added in further patches. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- > include/linux/bpf.h | 4 ++-- > kernel/bpf/btf.c | 21 +++++++++++---------- > kernel/bpf/helpers.c | 4 ++-- > kernel/bpf/verifier.c | 21 +++++++++++---------- > 4 files changed, 26 insertions(+), 24 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 4920ac252754..9e8b12c7061e 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -189,7 +189,7 @@ struct btf_field_kptr { > u32 btf_id; > }; > > -struct btf_field_list_head { > +struct btf_field_datastructure_head { > struct btf *btf; > u32 value_btf_id; > u32 node_offset; > @@ -201,7 +201,7 @@ struct btf_field { > enum btf_field_type type; > union { > struct btf_field_kptr kptr; > - struct btf_field_list_head list_head; > + struct btf_field_datastructure_head datastructure_head; > }; > }; > > diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c > index c80bd8709e69..284e3e4b76b7 100644 > --- a/kernel/bpf/btf.c > +++ b/kernel/bpf/btf.c > @@ -3227,7 +3227,7 @@ struct btf_field_info { > struct { > const char *node_name; > u32 value_btf_id; > - } list_head; > + } datastructure_head; > }; > }; > > @@ -3334,8 +3334,8 @@ static int btf_find_list_head(const struct btf *btf, const struct btf_type *pt, > return -EINVAL; > info->type = BPF_LIST_HEAD; > info->off = off; > - info->list_head.value_btf_id = id; > - info->list_head.node_name = list_node; > + info->datastructure_head.value_btf_id = id; > + info->datastructure_head.node_name = list_node; > return BTF_FIELD_FOUND; > } > > @@ -3603,13 +3603,14 @@ static int btf_parse_list_head(const struct btf *btf, struct btf_field *field, > u32 offset; > int i; > > - t = btf_type_by_id(btf, info->list_head.value_btf_id); > + t = btf_type_by_id(btf, info->datastructure_head.value_btf_id); > /* We've already checked that value_btf_id is a struct type. We > * just need to figure out the offset of the list_node, and > * verify its type. > */ > for_each_member(i, t, member) { > - if (strcmp(info->list_head.node_name, __btf_name_by_offset(btf, member->name_off))) > + if (strcmp(info->datastructure_head.node_name, > + __btf_name_by_offset(btf, member->name_off))) > continue; > /* Invalid BTF, two members with same name */ > if (n) > @@ -3626,9 +3627,9 @@ static int btf_parse_list_head(const struct btf *btf, struct btf_field *field, > if (offset % __alignof__(struct bpf_list_node)) > return -EINVAL; > > - field->list_head.btf = (struct btf *)btf; > - field->list_head.value_btf_id = info->list_head.value_btf_id; > - field->list_head.node_offset = offset; > + field->datastructure_head.btf = (struct btf *)btf; > + field->datastructure_head.value_btf_id = info->datastructure_head.value_btf_id; > + field->datastructure_head.node_offset = offset; > } > if (!n) > return -ENOENT; > @@ -3735,11 +3736,11 @@ int btf_check_and_fixup_fields(const struct btf *btf, struct btf_record *rec) > > if (!(rec->fields[i].type & BPF_LIST_HEAD)) > continue; > - btf_id = rec->fields[i].list_head.value_btf_id; > + btf_id = rec->fields[i].datastructure_head.value_btf_id; > meta = btf_find_struct_meta(btf, btf_id); > if (!meta) > return -EFAULT; > - rec->fields[i].list_head.value_rec = meta->record; > + rec->fields[i].datastructure_head.value_rec = meta->record; > > if (!(rec->field_mask & BPF_LIST_NODE)) > continue; > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index cca642358e80..6c67740222c2 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -1737,12 +1737,12 @@ void bpf_list_head_free(const struct btf_field *field, void *list_head, > while (head != orig_head) { > void *obj = head; > > - obj -= field->list_head.node_offset; > + obj -= field->datastructure_head.node_offset; > head = head->next; > /* The contained type can also have resources, including a > * bpf_list_head which needs to be freed. > */ > - bpf_obj_free_fields(field->list_head.value_rec, obj); > + bpf_obj_free_fields(field->datastructure_head.value_rec, obj); > /* bpf_mem_free requires migrate_disable(), since we can be > * called from map free path as well apart from BPF program (as > * part of map ops doing bpf_obj_free_fields). > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 6f0aac837d77..bc80b4c4377b 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -8615,21 +8615,22 @@ static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env, > > field = meta->arg_list_head.field; > > - et = btf_type_by_id(field->list_head.btf, field->list_head.value_btf_id); > + et = btf_type_by_id(field->datastructure_head.btf, field->datastructure_head.value_btf_id); > t = btf_type_by_id(reg->btf, reg->btf_id); > - if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->list_head.btf, > - field->list_head.value_btf_id, true)) { > + if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->datastructure_head.btf, > + field->datastructure_head.value_btf_id, true)) { > verbose(env, "operation on bpf_list_head expects arg#1 bpf_list_node at offset=%d " > "in struct %s, but arg is at offset=%d in struct %s\n", > - field->list_head.node_offset, btf_name_by_offset(field->list_head.btf, et->name_off), > + field->datastructure_head.node_offset, > + btf_name_by_offset(field->datastructure_head.btf, et->name_off), > list_node_off, btf_name_by_offset(reg->btf, t->name_off)); > return -EINVAL; > } > > - if (list_node_off != field->list_head.node_offset) { > + if (list_node_off != field->datastructure_head.node_offset) { > verbose(env, "arg#1 offset=%d, but expected bpf_list_node at offset=%d in struct %s\n", > - list_node_off, field->list_head.node_offset, > - btf_name_by_offset(field->list_head.btf, et->name_off)); > + list_node_off, field->datastructure_head.node_offset, > + btf_name_by_offset(field->datastructure_head.btf, et->name_off)); > return -EINVAL; > } > /* Set arg#1 for expiration after unlock */ > @@ -9078,9 +9079,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, > > mark_reg_known_zero(env, regs, BPF_REG_0); > regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC; > - regs[BPF_REG_0].btf = field->list_head.btf; > - regs[BPF_REG_0].btf_id = field->list_head.value_btf_id; > - regs[BPF_REG_0].off = field->list_head.node_offset; > + regs[BPF_REG_0].btf = field->datastructure_head.btf; > + regs[BPF_REG_0].btf_id = field->datastructure_head.value_btf_id; > + regs[BPF_REG_0].off = field->datastructure_head.node_offset; > } else if (meta.func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx]) { > mark_reg_known_zero(env, regs, BPF_REG_0); > regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_TRUSTED; > -- > 2.30.2 >
On 12/6/22 8:41 PM, Alexei Starovoitov wrote: > On Tue, Dec 06, 2022 at 03:09:51PM -0800, Dave Marchevsky wrote: >> Many of the structs recently added to track field info for linked-list >> head are useful as-is for rbtree root. So let's do a mechanical renaming >> of list_head-related types and fields: >> >> include/linux/bpf.h: >> struct btf_field_list_head -> struct btf_field_datastructure_head >> list_head -> datastructure_head in struct btf_field union >> kernel/bpf/btf.c: >> list_head -> datastructure_head in struct btf_field_info > > Looking through this patch and others it eventually becomes > confusing with 'datastructure head' name. > I'm not sure what is 'head' of the data structure. > There is head in the link list, but 'head of tree' is odd. > > The attemp here is to find a common name that represents programming > concept where there is a 'root' and there are 'nodes' that added to that 'root'. > The 'data structure' name is too broad in that sense. > Especially later it becomes 'datastructure_api' which is even broader. > > I was thinking to propose: > struct btf_field_list_head -> struct btf_field_tree_root > list_head -> tree_root in struct btf_field union > > and is_kfunc_tree_api later... > since link list is a tree too. > > But reading 'tree' next to other names like 'field', 'kfunc' > it might be mistaken that 'tree' applies to the former. > So I think using 'graph' as more general concept to describe both > link list and rb-tree would be the best. > > So the proposal: > struct btf_field_list_head -> struct btf_field_graph_root > list_head -> graph_root in struct btf_field union > > and is_kfunc_graph_api later... > > 'graph' is short enough and rarely used in names, > so it stands on its own next to 'field' and in combination > with other names. > wdyt? > I'm not a huge fan of 'graph', but it's certainly better than 'datastructure_api', and avoids the "all next-gen datastructures must do this" implication of a 'ng_ds' name. So will try the rename in v2. (all specific GRAPH naming suggestions in subsequent patches will be done as well) list 'head' -> list 'root' SGTM as well. Not ideal, but alternatives are worse (rbtree 'head'...) >> >> This is a nonfunctional change, functionality to actually use these >> fields for rbtree will be added in further patches. >> >> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> >> --- >> include/linux/bpf.h | 4 ++-- >> kernel/bpf/btf.c | 21 +++++++++++---------- >> kernel/bpf/helpers.c | 4 ++-- >> kernel/bpf/verifier.c | 21 +++++++++++---------- >> 4 files changed, 26 insertions(+), 24 deletions(-) >> >> diff --git a/include/linux/bpf.h b/include/linux/bpf.h >> index 4920ac252754..9e8b12c7061e 100644 >> --- a/include/linux/bpf.h >> +++ b/include/linux/bpf.h >> @@ -189,7 +189,7 @@ struct btf_field_kptr { >> u32 btf_id; >> }; >> >> -struct btf_field_list_head { >> +struct btf_field_datastructure_head { >> struct btf *btf; >> u32 value_btf_id; >> u32 node_offset; >> @@ -201,7 +201,7 @@ struct btf_field { >> enum btf_field_type type; >> union { >> struct btf_field_kptr kptr; >> - struct btf_field_list_head list_head; >> + struct btf_field_datastructure_head datastructure_head; >> }; >> }; >> >> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c >> index c80bd8709e69..284e3e4b76b7 100644 >> --- a/kernel/bpf/btf.c >> +++ b/kernel/bpf/btf.c >> @@ -3227,7 +3227,7 @@ struct btf_field_info { >> struct { >> const char *node_name; >> u32 value_btf_id; >> - } list_head; >> + } datastructure_head; >> }; >> }; >> >> @@ -3334,8 +3334,8 @@ static int btf_find_list_head(const struct btf *btf, const struct btf_type *pt, >> return -EINVAL; >> info->type = BPF_LIST_HEAD; >> info->off = off; >> - info->list_head.value_btf_id = id; >> - info->list_head.node_name = list_node; >> + info->datastructure_head.value_btf_id = id; >> + info->datastructure_head.node_name = list_node; >> return BTF_FIELD_FOUND; >> } >> >> @@ -3603,13 +3603,14 @@ static int btf_parse_list_head(const struct btf *btf, struct btf_field *field, >> u32 offset; >> int i; >> >> - t = btf_type_by_id(btf, info->list_head.value_btf_id); >> + t = btf_type_by_id(btf, info->datastructure_head.value_btf_id); >> /* We've already checked that value_btf_id is a struct type. We >> * just need to figure out the offset of the list_node, and >> * verify its type. >> */ >> for_each_member(i, t, member) { >> - if (strcmp(info->list_head.node_name, __btf_name_by_offset(btf, member->name_off))) >> + if (strcmp(info->datastructure_head.node_name, >> + __btf_name_by_offset(btf, member->name_off))) >> continue; >> /* Invalid BTF, two members with same name */ >> if (n) >> @@ -3626,9 +3627,9 @@ static int btf_parse_list_head(const struct btf *btf, struct btf_field *field, >> if (offset % __alignof__(struct bpf_list_node)) >> return -EINVAL; >> >> - field->list_head.btf = (struct btf *)btf; >> - field->list_head.value_btf_id = info->list_head.value_btf_id; >> - field->list_head.node_offset = offset; >> + field->datastructure_head.btf = (struct btf *)btf; >> + field->datastructure_head.value_btf_id = info->datastructure_head.value_btf_id; >> + field->datastructure_head.node_offset = offset; >> } >> if (!n) >> return -ENOENT; >> @@ -3735,11 +3736,11 @@ int btf_check_and_fixup_fields(const struct btf *btf, struct btf_record *rec) >> >> if (!(rec->fields[i].type & BPF_LIST_HEAD)) >> continue; >> - btf_id = rec->fields[i].list_head.value_btf_id; >> + btf_id = rec->fields[i].datastructure_head.value_btf_id; >> meta = btf_find_struct_meta(btf, btf_id); >> if (!meta) >> return -EFAULT; >> - rec->fields[i].list_head.value_rec = meta->record; >> + rec->fields[i].datastructure_head.value_rec = meta->record; >> >> if (!(rec->field_mask & BPF_LIST_NODE)) >> continue; >> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c >> index cca642358e80..6c67740222c2 100644 >> --- a/kernel/bpf/helpers.c >> +++ b/kernel/bpf/helpers.c >> @@ -1737,12 +1737,12 @@ void bpf_list_head_free(const struct btf_field *field, void *list_head, >> while (head != orig_head) { >> void *obj = head; >> >> - obj -= field->list_head.node_offset; >> + obj -= field->datastructure_head.node_offset; >> head = head->next; >> /* The contained type can also have resources, including a >> * bpf_list_head which needs to be freed. >> */ >> - bpf_obj_free_fields(field->list_head.value_rec, obj); >> + bpf_obj_free_fields(field->datastructure_head.value_rec, obj); >> /* bpf_mem_free requires migrate_disable(), since we can be >> * called from map free path as well apart from BPF program (as >> * part of map ops doing bpf_obj_free_fields). >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 6f0aac837d77..bc80b4c4377b 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -8615,21 +8615,22 @@ static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env, >> >> field = meta->arg_list_head.field; >> >> - et = btf_type_by_id(field->list_head.btf, field->list_head.value_btf_id); >> + et = btf_type_by_id(field->datastructure_head.btf, field->datastructure_head.value_btf_id); >> t = btf_type_by_id(reg->btf, reg->btf_id); >> - if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->list_head.btf, >> - field->list_head.value_btf_id, true)) { >> + if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->datastructure_head.btf, >> + field->datastructure_head.value_btf_id, true)) { >> verbose(env, "operation on bpf_list_head expects arg#1 bpf_list_node at offset=%d " >> "in struct %s, but arg is at offset=%d in struct %s\n", >> - field->list_head.node_offset, btf_name_by_offset(field->list_head.btf, et->name_off), >> + field->datastructure_head.node_offset, >> + btf_name_by_offset(field->datastructure_head.btf, et->name_off), >> list_node_off, btf_name_by_offset(reg->btf, t->name_off)); >> return -EINVAL; >> } >> >> - if (list_node_off != field->list_head.node_offset) { >> + if (list_node_off != field->datastructure_head.node_offset) { >> verbose(env, "arg#1 offset=%d, but expected bpf_list_node at offset=%d in struct %s\n", >> - list_node_off, field->list_head.node_offset, >> - btf_name_by_offset(field->list_head.btf, et->name_off)); >> + list_node_off, field->datastructure_head.node_offset, >> + btf_name_by_offset(field->datastructure_head.btf, et->name_off)); >> return -EINVAL; >> } >> /* Set arg#1 for expiration after unlock */ >> @@ -9078,9 +9079,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, >> >> mark_reg_known_zero(env, regs, BPF_REG_0); >> regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC; >> - regs[BPF_REG_0].btf = field->list_head.btf; >> - regs[BPF_REG_0].btf_id = field->list_head.value_btf_id; >> - regs[BPF_REG_0].off = field->list_head.node_offset; >> + regs[BPF_REG_0].btf = field->datastructure_head.btf; >> + regs[BPF_REG_0].btf_id = field->datastructure_head.value_btf_id; >> + regs[BPF_REG_0].off = field->datastructure_head.node_offset; >> } else if (meta.func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx]) { >> mark_reg_known_zero(env, regs, BPF_REG_0); >> regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_TRUSTED; >> -- >> 2.30.2 >>
On Wed, Dec 07, 2022 at 01:52:07PM -0500, Dave Marchevsky wrote: > On 12/6/22 8:41 PM, Alexei Starovoitov wrote: > > On Tue, Dec 06, 2022 at 03:09:51PM -0800, Dave Marchevsky wrote: > >> Many of the structs recently added to track field info for linked-list > >> head are useful as-is for rbtree root. So let's do a mechanical renaming > >> of list_head-related types and fields: > >> > >> include/linux/bpf.h: > >> struct btf_field_list_head -> struct btf_field_datastructure_head > >> list_head -> datastructure_head in struct btf_field union > >> kernel/bpf/btf.c: > >> list_head -> datastructure_head in struct btf_field_info > > > > Looking through this patch and others it eventually becomes > > confusing with 'datastructure head' name. > > I'm not sure what is 'head' of the data structure. > > There is head in the link list, but 'head of tree' is odd. > > > > The attemp here is to find a common name that represents programming > > concept where there is a 'root' and there are 'nodes' that added to that 'root'. > > The 'data structure' name is too broad in that sense. > > Especially later it becomes 'datastructure_api' which is even broader. > > > > I was thinking to propose: > > struct btf_field_list_head -> struct btf_field_tree_root > > list_head -> tree_root in struct btf_field union > > > > and is_kfunc_tree_api later... > > since link list is a tree too. > > > > But reading 'tree' next to other names like 'field', 'kfunc' > > it might be mistaken that 'tree' applies to the former. > > So I think using 'graph' as more general concept to describe both > > link list and rb-tree would be the best. > > > > So the proposal: > > struct btf_field_list_head -> struct btf_field_graph_root > > list_head -> graph_root in struct btf_field union > > > > and is_kfunc_graph_api later... > > > > 'graph' is short enough and rarely used in names, > > so it stands on its own next to 'field' and in combination > > with other names. > > wdyt? > > > > I'm not a huge fan of 'graph', but it's certainly better than > 'datastructure_api', and avoids the "all next-gen datastructures must do this" > implication of a 'ng_ds' name. So will try the rename in v2. fwiw I don't like 'next-' bit in 'next-gen ds'. A year from now the 'next' will sound really old. Just like N in NAPI used to be 'new'. > (all specific GRAPH naming suggestions in subsequent patches will > be done as well) > > list 'head' -> list 'root' SGTM as well. Not ideal, but alternatives > are worse (rbtree 'head'...) Thanks!
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 4920ac252754..9e8b12c7061e 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -189,7 +189,7 @@ struct btf_field_kptr { u32 btf_id; }; -struct btf_field_list_head { +struct btf_field_datastructure_head { struct btf *btf; u32 value_btf_id; u32 node_offset; @@ -201,7 +201,7 @@ struct btf_field { enum btf_field_type type; union { struct btf_field_kptr kptr; - struct btf_field_list_head list_head; + struct btf_field_datastructure_head datastructure_head; }; }; diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index c80bd8709e69..284e3e4b76b7 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -3227,7 +3227,7 @@ struct btf_field_info { struct { const char *node_name; u32 value_btf_id; - } list_head; + } datastructure_head; }; }; @@ -3334,8 +3334,8 @@ static int btf_find_list_head(const struct btf *btf, const struct btf_type *pt, return -EINVAL; info->type = BPF_LIST_HEAD; info->off = off; - info->list_head.value_btf_id = id; - info->list_head.node_name = list_node; + info->datastructure_head.value_btf_id = id; + info->datastructure_head.node_name = list_node; return BTF_FIELD_FOUND; } @@ -3603,13 +3603,14 @@ static int btf_parse_list_head(const struct btf *btf, struct btf_field *field, u32 offset; int i; - t = btf_type_by_id(btf, info->list_head.value_btf_id); + t = btf_type_by_id(btf, info->datastructure_head.value_btf_id); /* We've already checked that value_btf_id is a struct type. We * just need to figure out the offset of the list_node, and * verify its type. */ for_each_member(i, t, member) { - if (strcmp(info->list_head.node_name, __btf_name_by_offset(btf, member->name_off))) + if (strcmp(info->datastructure_head.node_name, + __btf_name_by_offset(btf, member->name_off))) continue; /* Invalid BTF, two members with same name */ if (n) @@ -3626,9 +3627,9 @@ static int btf_parse_list_head(const struct btf *btf, struct btf_field *field, if (offset % __alignof__(struct bpf_list_node)) return -EINVAL; - field->list_head.btf = (struct btf *)btf; - field->list_head.value_btf_id = info->list_head.value_btf_id; - field->list_head.node_offset = offset; + field->datastructure_head.btf = (struct btf *)btf; + field->datastructure_head.value_btf_id = info->datastructure_head.value_btf_id; + field->datastructure_head.node_offset = offset; } if (!n) return -ENOENT; @@ -3735,11 +3736,11 @@ int btf_check_and_fixup_fields(const struct btf *btf, struct btf_record *rec) if (!(rec->fields[i].type & BPF_LIST_HEAD)) continue; - btf_id = rec->fields[i].list_head.value_btf_id; + btf_id = rec->fields[i].datastructure_head.value_btf_id; meta = btf_find_struct_meta(btf, btf_id); if (!meta) return -EFAULT; - rec->fields[i].list_head.value_rec = meta->record; + rec->fields[i].datastructure_head.value_rec = meta->record; if (!(rec->field_mask & BPF_LIST_NODE)) continue; diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index cca642358e80..6c67740222c2 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1737,12 +1737,12 @@ void bpf_list_head_free(const struct btf_field *field, void *list_head, while (head != orig_head) { void *obj = head; - obj -= field->list_head.node_offset; + obj -= field->datastructure_head.node_offset; head = head->next; /* The contained type can also have resources, including a * bpf_list_head which needs to be freed. */ - bpf_obj_free_fields(field->list_head.value_rec, obj); + bpf_obj_free_fields(field->datastructure_head.value_rec, obj); /* bpf_mem_free requires migrate_disable(), since we can be * called from map free path as well apart from BPF program (as * part of map ops doing bpf_obj_free_fields). diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 6f0aac837d77..bc80b4c4377b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8615,21 +8615,22 @@ static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env, field = meta->arg_list_head.field; - et = btf_type_by_id(field->list_head.btf, field->list_head.value_btf_id); + et = btf_type_by_id(field->datastructure_head.btf, field->datastructure_head.value_btf_id); t = btf_type_by_id(reg->btf, reg->btf_id); - if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->list_head.btf, - field->list_head.value_btf_id, true)) { + if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->datastructure_head.btf, + field->datastructure_head.value_btf_id, true)) { verbose(env, "operation on bpf_list_head expects arg#1 bpf_list_node at offset=%d " "in struct %s, but arg is at offset=%d in struct %s\n", - field->list_head.node_offset, btf_name_by_offset(field->list_head.btf, et->name_off), + field->datastructure_head.node_offset, + btf_name_by_offset(field->datastructure_head.btf, et->name_off), list_node_off, btf_name_by_offset(reg->btf, t->name_off)); return -EINVAL; } - if (list_node_off != field->list_head.node_offset) { + if (list_node_off != field->datastructure_head.node_offset) { verbose(env, "arg#1 offset=%d, but expected bpf_list_node at offset=%d in struct %s\n", - list_node_off, field->list_head.node_offset, - btf_name_by_offset(field->list_head.btf, et->name_off)); + list_node_off, field->datastructure_head.node_offset, + btf_name_by_offset(field->datastructure_head.btf, et->name_off)); return -EINVAL; } /* Set arg#1 for expiration after unlock */ @@ -9078,9 +9079,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, mark_reg_known_zero(env, regs, BPF_REG_0); regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC; - regs[BPF_REG_0].btf = field->list_head.btf; - regs[BPF_REG_0].btf_id = field->list_head.value_btf_id; - regs[BPF_REG_0].off = field->list_head.node_offset; + regs[BPF_REG_0].btf = field->datastructure_head.btf; + regs[BPF_REG_0].btf_id = field->datastructure_head.value_btf_id; + regs[BPF_REG_0].off = field->datastructure_head.node_offset; } else if (meta.func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx]) { mark_reg_known_zero(env, regs, BPF_REG_0); regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_TRUSTED;
Many of the structs recently added to track field info for linked-list head are useful as-is for rbtree root. So let's do a mechanical renaming of list_head-related types and fields: include/linux/bpf.h: struct btf_field_list_head -> struct btf_field_datastructure_head list_head -> datastructure_head in struct btf_field union kernel/bpf/btf.c: list_head -> datastructure_head in struct btf_field_info This is a nonfunctional change, functionality to actually use these fields for rbtree will be added in further patches. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> --- include/linux/bpf.h | 4 ++-- kernel/bpf/btf.c | 21 +++++++++++---------- kernel/bpf/helpers.c | 4 ++-- kernel/bpf/verifier.c | 21 +++++++++++---------- 4 files changed, 26 insertions(+), 24 deletions(-)