On Thu, 2025-02-06 at 18:14 -0800, Ihor Solodrai wrote: > From: Ihor Solodrai <ihor.solodrai@xxxxx> > > btf_encoder__tag_kfuncs() is a post-processing step of BTF encoding, > executed right before BTF is deduped and dumped to the output. > > Split btf_encoder__tag_kfuncs() routine in two parts: > * btf_encoder__collect_kfuncs() > * btf_encoder__tag_kfuncs() > > btf_encoder__collect_kfuncs() reads the .BTF_ids section of the ELF, > collecting kfunc information into a list of kfunc_info structs in the > btf_encoder. It is executed in btf_encoder__new() when tag_kfuncs flag > is set. This way kfunc information is available during entire lifetime > of the btf_encoder. > > btf_encoder__tag_kfuncs() is basically the same: collect BTF > functions, and then for each kfunc find and tag correspoding BTF > func. Except now kfunc information is not collected in-place, but is > simply read from the btf_encoder. > > Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxxxxxx> > --- Tbh, I don't think this split is necessary, modifying btf_type in-place should be fine (and libbpf does it at-least in one place). E.g. like here: https://github.com/acmel/dwarves/compare/master...eddyz87:dwarves:arena-attrs-no-split I like it because it keeps the change a bit more contained, but I do not insist. [...] > @@ -1876,11 +1886,10 @@ static int btf_encoder__tag_kfunc(struct btf_encoder *encoder, struct gobuffer * > return 0; > } > > -static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > +static int btf_encoder__collect_kfuncs(struct btf_encoder *encoder) > { > const char *filename = encoder->source_filename; > struct gobuffer btf_kfunc_ranges = {}; > - struct gobuffer btf_funcs = {}; > Elf_Data *symbols = NULL; > Elf_Data *idlist = NULL; > Elf_Scn *symscn = NULL; > @@ -1897,6 +1906,8 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > int nr_syms; > int i = 0; > > + INIT_LIST_HEAD(&encoder->kfuncs); > + Nit: do this in the btf_encoder__new? > fd = open(filename, O_RDONLY); > if (fd < 0) { > fprintf(stderr, "Cannot open %s\n", filename); > @@ -1977,12 +1988,6 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > } > nr_syms = shdr.sh_size / shdr.sh_entsize; > > - err = btf_encoder__collect_btf_funcs(encoder, &btf_funcs); > - if (err) { > - fprintf(stderr, "%s: failed to collect BTF funcs\n", __func__); > - goto out; > - } > - > /* First collect all kfunc set ranges. > * > * Note we choose not to sort these ranges and accept a linear > @@ -2015,12 +2020,13 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > for (i = 0; i < nr_syms; i++) { > const struct btf_kfunc_set_range *ranges; > const struct btf_id_and_flag *pair; > + struct elf_function *elf_fn; > + struct kfunc_info *kfunc; > unsigned int ranges_cnt; > char *func, *name; > ptrdiff_t off; > GElf_Sym sym; > bool found; > - int err; > int j; > > if (!gelf_getsym(symbols, i, &sym)) { > @@ -2061,18 +2067,26 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder) > continue; > } > > - err = btf_encoder__tag_kfunc(encoder, &btf_funcs, func, pair->flags); > - if (err) { > - fprintf(stderr, "%s: failed to tag kfunc '%s'\n", __func__, func); > - free(func); > + elf_fn = btf_encoder__find_function(encoder, func, 0); > + free(func); > + if (!elf_fn) > + continue; > + elf_fn->kfunc = true; > + > + kfunc = calloc(1, sizeof(*kfunc)); > + if (!kfunc) { > + fprintf(stderr, "%s: failed to allocate memory for kfunc info\n", __func__); > + err = -ENOMEM; > goto out; > } > - free(func); > + kfunc->id = pair->id; > + kfunc->flags = pair->flags; > + kfunc->name = elf_fn->name; If we do go with split, maybe make refactoring a bit more drastic and merge kfunc_info with elf_function? This would make maintaining a separate encoder->kfuncs list unnecessary. Also, can get rid of separate 'struct gobuffer *funcs'. E.g. see my commit on top of yours: https://github.com/acmel/dwarves/compare/master...eddyz87:dwarves:arena-attrs-merge-kfunc-info > + list_add(&kfunc->node, &encoder->kfuncs); > } > > err = 0; > out: > - __gobuffer__delete(&btf_funcs); > __gobuffer__delete(&btf_kfunc_ranges); > if (elf) > elf_end(elf); > @@ -2081,6 +2095,34 @@ out: > return err; > } > [...]
