Re: [PATCH dwarves v2 4/4] btf_encoder: add global_var feature to encode globals

[Arnaldo Carvalho de Melo <acme@kernel.org>]

On Tue, Oct 01, 2024 at 10:13:29AM -0700, Andrii Nakryiko wrote:
> On Tue, Oct 1, 2024 at 8:10 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > On Fri, Sep 20, 2024 at 01:19:01AM -0700, Stephen Brennan wrote:
> > > Currently the "var" feature only encodes percpu variables. Add the
> > > ability to encode all global variables.
> > >
> > > This also drops the use of the symbol table to find variable names and
> > > compare them against DWARF names. We simply rely on the DWARF
> > > information to give us all the variables.
> >
> > I applied the three first patches to the next branch that soon will move
> > to master, but the last patch I think does too many things and ends up
> > being too big.
> >
> > For instance, you could have done the btf_encoder->skip_encoding_vars
> > transformation into a bitfield in a separate, prep patch, also you
> > mentions "this also drops the use of the symbol table", can this be made
> > a separate, prep patch?
> >
> > There was a conflict with some new options I added (--padding,
> > --padding_ge) and I fixed that up and made the series available in the
> > btf_global_vars branch, can you please go from there and split the last
> > patch into smaller chunks?
> >
> > Thanks for your work on this! I noticed that this is not the default,
> > i.e. one has to explicitely opt in to have the global variables encoded
> > in BTF, so that would be interesting to have spelled out in the chunked
> > out patch that introduces the feature, etc.
> 
> We probably shouldn't enable this option in kernel build until we work
> out details of loading vmlinux BTF(s) through the kernel module.

Sure, this should be completely opt-in, and for kernel features, even
for documentational purposes, we need to enable it via --btf_features in
the Kbuild files, etc.

But with the feature in pahole we can go on experimenting with it, etc.

- Arnaldo
 
> > Also since we have it as a feature and can ask for global variables
> > using --btf_features=global_var, I don't think we need
> > --encode_btf_global_vars, right?

> > That will also make the patch smaller, and even if it was required, that
> > would be something to have in a separate patch.