Re: [PATCH 1/5] core: Add method to get the vmlinux BTF filename, allow overriding it via env var

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

On Tue, Nov 19, 2024 at 10:13:32AM -0300, Arnaldo Carvalho de Melo wrote:
> On Tue, Nov 19, 2024 at 12:57:44PM +0000, Alan Maguire wrote:
> > On 18/11/2024 20:41, Arnaldo Carvalho de Melo wrote:
> > > In systems where BTF isn't available there were reports that the
> > > simplest pahole call, without any args, segfaults.
> 
> > > To have a proper test before fixing this problem, allow overriding the
> > > /sys/kernel/btf/vmlinux filename, so that even in systems with BTF we an
> > > point it to a invalid location, making pahole think that there is no BTF
> > > available and thus fallback to something that currently segfaults.
> 
> <SNIP>
> 
> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> 
> Thanks!

Ah, thanks, added to the cset,

- Arnaldo
  
> > Small thing - would it be worth dropping the _FILENAME suffix of the
> > envvar to just have PAHOLE_VMLINUX_BTF? One other thing below..
> 
> See below ends up being related to the other point you made.
>  
> > > +++ b/pahole.c
> > > @@ -3754,7 +3754,7 @@ try_sole_arg_as_class_names:
> > >  		if (filename &&
> > >  		    strstarts(filename, "/sys/kernel/btf/") &&
> > >  		    strstr(filename, "/vmlinux") == NULL) {
> > > -			base_btf_file = "/sys/kernel/btf/vmlinux";
> > > +			base_btf_file = vmlinux_path__btf_filename();
> > 
> > so in this case, we are specifying a module in /sys/kernel/btf and
> > implicitly we use /sys/kernel/btf/vmlinux as the base. I wonder if
> > there's much value for the envvar override here; I guess it might allow
> > us to test using a mismatched base vmlinux with split BTF?
> 
> Right, that would be possible with this patch as-is, which may be
> useful, we may want to have PAHOLE_VMLINUX_BTF_DIR as a counterpart to
> PAHOLE_VMLINUX_BTF_FILENAME, that would allow us to use some different
> directory with both vmlinux and modules.
> 
> Having both allows more flexibility, picking some arbitrary vmlinux
> plust some arbitrary directory with modules, etc. So I'd keep the patch
> as is and at some point add the PAHOLE_VMLINUX_BTF_DIR knob.
> 
> - Arnaldo
>  
> > >  			conf_load.base_btf = btf__parse(base_btf_file, NULL);
> > >  			if (libbpf_get_error(conf_load.base_btf)) {
> > >  				fprintf(stderr, "Failed to parse base BTF '%s': %ld\n",