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@xxxxxxxxxx> 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.
