Re: [PATCH] perf test bpf: Skip test if kernel-debuginfo is not present

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

Em Mon, Jan 02, 2023 at 07:12:50PM +0530, Athira Rajeev escreveu:
> > On 14-Dec-2022, at 9:21 PM, Arnaldo Carvalho de Melo <acme@kernel.org <mailto:acme@kernel.org>> wrote:
> > Em Tue, Dec 13, 2022 at 03:21:03PM +0530, Athira Rajeev escreveu:
> >>> On 13-Dec-2022, at 12:27 AM, Arnaldo Carvalho de Melo <acme@kernel.org <mailto:acme@kernel.org>> wrote:
> >>> Em Thu, Nov 03, 2022 at 12:27:01PM +0530, Athira Rajeev escreveu:
> >>>>> On 28-Oct-2022, at 9:12 PM, Kajol Jain <kjain@linux.ibm.com <mailto:kjain@linux.ibm.com>> wrote:
> >>>>> 
> >>>>> Perf BPF filter test fails in environment where "kernel-debuginfo"
> >>>>> is not installed.
> >>>>> 
> >>>>> Test failure logs:
> >>>>> <<>>
> >>>>> 42: BPF filter                            :
> >>>>> 42.1: Basic BPF filtering                 : Ok
> >>>>> 42.2: BPF pinning                         : Ok
> >>>>> 42.3: BPF prologue generation             : FAILED!
> >>>>> <<>>
> >>>>> 
> >>>>> Enabling verbose option provided debug logs, which says debuginfo
> >>>>> needs to be installed. Snippet of verbose logs:
> >>>>> 
> >>>>> <<>>
> >>>>> 42.3: BPF prologue generation                                       :
> >>>>> --- start ---
> >>>>> test child forked, pid 28218
> >>>>> <<>>
> >>>>> Rebuild with CONFIG_DEBUG_INFO=y, or install an appropriate debuginfo
> >>>>> package.
> >>>>> bpf_probe: failed to convert perf probe events
> >>>>> Failed to add events selected by BPF
> >>>>> test child finished with -1
> >>>>> ---- end ----
> >>>>> BPF filter subtest 3: FAILED!
> >>>>> <<>>
> >>>>> 
> >>>>> Here subtest, "BPF prologue generation" failed and
> >>>>> logs shows debuginfo is needed. After installing
> >>>>> kernel-debuginfo package, testcase passes.
> >>>>> 
> >>>>> Subtest "BPF prologue generation" failed because, the "do_test"
> >>>>> function returns "TEST_FAIL" without checking the error type
> >>>>> returned by "parse_events_load_bpf_obj" function.
> >>>>> Function parse_events_load_bpf_obj can also return error of type
> >>>>> "-ENOENT" incase kernel-debuginfo package is not installed. Fix this
> >>>>> by adding check for -ENOENT error.
> >>>>> 
> >>>>> Test result after the patch changes:
> >>>>> 
> >>>>> Test failure logs:
> >>>>> <<>>
> >>>>> 42: BPF filter                 :
> >>>>> 42.1: Basic BPF filtering      : Ok
> >>>>> 42.2: BPF pinning              : Ok
> >>>>> 42.3: BPF prologue generation  : Skip (clang/debuginfo isn't
> >>>>> installed or environment missing BPF support)
> >>>>> 
> >>>>> Fixes: ba1fae431e74bb42 ("perf test: Add 'perf test BPF'")
> >>>>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com <mailto:kjain@linux.ibm.com>>
> >>>>> Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com <mailto:maddy@linux.ibm.com>>
> >>>>> ---
> >>>>> tools/perf/tests/bpf.c | 6 +++++-
> >>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
> >>>>> 
> >>>>> diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c
> >>>>> index 17c023823713..57cecadc1da2 100644
> >>>>> --- a/tools/perf/tests/bpf.c
> >>>>> +++ b/tools/perf/tests/bpf.c
> >>>>> @@ -126,6 +126,10 @@ static int do_test(struct bpf_object *obj, int (*func)(void),
> >>>>> 
> >>>>> 	err = parse_events_load_bpf_obj(&parse_state, &parse_state.list, obj, NULL);
> >>>>> 	parse_events_error__exit(&parse_error);
> >>>>> +	if (err == -ENOENT) {
> >>>>> +		pr_debug("Failed to add events selected by BPF, debuginfo package not installed\n");
> >>>>> +		return TEST_SKIP;
> >>>>> +	}
> >>>> 
> >>>> Hi Kajol,
> >>>> 
> >>>> Here, you have used ENOENT to skip the test. But there could be other places in the code path for “parse_events_load_bpf_obj”
> >>>> which also returns ENOENT. In that case, for any exit that returns ENOENT, test will get skipped.
> >>>> 
> >>>> Can we look at the logs, example we have this in commit logs:
> >>>> 
> >>>> 	Rebuild with CONFIG_DEBUG_INFO=y, or install an appropriate debuginfo
> >>>> 	package.
> >>>> 
> >>>> so as to decide whether to skip for debug info ?
> >>> 
> >>> Kajol?
> > 
> >> Hi Arnaldo, looking for your suggestion on how to handle the case where debuginfo is missing.
> > 
> >> Here the bpf test fails because of missing debuginfo. The function
> >> which goes through the debuginfo check is "parse_events_load_bpf_obj"
> >> . parse_events_load_bpf_obj internally calls "open_debuginfo" which
> >> returns ENOENT when debuginfo is missing. The patch fix from Kajol is
> >> to skip the test using error code ENOENT for debuginfo.
> > 
> > Lets see:
> > 
> > root@roc-rk3399-pc:~# uname -a
> > Linux roc-rk3399-pc 6.1.0-rc5-00123-g4dd7ff4a0311 #2 SMP PREEMPT Wed Nov 16 19:55:11 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
> > root@roc-rk3399-pc:~# perf probe -x ~/bin/perf open_debuginfo
> > Added new event:
> > probe_perf:open_debuginfo (on open_debuginfo in /home/acme/bin/perf)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe_perf:open_debuginfo -aR sleep 1
> > 
> > root@roc-rk3399-pc:~#
> > 
> > root@roc-rk3399-pc:~# perf trace --call-graph=dwarf -a -e probe_perf:* perf test bpf
> > 40: LLVM search and compile                                         :
> > 40.1: Basic BPF llvm compile                                        : Ok
> > 40.3: Compile source for BPF prologue generation                    : FAILED!
> > 40.4: Compile source for BPF relocation                             : Ok
> > 42: BPF filter                                                      :
> > 42.1: Basic BPF filtering                                           :     0.000 perf/38363 probe_perf:open_debuginfo(__probe_ip: 187650778659428)
> >                                     open_debuginfo (/home/acme/bin/perf)
> >                                     try_to_find_probe_trace_events (inlined)
> >                                     convert_to_probe_trace_events (inlined)
> >                                     convert_perf_probe_events (/home/acme/bin/perf)
> >                                     bpf__probe (/home/acme/bin/perf)
> >                                     parse_events_load_bpf_obj (/home/acme/bin/perf)
> >                                     do_test (/home/acme/bin/perf)
> > FAILED!
> > 42.2: BPF pinning                                                   :  5594.218 perf/38582 probe_perf:open_debuginfo(__probe_ip: 187650778659428)
> >                                     open_debuginfo (/home/acme/bin/perf)
> >                                     try_to_find_probe_trace_events (inlined)
> >                                     convert_to_probe_trace_events (inlined)
> >                                     convert_perf_probe_events (/home/acme/bin/perf)
> >                                     bpf__probe (/home/acme/bin/perf)
> >                                     parse_events_load_bpf_obj (/home/acme/bin/perf)
> >                                     do_test (/home/acme/bin/perf)
> > FAILED!
> > 42.3: BPF prologue generation                                       : FAILED!
> > 63: Test libpfm4 support                                            :
> > 99: perf stat --bpf-counters test                                   : Skip
> > 100: perf stat --bpf-counters --for-each-cgroup test                 : Skip
> > root@roc-rk3399-pc:~#
> > 
> > So that is the callchains leading to open_debuginfo(), perhaps we should
> > return ENODATA at try_to_find_probe_trace_events() when open_debuginfo()
> > fails?
> 
> Hi Arnaldo,
> 
> Thanks for the suggestions. I tried with changing return code to ENODATA when open_debuginfo fails and will
> send a separate patch addressing this change.
> 
> > 
> > ⬢[acme@toolbox perf]$ find tools/perf/ -name "*.[ch]" | xargs grep try_to_find_probe_trace_events
> > tools/perf/util/probe-event.c:static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> > tools/perf/util/probe-event.c:static int try_to_find_probe_trace_events(struct perf_probe_event *pev,
> > tools/perf/util/probe-event.c:	ret = try_to_find_probe_trace_events(pev, tevs);
> > ⬢[acme@toolbox perf]$ 
> > 
> > Also it returns ENOENT as well when not finding the probe point... There
> > we should return perhaps ENOSYM?
> 
> ENOSYM is not defined for all the archs. 
> 
> 	arch/parisc/include/uapi/asm/errno.h:#define ENOSYM		215	/* symbol does not exist in executable */
> 
> So we need to make this error code generic to use in probe-event. Shall we make this error code
> generic for all archs to use ?

Try to find some other errno that is available everywhere and use
instead, I think.

- Arnaldo