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

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

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> 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> 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>
> >>> Reviewed-by: Madhavan Srinivasan <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?

⬢[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?
 
> But issue with using this return code is that, there are other places
> in the code path for "parse_events_load_bpf_obj" which also returns
> ENOENT. In that case, for any exit path that returns ENOENT, test will
> get skipped.  Hence looking for an alternative way to identify missing
> debuginfo to skip the test. Please share your thoughts on this.

See above.

- Arnaldo