Re: [PATCH dwarves v2 06/10] btf_encoder: switch to shared elf_functions table

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thursday, December 19th, 2024 at 6:58 AM, Jiri Olsa <olsajiri@xxxxxxxxx> wrote:

> 
> 
> On Fri, Dec 13, 2024 at 10:37:13PM +0000, Ihor Solodrai wrote:
> 
> SNIP
> 
> > @@ -2116,9 +2128,6 @@ int btf_encoder__encode(struct btf_encoder *encoder)
> > int err;
> > size_t shndx;
> > 
> > - /* for single-threaded case, saved funcs are added here */
> > - btf_encoder__add_saved_funcs(encoder);
> > -
> > for (shndx = 1; shndx < encoder->seccnt; shndx++)
> > if (gobuffer__size(&encoder->secinfo[shndx].secinfo))
> > btf_encoder__add_datasec(encoder, shndx);
> > @@ -2477,14 +2486,13 @@ struct btf_encoder *btf_encoder__new(struct cu *cu, const char *detached_filenam
> > goto out_delete;
> > }
> > 
> > - encoder->symtab = elf_symtab__new(NULL, cu->elf);
> > + encoder->functions = elf_functions__get(cu->elf);
> 
> 
> elf_functions__get should always return != NULL right? should we add assert call for that?
> 
> SNIP
> 
> > diff --git a/pahole.c b/pahole.c
> > index 17af0b4..eb2e71a 100644
> > --- a/pahole.c
> > +++ b/pahole.c
> > @@ -3185,13 +3185,16 @@ static int pahole_threads_collect(struct conf_load *conf, int nr_threads, void *
> > if (error)
> > goto out;
> > 
> > - btf_encoder__add_saved_funcs(btf_encoder);
> > + err = btf_encoder__add_saved_funcs(btf_encoder);
> > + if (err < 0)
> > + goto out;
> > +
> > for (i = 0; i < nr_threads; i++) {
> > /*
> > * Merge content of the btf instances of worker threads to the btf
> > * instance of the primary btf_encoder.
> > */
> > - if (!threads[i]->btf)
> > + if (!threads[i]->encoder || !threads[i]->btf)
> 
> 
> is this related to this change? seems like separate fix
> 
> > continue;
> > err = btf_encoder__add_encoder(btf_encoder, threads[i]->encoder);
> > if (err < 0)
> > @@ -3846,6 +3849,9 @@ try_sole_arg_as_class_names:
> > exit(1);
> > }
> > 
> > + if (conf_load.nr_jobs <= 1 || conf_load.reproducible_build)
> > + btf_encoder__add_saved_funcs(btf_encoder);
> 
> 
> should we check the return value here as well?
> 
> thanks,
> jirka

Jiri, thanks for review.

This patch is going to be removed in the next version of the series,
following a discussion with Eduard and Andrii [1].

If you're interested you can inspect WIP v3 branch on github [2].

I am still testing it, and plan to add a patch removing global
btf_encoders list. Other than that it's close to what I am going to
submit.

[1] https://lore.kernel.org/dwarves/C82bYTvJaV4bfT15o25EsBiUvFsj5eTlm17933Hvva76CXjIcu3gvpaOCWPgeZ8g3cZ-RMa8Vp0y1o_QMR2LhPB-LEUYfZCGuCfR_HvkIP8=@pm.me/
[2] https://github.com/theihor/dwarves/tree/v3.workers

> 
> > +
> > err = btf_encoder__encode(btf_encoder);
> > btf_encoder__delete(btf_encoder);
> > if (err) {
> > --
> > 2.47.1





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux