From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 78BBDC4332F for ; Wed, 14 Dec 2022 15:55:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238878AbiLNPze (ORCPT ); Wed, 14 Dec 2022 10:55:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55970 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238489AbiLNPzG (ORCPT ); Wed, 14 Dec 2022 10:55:06 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A0D8C2B181 for ; Wed, 14 Dec 2022 07:51:41 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 3B6BDB818E8 for ; Wed, 14 Dec 2022 15:51:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 908ECC433EF; Wed, 14 Dec 2022 15:51:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1671033098; bh=0AiPOWaXzR9wFk0mvBF4P1v2VviLkdCesDZbtqkudPo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=s9tSyzsP7bNgg/C0KPShsPZYdkcVAGsMPskh94D3+1/sWDJCviFHBlSagZBPrgIs4 P8d4u1wKA3lkh20qclXr8YCuZeItKOBq3qB1YLL/ZIf4B8G7utm2n+ymFQ+pzoTtag tH7/bGdlaM/yqxkZBwXVcvsOK/GhNb9D+T7kNNFnn/dS7AoTIOsJ2ynWVsq62/x1kr +lssn5opURUhugct4Im8CTvSrAMJyfch5HxmIkgJ3OzizJP17sIqYjCgRY5mYEgq5r qfM9BQI+drINAG6NNmXwSWQdfc+t3ujkO9KSNEaoMUrvrSk20wSYOz0xfDaytpf9Cf IcG69uzPJs1Ag== Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id BC85040367; Wed, 14 Dec 2022 12:51:34 -0300 (-03) Date: Wed, 14 Dec 2022 12:51:34 -0300 From: Arnaldo Carvalho de Melo To: Athira Rajeev Cc: Ian Rogers , Madhavan Srinivasan , Nageswara Sastry , Kajol Jain , linux-perf-users@vger.kernel.org, Jiri Olsa , Disha Goel , linuxppc-dev@lists.ozlabs.org Subject: Re: [PATCH] perf test bpf: Skip test if kernel-debuginfo is not present Message-ID: References: <20221028154230.140709-1-kjain@linux.ibm.com> <6D5F1D8A-47FF-46CF-8251-20BABDF283F6@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <6D5F1D8A-47FF-46CF-8251-20BABDF283F6@linux.vnet.ibm.com> X-Url: http://acmel.wordpress.com Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.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 wrote: > > Em Thu, Nov 03, 2022 at 12:27:01PM +0530, Athira Rajeev escreveu: > >>> On 28-Oct-2022, at 9:12 PM, Kajol Jain 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 > >>> Reviewed-by: Madhavan Srinivasan > >>> --- > >>> 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