Re: [PATCH 2/5] tests default_vmlinux_btf: Introduce test for using BTF by default

Arnaldo Carvalho de Melo
2024-11-19T19:49:10Z

On Tue, Nov 19, 2024 at 10:35:35AM -0800, Eduard Zingerman wrote:
> On Mon, 2024-11-18 at 17:41 -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > On a system without any debugging info or when one specifies BTF as the
> > only BTF info desired but no BTF is available (or invalidated using
> > PAHOLE_VMLINUX_BTF_FILENAME to an invalid/non-existent file), we're
> > getting a segfault:
> >
> > root@x1:/home/acme/git/pahole# export PAHOLE_VMLINUX_BTF_FILENAME=non-existent
> > root@x1:/home/acme/git/pahole# pahole --running_kernel_vmlinux
> > pahole: couldn't find a vmlinux that matches the running kernel
> > HINT: Maybe you're inside a container or missing a debuginfo package?
> > root@x1:/home/acme/git/pahole# pahole
> > Segmentation fault (core dumped)
> > root@x1:/home/acme/git/pahole#
> >
> > So add a test that checks for that before we fix it:
> >
> > root@x1:/home/acme/git/pahole# tests/default_vmlinux_btf.sh
> > Default BTF on a system without BTF: FAILED
> > root@x1:/home/acme/git/pahole#
> >
> > Reported-by: Matthias Schwarzott <zzam@gentoo.org>
> > Cc: Alan Maguire <alan.maguire@oracle.com>
> > Cc: Andrii Nakryiko <andrii@kernel.org>
> > Cc: Eduard Zingerman <eddyz87@gmail.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Song Liu <song@kernel.org>
> > Cc: Yonghong Song <yonghong.song@linux.dev>
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
>
> Lgtm.
> Would it make sense to move this patch to the end of the series?
> In case someone does a bisect and runs the tests to find some regression.

Humm?

So you think it should be introduced only when it passes? I.e. when the
problem is fixed?

My practice so far has been to reproduce the problem manually, write a
test, show that it detects the problem, fix, then show that the
regression test shows that the problem is not present anymore.

I see your point about a bisection when running in the cset that
introduces the test case and in all before the fix is added will fail,
confusing the bisector or not allowing it to be automated :-\

So, yeah, probably, for automated bisection we should move it to after
the fix, when finishing the devel cycle, which is now, will do.

If someone disagrees, please holer.

- Arnaldo