From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ian Rogers <irogers@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@chromium.org>,
Adrian Hunter <adrian.hunter@intel.com>,
Leo Yan <leo.yan@linaro.org>, LKML <linux-kernel@vger.kernel.org>,
Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>,
Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH] perf parse-events: report bpf errors
Date: Wed, 8 Jul 2020 12:45:34 -0300 [thread overview]
Message-ID: <20200708154534.GA22437@kernel.org> (raw)
In-Reply-To: <CAP-5=fXr2xKbiYaNKOGytnwgNYOKYuGK-qT+GYpJZ4tdPb88eA@mail.gmail.com>
Em Wed, Jul 08, 2020 at 08:15:24AM -0700, Ian Rogers escreveu:
> On Wed, Jul 8, 2020 at 4:19 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Tue, Jul 07, 2020 at 02:14:49PM -0700, Ian Rogers escreveu:
> > > Setting the parse_events_error directly doesn't increment num_errors
> > > causing the error message not to be displayed. Use the
> > > parse_events__handle_error function that sets num_errors and handle
> > > multiple errors.
> > What was the command line you used to exercise the error and then the
> > fix?
> You need something to stand in for the BPF event so:
> Before:
> ```
> $ /tmp/perf/perf record -e /tmp/perf/util/parse-events.o
> Run 'perf list' for a list of valid events
> Usage: perf record [<options>] [<command>]
> or: perf record [<options>] -- <command> [<options>]
> -e, --event <event> event selector. use 'perf list' to list available event
> ```
> After:
> ```
> $ /tmp/perf/perf record -e /tmp/perf/util/parse-events.o
> event syntax error: '/tmp/perf/util/parse-events.o'
> \___ Failed to load /tmp/perf/util/parse-events.o:
> BPF object format invalid
>
> (add -v to see detail)
> Run 'perf list' for a list of valid events
> Usage: perf record [<options>] [<command>]
> or: perf record [<options>] -- <command> [<options>]
> -e, --event <event> event selector. use 'perf list' to list
> available events
> ```
Cool, I'll add that to the cset comment log.
If you need programs to test the Ok path, consider:
# perf trace -e tools/perf/examples/bpf/5sec.c sleep 5.1
0.000 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1508461648, rqtp: 5100000000)
#
After applying tge patch below, that I'll have soon in my repo, I guess
this comes from those header includes path fixes from you :)
# cat tools/perf/examples/bpf/5sec.c
#include <bpf.h>
#define NSEC_PER_SEC 1000000000L
int probe(hrtimer_nanosleep, rqtp)(void *ctx, int err, long long sec)
{
return sec / NSEC_PER_SEC == 5ULL;
}
license(GPL);
#
Backtraces works as well and you can ask for the .o file to be retained
so that you then skip the compilation phase and use the .o file
directly:
# perf config llvm.dump-obj=true
# perf config llvm.dump-obj
llvm.dump-obj=true
# perf trace -e tools/perf/examples/bpf/5sec.c/max-stack=99/ sleep 5.1
0.000 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1508461648, rqtp: 5100000000)
hrtimer_nanosleep ([kernel.kallsyms])
__x64_sys_nanosleep ([kernel.kallsyms])
do_syscall_64 ([kernel.kallsyms])
entry_SYSCALL_64 ([kernel.kallsyms])
__GI___nanosleep (/usr/lib64/libc-2.29.so)
#
# file tools/perf/examples/bpf/5sec.o
tools/perf/examples/bpf/5sec.o: ELF 64-bit LSB relocatable, eBPF, version 1 (SYSV), with debug_info, not stripped
#
# perf trace -e tools/perf/examples/bpf/5sec.o/max-stack=3/ sleep 5.1
0.000 perf_bpf_probe:hrtimer_nanosleep(__probe_ip: -1508461648, rqtp: 5100000000)
hrtimer_nanosleep ([kernel.kallsyms])
__x64_sys_nanosleep ([kernel.kallsyms])
do_syscall_64 ([kernel.kallsyms])
#
- Arnaldo
[root@quaco perf]# git diff
diff --git a/tools/perf/examples/bpf/5sec.c b/tools/perf/examples/bpf/5sec.c
index 65c4ff6892d9..e6b6181c6dc6 100644
--- a/tools/perf/examples/bpf/5sec.c
+++ b/tools/perf/examples/bpf/5sec.c
@@ -39,7 +39,7 @@
Copyright (C) 2018 Red Hat, Inc., Arnaldo Carvalho de Melo <acme@redhat.com>
*/
-#include <bpf/bpf.h>
+#include <bpf.h>
#define NSEC_PER_SEC 1000000000L
[root@quaco perf]#
- Arnaldo
> Thanks,
> Ian
>
> > - Arnaldo
> >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > > tools/perf/util/parse-events.c | 38 ++++++++++++++++++----------------
> > > 1 file changed, 20 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > > index c4906a6a9f1a..e88e4c7a2a9a 100644
> > > --- a/tools/perf/util/parse-events.c
> > > +++ b/tools/perf/util/parse-events.c
> > > @@ -767,8 +767,8 @@ int parse_events_load_bpf_obj(struct parse_events_state *parse_state,
> > >
> > > return 0;
> > > errout:
> > > - parse_state->error->help = strdup("(add -v to see detail)");
> > > - parse_state->error->str = strdup(errbuf);
> > > + parse_events__handle_error(parse_state->error, 0,
> > > + strdup(errbuf), strdup("(add -v to see detail)"));
> > > return err;
> > > }
> > >
> > > @@ -784,36 +784,38 @@ parse_events_config_bpf(struct parse_events_state *parse_state,
> > > return 0;
> > >
> > > list_for_each_entry(term, head_config, list) {
> > > - char errbuf[BUFSIZ];
> > > int err;
> > >
> > > if (term->type_term != PARSE_EVENTS__TERM_TYPE_USER) {
> > > - snprintf(errbuf, sizeof(errbuf),
> > > - "Invalid config term for BPF object");
> > > - errbuf[BUFSIZ - 1] = '\0';
> > > -
> > > - parse_state->error->idx = term->err_term;
> > > - parse_state->error->str = strdup(errbuf);
> > > + parse_events__handle_error(parse_state->error, term->err_term,
> > > + strdup("Invalid config term for BPF object"),
> > > + NULL);
> > > return -EINVAL;
> > > }
> > >
> > > err = bpf__config_obj(obj, term, parse_state->evlist, &error_pos);
> > > if (err) {
> > > + char errbuf[BUFSIZ];
> > > + int idx;
> > > +
> > > bpf__strerror_config_obj(obj, term, parse_state->evlist,
> > > &error_pos, err, errbuf,
> > > sizeof(errbuf));
> > > - parse_state->error->help = strdup(
> > > +
> > > + if (err == -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE)
> > > + idx = term->err_val;
> > > + else
> > > + idx = term->err_term + error_pos;
> > > +
> > > + parse_events__handle_error(parse_state->error, idx,
> > > + strdup(errbuf),
> > > + strdup(
> > > "Hint:\tValid config terms:\n"
> > > " \tmap:[<arraymap>].value<indices>=[value]\n"
> > > " \tmap:[<eventmap>].event<indices>=[event]\n"
> > > "\n"
> > > " \twhere <indices> is something like [0,3...5] or [all]\n"
> > > -" \t(add -v to see detail)");
> > > - parse_state->error->str = strdup(errbuf);
> > > - if (err == -BPF_LOADER_ERRNO__OBJCONF_MAP_VALUE)
> > > - parse_state->error->idx = term->err_val;
> > > - else
> > > - parse_state->error->idx = term->err_term + error_pos;
> > > +" \t(add -v to see detail)"));
> > > return err;
> > > }
> > > }
> > > @@ -877,8 +879,8 @@ int parse_events_load_bpf(struct parse_events_state *parse_state,
> > > -err, errbuf,
> > > sizeof(errbuf));
> > >
> > > - parse_state->error->help = strdup("(add -v to see detail)");
> > > - parse_state->error->str = strdup(errbuf);
> > > + parse_events__handle_error(parse_state->error, 0,
> > > + strdup(errbuf), strdup("(add -v to see detail)"));
> > > return err;
> > > }
> > >
> > > --
> > > 2.27.0.383.g050319c2ae-goog
> > >
> >
> > --
> >
> > - Arnaldo
--
- Arnaldo
next prev parent reply other threads:[~2020-07-08 15:45 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-07 21:14 [PATCH] perf parse-events: report bpf errors Ian Rogers
2020-07-08 11:19 ` Arnaldo Carvalho de Melo
2020-07-08 15:15 ` Ian Rogers
2020-07-08 15:45 ` Arnaldo Carvalho de Melo [this message]
2020-07-08 18:47 ` Jiri Olsa
2020-07-10 12:37 ` Arnaldo Carvalho de Melo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200708154534.GA22437@kernel.org \
--to=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@redhat.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=leo.yan@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).