Re: [PATCH perf/core 07/10] perf probe: Support @BUILDID or @FILE suffix for SDT events

From: Arnaldo Carvalho de Melo
Date: Wed Jul 13 2016 - 20:07:35 EST

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH perf/core 07/10] perf probe: Support @BUILDID or @FILE suffix for SDT events"
Previous message: David Rientjes: "Re: System freezes after OOM"
In reply to: Arnaldo Carvalho de Melo: "Re: [PATCH perf/core 07/10] perf probe: Support @BUILDID or @FILE suffix for SDT events"
Next in thread: Arnaldo Carvalho de Melo: "Re: [PATCH perf/core 07/10] perf probe: Support @BUILDID or @FILE suffix for SDT events"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Wed, Jul 13, 2016 at 04:50:19PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Jul 12, 2016 at 07:05:37PM +0900, Masami Hiramatsu escreveu:
> > Support @BUILDID or @FILE suffix for SDT events. This allows
> > perf to add probes on SDTs/pre-cached events on given FILE
> > or the file which has given BUILDID (also, this complements
> > BUILDID.)
> > For example, both gcc and libstdc++ has same SDTs as below.
> > If you would like to add a probe on sdt_libstdcxx:catch on gcc,
> > you can do as below.
>
> Ok, how to add those entries?
>
> [root@jouet hello_world]# perf probe -x /usr/bin/gcc -a %sdt_gcc:\*
> Error: Failed to add events.
> [root@jouet hello_world]# perf probe --verbose -x /usr/bin/gcc -a %sdt_gcc:\*
> probe-definition(0): %sdt_gcc:*
> 0 arguments
> symbol:throw file:(null) line:0 offset:0 return:0 lazy:(null)
> symbol:rethrow file:(null) line:0 offset:0 return:0 lazy:(null)
> symbol:catch file:(null) line:0 offset:0 return:0 lazy:(null)
> symbol:unwind file:(null) line:0 offset:0 return:0 lazy:(null)
> Error: Failed to add events. Reason: No such file or directory (Code: -2)
> [root@jouet hello_world]#

So, ok, I managed with:

[root@jouet ~]# perf probe -x /usr/bin/gcc %sdt_libstdcxx:\*
Added new events:
sdt_libstdcxx:throw (on %* in /usr/bin/gcc)
sdt_libstdcxx:rethrow (on %* in /usr/bin/gcc)
sdt_libstdcxx:catch (on %* in /usr/bin/gcc)

You can now use it in all perf tools, such as:

perf record -e sdt_libstdcxx:catch -aR sleep 1

[root@jouet ~]#

I then tried to remove the events just created:

# perf probe -d %sdt_libstdcxx:\*
#

It tells me nothing, cool, probably it removed it all, then I go and
try re-adding them:

[root@jouet ~]# perf probe -x /usr/bin/gcc %sdt_libstdcxx:\*
Error: event "throw" already exists.
Hint: Remove existing event by 'perf probe -d'
or force duplicates by 'perf probe -f'
or set 'force=yes' in BPF source.
Error: Failed to add events.
[root@jouet ~]#

Huh?

Then I tried like you did above, using 'perf list sdt'

[root@jouet ~]# perf list sdt

List of pre-defined events (to be used in -e):

[root@jouet ~]#

But they appear via 'perf probe -l'

[root@jouet ~]# perf probe -l sdt_libs*
sdt_libstdcxx:catch (on execute_cfa_program+1551@../../../libgcc/unwind-dw2.c in /usr/bin/gcc)
sdt_libstdcxx:rethrow (on d_print_subexpr+280@libsupc++/cp-demangle.c in /usr/bin/gcc)
sdt_libstdcxx:throw (on d_print_subexpr+93@libsupc++/cp-demangle.c in /usr/bin/gcc)
[root@jouet ~]#

There is no event, then, guessing, I tried removing that %, since I don't see
it in the 'perf probe -l' output, so it must've been just some distracting
artifact at the time of adding the SDT prove:

[root@jouet ~]# perf probe -d sdt_libstdcxx:\*
Removed event: sdt_libstdcxx:catch
Removed event: sdt_libstdcxx:rethrow
Removed event: sdt_libstdcxx:throw
[root@jouet ~]# perf probe -l sdt_libs*
[root@jouet ~]#

Ok, now it works, but then if I try having both the gcc and libstdc++ ones, I can't:

[root@jouet ~]# perf probe -x /usr/lib64/libstdc++.so.6 %sdt_libstdcxx:\*
Error: event "catch" already exists.
Hint: Remove existing event by 'perf probe -d'
or force duplicates by 'perf probe -f'
or set 'force=yes' in BPF source.
Error: Failed to add events.

So I forced it:

[root@jouet ~]# perf probe -f -x /usr/lib64/libstdc++.so.6 %sdt_libstdcxx:\*
Added new events:
sdt_libstdcxx:catch_1 (on %* in /usr/lib64/libstdc++.so.6.0.22)
sdt_libstdcxx:throw_1 (on %* in /usr/lib64/libstdc++.so.6.0.22)
sdt_libstdcxx:rethrow_1 (on %* in /usr/lib64/libstdc++.so.6.0.22)

You can now use it in all perf tools, such as:

perf record -e sdt_libstdcxx:rethrow_1 -aR sleep 1

[root@jouet ~]#

If it will rename it, why require -f? Where is the duplicity?

If I go and re-read all the discussion on the thread that lead to these
patches, I may figure out all those questions, but then, that would be a lot
of info to have in mind :-\

I.e. I would suggest that no -f should be needed, that that "Error: event
"catch" already exists" shouldn't be even emitted, since those will be
different events, i.e. one is named "sdt_libstdcxx:catch" and the other
"sdt_libstdcxx:catch_1"...

But that becomes an usability problem, couldn't it have the basename in it?
I.e.:

sdt_gcc_libstdcxx:catch and sdt_libstdc++:catch?

I'm going to look my inbox for patches I overlook that added 'perf list sdt'...

- Arnaldo

> Can those messages be improved? What am I missing?
>
> > ----
> > # perf list sdt | tail -n 6
> > sdt_libstdcxx:catch@/usr/bin/gcc(0cc207fc4b27) [SDT event]
> > sdt_libstdcxx:catch@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
> > sdt_libstdcxx:rethrow@/usr/bin/gcc(0cc207fc4b27) [SDT event]
> > sdt_libstdcxx:rethrow@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
> > sdt_libstdcxx:throw@/usr/bin/gcc(0cc207fc4b27) [SDT event]
> > sdt_libstdcxx:throw@/usr/lib64/libstdc++.so.6.0.20(91c7a88fdf49)
> > # perf probe -a %sdt_libstdcxx:catch@0cc
> > Added new event:
> > sdt_libstdcxx:catch (on %catch in /usr/bin/gcc)
> >
> > You can now use it in all perf tools, such as:
> >
> > perf record -e sdt_libstdcxx:catch -aR sleep 1
> > ----
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > ---
> > Changes in v12:
> > - Rename strlist__for_each to strlist__for_each_entry.
> > ---
> > tools/perf/util/build-id.c | 43 +++++++++++++++++++++++++++++++++++++++++
> > tools/perf/util/build-id.h | 1 +
> > tools/perf/util/probe-event.c | 17 ++++++++++++++--
> > 3 files changed, 59 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> > index 8e86a83..e59003e 100644
> > --- a/tools/perf/util/build-id.c
> > +++ b/tools/perf/util/build-id.c
> > @@ -523,6 +523,49 @@ err_out:
> > goto out_free;
> > }
> >
> > +static bool str_is_build_id(const char *maybe_sbuild_id, size_t len)
> > +{
> > + size_t i;
> > +
> > + for (i = 0; i < len; i++) {
> > + if (!isxdigit(maybe_sbuild_id[i]))
> > + return false;
> > + }
> > + return true;
> > +}
> > +
> > +/* Return the valid complete build-id */
> > +char *build_id_cache__complement(const char *incomplete_sbuild_id)
> > +{
> > + struct strlist *bidlist;
> > + struct str_node *nd, *cand = NULL;
> > + char *sbuild_id = NULL;
> > + size_t len = strlen(incomplete_sbuild_id);
> > +
> > + if (len >= SBUILD_ID_SIZE ||
> > + !str_is_build_id(incomplete_sbuild_id, len))
> > + return NULL;
> > +
> > + bidlist = build_id_cache__list_all(true);
> > + if (!bidlist)
> > + return NULL;
> > +
> > + strlist__for_each_entry(nd, bidlist) {
> > + if (strncmp(nd->s, incomplete_sbuild_id, len) != 0)
> > + continue;
> > + if (cand) { /* Error: There are more than 2 candidates. */
> > + cand = NULL;
> > + break;
> > + }
> > + cand = nd;
> > + }
> > + if (cand)
> > + sbuild_id = strdup(cand->s);
> > + strlist__delete(bidlist);
> > +
> > + return sbuild_id;
> > +}
> > +
> > char *build_id_cache__cachedir(const char *sbuild_id, const char *name,
> > bool is_kallsyms, bool is_vdso)
> > {
> > diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> > index 64e740f..d279906 100644
> > --- a/tools/perf/util/build-id.h
> > +++ b/tools/perf/util/build-id.h
> > @@ -35,6 +35,7 @@ char *build_id_cache__linkname(const char *sbuild_id, char *bf, size_t size);
> > char *build_id_cache__cachedir(const char *sbuild_id, const char *name,
> > bool is_kallsyms, bool is_vdso);
> > struct strlist *build_id_cache__list_all(bool validonly);
> > +char *build_id_cache__complement(const char *incomplete_sbuild_id);
> > int build_id_cache__list_build_ids(const char *pathname,
> > struct strlist **result);
> > bool build_id_cache__cached(const char *sbuild_id);
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 679aa6b..0dadaa3 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -1251,8 +1251,21 @@ static int parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
> > ptr = strpbrk(arg, ";=@+%");
> > if (pev->sdt) {
> > if (ptr) {
> > - semantic_error("%s must contain only an SDT event name.\n", arg);
> > - return -EINVAL;
> > + if (*ptr != '@') {
> > + semantic_error("%s must be an SDT name.\n",
> > + arg);
> > + return -EINVAL;
> > + }
> > + /* This must be a target file name or build id */
> > + tmp = build_id_cache__complement(ptr + 1);
> > + if (tmp) {
> > + pev->target = build_id_cache__origname(tmp);
> > + free(tmp);
> > + } else
> > + pev->target = strdup(ptr + 1);
> > + if (!pev->target)
> > + return -ENOMEM;
> > + *ptr = '\0';
> > }
> > ret = parse_perf_probe_event_name(&arg, pev);
> > if (ret == 0) {

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH perf/core 07/10] perf probe: Support @BUILDID or @FILE suffix for SDT events"
Previous message: David Rientjes: "Re: System freezes after OOM"
In reply to: Arnaldo Carvalho de Melo: "Re: [PATCH perf/core 07/10] perf probe: Support @BUILDID or @FILE suffix for SDT events"
Next in thread: Arnaldo Carvalho de Melo: "Re: [PATCH perf/core 07/10] perf probe: Support @BUILDID or @FILE suffix for SDT events"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]