Since multithreading was moved to dwarf_loader [1], pahole maintains a single global btf_encoder instance. However parts of btf_encoder.c exist to manage mulitple encoders, which is not necessary anymore. This patch removes encoder pointer from struct btf_encoder_func_state. We create a state for every encountered instance of a function in DWARF, and currently they carry around the encoder unnecessarily. [1] https://lore.kernel.org/all/20250109185950.653110-9-ihor.solodrai@pm.me/ Signed-off-by: Ihor Solodrai --- btf_encoder.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/btf_encoder.c b/btf_encoder.c index 03bc3c7..a6fdc58 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -79,7 +79,6 @@ struct btf_encoder_func_annot { /* state used to do later encoding of saved functions */ struct btf_encoder_func_state { - struct btf_encoder *encoder; struct elf_function *elf; uint32_t type_id_off; uint16_t nr_parms; @@ -819,7 +818,7 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f struct btf_encoder_func_state *state) { const struct btf_type *t; - struct btf *btf; + struct btf *btf = encoder->btf; struct parameter *param; uint16_t nr_params, param_idx; int32_t id, type_id; @@ -834,12 +833,9 @@ static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct f /* add btf_type for func_proto */ if (ftype) { - btf = encoder->btf; nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0); type_id = btf_encoder__tag_type(encoder, ftype->tag.type); } else if (state) { - encoder = state->encoder; - btf = state->encoder->btf; nr_params = state->nr_parms; type_id = state->ret_type_id; } else { @@ -1123,13 +1119,12 @@ static bool types__match(struct btf_encoder *encoder, return false; } -static bool funcs__match(struct btf_encoder_func_state *s1, +static bool funcs__match(struct btf_encoder *encoder, + struct btf_encoder_func_state *s1, struct btf_encoder_func_state *s2) { - struct btf_encoder *encoder = s1->encoder; struct elf_function *func = s1->elf; - struct btf *btf1 = s1->encoder->btf; - struct btf *btf2 = s2->encoder->btf; + struct btf *btf = encoder->btf; uint8_t i; if (s1->nr_parms != s2->nr_parms) { @@ -1138,7 +1133,7 @@ static bool funcs__match(struct btf_encoder_func_state *s1, s1->nr_parms, s2->nr_parms); return false; } - if (!types__match(encoder, btf1, s1->ret_type_id, btf2, s2->ret_type_id)) { + if (!types__match(encoder, btf, s1->ret_type_id, btf, s2->ret_type_id)) { btf_encoder__log_func_skip(encoder, func, "return type mismatch\n"); return false; } @@ -1146,11 +1141,11 @@ static bool funcs__match(struct btf_encoder_func_state *s1, return true; for (i = 0; i < s1->nr_parms; i++) { - if (!types__match(encoder, btf1, s1->parms[i].type_id, - btf2, s2->parms[i].type_id)) { + if (!types__match(encoder, btf, s1->parms[i].type_id, + btf, s2->parms[i].type_id)) { if (encoder->verbose) { - const char *p1 = btf__name_by_offset(btf1, s1->parms[i].name_off); - const char *p2 = btf__name_by_offset(btf2, s2->parms[i].name_off); + const char *p1 = btf__name_by_offset(btf, s1->parms[i].name_off); + const char *p2 = btf__name_by_offset(btf, s2->parms[i].name_off); btf_encoder__log_func_skip(encoder, func, "param type mismatch for param#%d %s %s %s\n", @@ -1244,7 +1239,6 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi if (!state) return -ENOMEM; - state->encoder = encoder; state->elf = func; state->nr_parms = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0); state->ret_type_id = ftype->tag.type == 0 ? 0 : encoder->type_id_off + ftype->tag.type; @@ -1410,7 +1404,9 @@ static int saved_functions_cmp(const void *_a, const void *_b) return elf_function__name_cmp(a->elf, b->elf); } -static int saved_functions_combine(struct btf_encoder_func_state *a, struct btf_encoder_func_state *b) +static int saved_functions_combine(struct btf_encoder *encoder, + struct btf_encoder_func_state *a, + struct btf_encoder_func_state *b) { uint8_t optimized, unexpected, inconsistent, uncertain_parm_loc; @@ -1421,7 +1417,7 @@ static int saved_functions_combine(struct btf_encoder_func_state *a, struct btf_ unexpected = a->unexpected_reg | b->unexpected_reg; inconsistent = a->inconsistent_proto | b->inconsistent_proto; uncertain_parm_loc = a->uncertain_parm_loc | b->uncertain_parm_loc; - if (!unexpected && !inconsistent && !funcs__match(a, b)) + if (!unexpected && !inconsistent && !funcs__match(encoder, a, b)) inconsistent = 1; a->optimized_parms = b->optimized_parms = optimized; a->unexpected_reg = b->unexpected_reg = unexpected; @@ -1471,7 +1467,7 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e */ j = i + 1; - while (j < nr_saved_fns && saved_functions_combine(&saved_fns[i], &saved_fns[j]) == 0) + while (j < nr_saved_fns && saved_functions_combine(encoder, &saved_fns[i], &saved_fns[j]) == 0) j++; /* do not exclude functions with optimized-out parameters; they @@ -1488,7 +1484,7 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e 0, 0); if (add_to_btf) { - err = btf_encoder__add_func(state->encoder, state); + err = btf_encoder__add_func(encoder, state); if (err < 0) goto out; } -- 2.51.1 btf_encoder__add_func_proto() essentially implements two independent code paths depending on input arguments: one for struct ftype and the other for struct btf_encoder_func_state. Split btf_encoder__add_func_proto() into two variants: * btf_encoder__add_func_proto_for_ftype() * btf_encoder__add_func_proto_for_state() And factor out common btf_encoder__emit_func_proto() subroutine. No functional changes. Signed-off-by: Ihor Solodrai --- btf_encoder.c | 135 +++++++++++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 56 deletions(-) diff --git a/btf_encoder.c b/btf_encoder.c index a6fdc58..bdda7d0 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -814,79 +814,102 @@ static inline bool is_kfunc_state(struct btf_encoder_func_state *state) return state && state->elf && state->elf->kfunc; } -static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype, - struct btf_encoder_func_state *state) +static int32_t btf_encoder__emit_func_proto(struct btf_encoder *encoder, + uint32_t type_id, + uint16_t nr_params) { const struct btf_type *t; - struct btf *btf = encoder->btf; - struct parameter *param; + uint32_t ret; + + ret = btf__add_func_proto(encoder->btf, type_id); + if (ret > 0) { + t = btf__type_by_id(encoder->btf, ret); + btf_encoder__log_type(encoder, t, false, false, + "return=%u args=(%s", t->type, !nr_params ? "void)\n" : ""); + } else { + btf__log_err(encoder->btf, BTF_KIND_FUNC_PROTO, NULL, true, ret, + "return=%u vlen=%u Error emitting BTF type", + type_id, nr_params); + } + + return ret; +} + +static int32_t btf_encoder__add_func_proto_for_ftype(struct btf_encoder *encoder, + struct ftype *ftype) +{ uint16_t nr_params, param_idx; + struct parameter *param; int32_t id, type_id; - char tmp_name[KSYM_NAME_LEN]; const char *name; - assert(ftype != NULL || state != NULL); - - if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes) - if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0) - return -1; + assert(ftype != NULL); /* add btf_type for func_proto */ - if (ftype) { - nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0); - type_id = btf_encoder__tag_type(encoder, ftype->tag.type); - } else if (state) { - nr_params = state->nr_parms; - type_id = state->ret_type_id; - } else { - return 0; - } + nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0); + type_id = btf_encoder__tag_type(encoder, ftype->tag.type); - id = btf__add_func_proto(btf, type_id); - if (id > 0) { - t = btf__type_by_id(btf, id); - btf_encoder__log_type(encoder, t, false, false, "return=%u args=(%s", t->type, !nr_params ? "void)\n" : ""); - } else { - btf__log_err(btf, BTF_KIND_FUNC_PROTO, NULL, true, id, - "return=%u vlen=%u Error emitting BTF type", - type_id, nr_params); + id = btf_encoder__emit_func_proto(encoder, type_id, nr_params); + if (id < 0) return id; - } /* add parameters */ param_idx = 0; - if (ftype) { - ftype__for_each_parameter(ftype, param) { - const char *name = parameter__name(param); - - type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type; - ++param_idx; - if (btf_encoder__add_func_param(encoder, name, type_id, - param_idx == nr_params)) - return -1; - } + ftype__for_each_parameter(ftype, param) { + name = parameter__name(param); + type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type; ++param_idx; - if (ftype->unspec_parms) - if (btf_encoder__add_func_param(encoder, NULL, 0, - param_idx == nr_params)) - return -1; - } else { - for (param_idx = 0; param_idx < nr_params; param_idx++) { - struct btf_encoder_func_parm *p = &state->parms[param_idx]; + if (btf_encoder__add_func_param(encoder, name, type_id, param_idx == nr_params)) + return -1; + } - name = btf__name_by_offset(btf, p->name_off); + ++param_idx; + if (ftype->unspec_parms) + if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params)) + return -1; - /* adding BTF data may result in a move of the - * name string memory, so make a temporary copy. - */ - strncpy(tmp_name, name, sizeof(tmp_name) - 1); + return id; +} - if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id, - param_idx == nr_params)) - return -1; - } +static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder, + struct btf_encoder_func_state *state) +{ + const struct btf *btf = encoder->btf; + struct btf_encoder_func_parm *p; + uint16_t nr_params, param_idx; + char tmp_name[KSYM_NAME_LEN]; + int32_t id, type_id; + const char *name; + bool is_last; + + /* Beware: btf__add_bpf_arena_type_tags may change some members of the state */ + if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes) + if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0) + return -1; + + type_id = state->ret_type_id; + nr_params = state->nr_parms; + + id = btf_encoder__emit_func_proto(encoder, type_id, nr_params); + if (id < 0) + return id; + + /* add parameters */ + for (param_idx = 0; param_idx < nr_params; param_idx++) { + p = &state->parms[param_idx]; + name = btf__name_by_offset(btf, p->name_off); + is_last = param_idx == nr_params; + + /* adding BTF data may result in a move of the + * name string memory, so make a temporary copy. + */ + strncpy(tmp_name, name, sizeof(tmp_name) - 1); + + if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id, is_last)) + return -1; } + return id; } @@ -1343,7 +1366,7 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, uint16_t idx; int err; - btf_fnproto_id = btf_encoder__add_func_proto(encoder, NULL, state); + btf_fnproto_id = btf_encoder__add_func_proto_for_state(encoder, state); name = func->name; if (btf_fnproto_id >= 0) btf_fn_id = btf_encoder__add_ref_type(encoder, BTF_KIND_FUNC, btf_fnproto_id, @@ -1682,7 +1705,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag, case DW_TAG_enumeration_type: return btf_encoder__add_enum_type(encoder, tag, conf_load); case DW_TAG_subroutine_type: - return btf_encoder__add_func_proto(encoder, tag__ftype(tag), NULL); + return btf_encoder__add_func_proto_for_ftype(encoder, tag__ftype(tag)); case DW_TAG_unspecified_type: /* Just don't encode this for now, converting anything with this type to void (0) instead. * -- 2.51.1 Generating BTF for BPF kernel functions requires special handling. Consolidate this behavior into btf_encoder__add_bpf_kfunc(), which uses simplified btf_encoder_add_func(). No functional changes. Signed-off-by: Ihor Solodrai --- btf_encoder.c | 49 ++++++++++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/btf_encoder.c b/btf_encoder.c index bdda7d0..0bb7831 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -773,6 +773,7 @@ static int btf__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state return id; } +/* Modifies state->ret_type_id and state->parms[i].type_id for flagged kfuncs */ static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func_state *state) { uint32_t flags = state->elf->kfunc_flags; @@ -883,11 +884,6 @@ static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder const char *name; bool is_last; - /* Beware: btf__add_bpf_arena_type_tags may change some members of the state */ - if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes) - if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0) - return -1; - type_id = state->ret_type_id; nr_params = state->nr_parms; @@ -1357,14 +1353,13 @@ static int btf__tag_kfunc(struct btf *btf, struct elf_function *kfunc, __u32 btf static int32_t btf_encoder__add_func(struct btf_encoder *encoder, struct btf_encoder_func_state *state) { - struct elf_function *func = state->elf; int btf_fnproto_id, btf_fn_id, tag_type_id = 0; + struct elf_function *func = state->elf; + char tmp_value[KSYM_NAME_LEN]; int16_t component_idx = -1; - const char *name; const char *value; - char tmp_value[KSYM_NAME_LEN]; + const char *name; uint16_t idx; - int err; btf_fnproto_id = btf_encoder__add_func_proto_for_state(encoder, state); name = func->name; @@ -1377,15 +1372,6 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, return -1; } - if (func->kfunc && encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) { - err = btf__tag_kfunc(encoder->btf, func, btf_fn_id); - if (err < 0) - return err; - } - - if (state->nr_annots == 0) - return 0; - for (idx = 0; idx < state->nr_annots; idx++) { struct btf_encoder_func_annot *a = &state->annots[idx]; @@ -1408,6 +1394,28 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder, return -1; } + return btf_fn_id; +} + +static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder, + struct btf_encoder_func_state *state) +{ + int btf_fn_id, err; + + if (encoder->tag_kfuncs && encoder->encode_attributes) + if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0) + return -1; + + btf_fn_id = btf_encoder__add_func(encoder, state); + if (btf_fn_id < 0) + return -1; + + if (encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) { + err = btf__tag_kfunc(encoder->btf, state->elf, btf_fn_id); + if (err < 0) + return -1; + } + return 0; } @@ -1507,7 +1515,10 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e 0, 0); if (add_to_btf) { - err = btf_encoder__add_func(encoder, state); + if (is_kfunc_state(state)) + err = btf_encoder__add_bpf_kfunc(encoder, state); + else + err = btf_encoder__add_func(encoder, state); if (err < 0) goto out; } -- 2.51.1