On Thu, Oct 17, 2024 at 11:56:04AM +0100, Alan Maguire wrote:
> On 16/10/2024 01:10, Ihor Solodrai wrote:
> > Introduce a global elf_functions_list variable in btf_encoder.c that
> > contains an elf_functions per ELF.
> >
>
> Arnaldo can help provide context here, but at least notionally I think
> the idea of maintaining libdwarves as a library has value. In that
Yeah, but in all these years I'm not aware of any user, I even need to
do some testing on building pahole statically with libdwarves to see if
we get performance improvements...
> context, avoiding global lists where possible is a good thing I think,
Even without a library :-)
> since if it was used as a library, multiple invokations could confuse
> the elf_functions list. To that end, can we make the elf_functions_list
> a field in the conf_load perhaps? It already contains base_btf so there
conf_load looks with the structs we have now, so I would try to start
there.
- Arnaldo
> is a precedent for storing data relevant to all encoders there, and
> btf_encoder__new() has conf_load as a parameter, so the elf functions
> list can still always be retrieved on encoder creation. In addition the
> parameter to cus__process_dwflmod() has the parms structure which
> contains the conf_load; you'd just need to pass that through to your
> pre_load_module() callback I think.
>
> It shouldn't be a massive change but I think it would be worthwhile.
>
> Thanks!
>
> > An elf_functions structure is allocated and filled out by
> > btf_encoder__pre_load_module() hook, and the list is cleared after
> > btf_encoder__encode() is done.
> >
> > At this point btf_encoders don't use shared elf_functions yet (each
> > maintains their own copy as before), but it is built before encoders
> > are initialized.
> >
> > Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxx>
> > ---
> > btf_encoder.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > btf_encoder.h | 2 ++
> > pahole.c | 3 +++
> > 3 files changed, 71 insertions(+)
> >
> > diff --git a/btf_encoder.c b/btf_encoder.c
> > index 9c840fa..8e8fd05 100644
> > --- a/btf_encoder.c
> > +++ b/btf_encoder.c
> > @@ -103,6 +103,8 @@ struct elf_secinfo {
> > };
> >
> > struct elf_functions {
> > + struct list_head node; /* for elf_functions_list */
> > + Elf *elf; /* source ELF */
> > struct elf_symtab *symtab;
> > struct elf_function *entries;
> > int cnt;
> > @@ -147,6 +149,67 @@ struct btf_kfunc_set_range {
> > uint64_t end;
> > };
> >
> > +
> > +/* In principle, multiple ELFs can be processed in one pahole run,
> > + * so we have to store elf_functions table per ELF.
> > + * An element is added to the list on btf_encoder__pre_load_module,
> > + * and removed after btf_encoder__encode is done.
> > + */
> > +static LIST_HEAD(elf_functions_list);
> > +
> > +static inline void elf_functions__delete(struct elf_functions *funcs)
> > +{
> > + free(funcs->entries);
> > + elf_symtab__delete(funcs->symtab);
> > + list_del(&funcs->node);
> > + free(funcs);
> > +}
> > +
> > +static inline void elf_functions__delete_all(void)
> > +{
> > + struct list_head *pos, *tmp;
> > +
> > + list_for_each_safe(pos, tmp, &elf_functions_list) {
> > + struct elf_functions *funcs = list_entry(pos, struct elf_functions, node);
> > +
> > + elf_functions__delete(funcs);
> > + }
> > +}
> > +
> > +static int elf_functions__collect(struct elf_functions *functions);
> > +
> > +int btf_encoder__pre_load_module(Dwfl_Module *mod, Elf *elf)
> > +{
> > + struct elf_functions *funcs;
> > + int err;
> > +
> > + funcs = calloc(1, sizeof(*funcs));
> > + if (!funcs) {
> > + err = -ENOMEM;
> > + goto out_delete;
> > + }
> > +
> > + funcs->symtab = elf_symtab__new(NULL, elf);
> > + if (!funcs->symtab) {
> > + err = -1;
> > + goto out_delete;
> > + }
> > +
> > + funcs->elf = elf;
> > + err = elf_functions__collect(funcs);
> > + if (err)
> > + goto out_delete;
> > +
> > + list_add_tail(&funcs->node, &elf_functions_list);
> > +
> > + return 0;
> > +
> > +out_delete:
> > + elf_functions__delete(funcs);
> > + return err;
> > +}
> > +
> > +
> > static LIST_HEAD(encoders);
> > static pthread_mutex_t encoders__lock = PTHREAD_MUTEX_INITIALIZER;
> >
> > @@ -2071,6 +2134,8 @@ int btf_encoder__encode(struct btf_encoder *encoder)
> > #endif
> > err = btf_encoder__write_elf(encoder, encoder->btf, BTF_ELF_SEC);
> > }
> > +
> > + elf_functions__delete_all();
> > return err;
> > }
> >
> > @@ -2387,6 +2452,7 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
> > goto out;
> > }
> > encoder->functions.symtab = encoder->symtab;
> > + encoder->functions.elf = cu->elf;
> >
> > /* index the ELF sections for later lookup */
> >
> > diff --git a/btf_encoder.h b/btf_encoder.h
> > index 824963b..7debd67 100644
> > --- a/btf_encoder.h
> > +++ b/btf_encoder.h
> > @@ -34,4 +34,6 @@ struct btf *btf_encoder__btf(struct btf_encoder *encoder);
> >
> > int btf_encoder__add_encoder(struct btf_encoder *encoder, struct btf_encoder *other);
> >
> > +int btf_encoder__pre_load_module(Dwfl_Module *mod, Elf *elf);
> > +
> > #endif /* _BTF_ENCODER_H_ */
> > diff --git a/pahole.c b/pahole.c
> > index b9e97ef..891af3a 100644
> > --- a/pahole.c
> > +++ b/pahole.c
> > @@ -3814,6 +3814,9 @@ int main(int argc, char *argv[])
> > conf_load.threads_collect = pahole_threads_collect;
> > }
> >
> > + if (btf_encode)
> > + conf_load.pre_load_module = btf_encoder__pre_load_module;
> > +
> > // Make 'pahole --header type < file' a shorter form of 'pahole -C type --count 1 < file'
> > if (conf.header_type && !class_name && prettify_input) {
> > conf.count = 1;
