Re: [PATCH perf/core v12 00/16] perf-probe --cache and SDT support

[Masami Hiramatsu]

On Fri, 24 Jun 2016 13:25:20 -0700
Brendan Gregg <brendan.d.gregg@xxxxxxxxx> wrote:

> On Fri, Jun 24, 2016 at 2:05 AM, Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> > Hi,
> >
> > Here is the 12th version of the patchset for probe-cache and
> > initial SDT support.
> >
> > Here is the previous v11: https://lkml.org/lkml/2016/6/14/1041
> >
> > In this version I just rename strlist__for_each to
> > strlist__for_each_entry, and change some patch description.
> >
> 
> I tested it on Node.js (and applied to acme's perf/core), and it works. Thanks!
> 
> # perf script
>             node 36750 [006]  3080.761533:
> sdt_node:http__server__request: (da8b0c)
>             node 36750 [006]  3081.368771:
> sdt_node:http__server__request: (da8b0c)
>             node 36750 [006]  3081.592925:
> sdt_node:http__server__request: (da8b0c)
>             node 36750 [006]  3081.976802:
> sdt_node:http__server__request: (da8b0c)
>             node 36750 [006]  3082.296709:
> sdt_node:http__server__request: (da8b0c)
> 
> Since it doesn't set the is-enabled semaphore yet, I had to do that
> manually for the probes that use it. I know that will be a latter
> addition.

Yes, I'd like to support it. However, the question is who should enable it.

To enable it from perf, perf must attach the target running processes
and change the semaphore. If we run "perf record -a", it has to check
all the running process and tweak it. Moreover, it may have to hook
the execve syscall.

To enable it from kernel, hooking the target process is easier, since
we already did it in uprobes. However, it means we'll introduce a
special functionality to tweak user binary, and of course it may
need to understand SDT section to find out the semaphore.

So, one possible solution is, adding a semaphore option(like an address)
to uprobe events and enable it via ftrace. On this way, perf just
analyzes SDT and pass the semaphore address to ftrace when defining
a probe, and ftrace set 1 to the semaphre address.

> 
> # perf list | grep sdt
>   sdt_node:http__client__request                     [Tracepoint event]
>   sdt_node:http__server__request                     [Tracepoint event]
>   sdt_node:gc__done                                  [SDT event]
>   sdt_node:gc__start                                 [SDT event]
>   sdt_node:http__client__request                     [SDT event]
>   sdt_node:http__client__response                    [SDT event]
>   sdt_node:http__server__request                     [SDT event]
>   sdt_node:http__server__response                    [SDT event]
>   sdt_node:net__server__connection                   [SDT event]
>   sdt_node:net__stream__end                          [SDT event]
> 
> It's also a bit weird to see these listed twice, but I understand
> what's happening. If this continues to prove confusing, I guess later
> on we could change it to exclude listing SDT events that have been
> promoted to Tracepoint events, or not list SDT events in "perf list"
> at all, leaving them for "perf probe --cache --list".

Ah, right. Hmm, at this point, I'd like latter method, since perf
record doesn't support SDT directly yet. So we just need to drop
08/16 and 09/16. It should be implemented with perf-record support.

Thank you!

> 
> Anyway, this is great as is, and thanks again.
> 
> Brendan
> 
> 
> > Thank you,
> >
> > ---
> >
> > Hemant Kumar (1):
> >       perf/sdt: ELF support for SDT
> >
> > Masami Hiramatsu (15):
> >       perf probe: Use cache entry if possible
> >       perf probe: Show all cached probes
> >       perf probe: Remove caches when --cache is given
> >       perf probe: Add group name support
> >       perf buildid-cache: Scan and import user SDT events to probe cache
> >       perf probe: Accept %sdt and %cached event name
> >       perf-list: Show SDT and pre-cached events
> >       perf-list: Skip SDTs placed in invalid binaries
> >       perf: probe-cache: Add for_each_probe_cache_entry() wrapper
> >       perf probe: Allow wildcard for cached events
> >       perf probe: Search SDT/cached event from all probe caches
> >       perf probe: Support @BUILDID or @FILE suffix for SDT events
> >       perf probe: Support a special SDT probe format
> >       perf build: Add sdt feature detection
> >       perf-test: Add a test case for SDT event
> >
> >
> >  tools/perf/Documentation/perf-buildid-cache.txt |    3
> >  tools/perf/Documentation/perf-probe.txt         |   30 +-
> >  tools/perf/Makefile.perf                        |    3
> >  tools/perf/builtin-list.c                       |    6
> >  tools/perf/builtin-probe.c                      |   31 ++
> >  tools/perf/config/Makefile                      |   10 +
> >  tools/perf/tests/Build                          |    1
> >  tools/perf/tests/builtin-test.c                 |    4
> >  tools/perf/tests/make                           |    3
> >  tools/perf/tests/sdt.c                          |  115 +++++++
> >  tools/perf/tests/tests.h                        |    1
> >  tools/perf/util/build-id.c                      |  212 +++++++++++++
> >  tools/perf/util/build-id.h                      |    4
> >  tools/perf/util/parse-events.c                  |   82 +++++
> >  tools/perf/util/parse-events.h                  |    2
> >  tools/perf/util/probe-event.c                   |  366 +++++++++++++++++++++--
> >  tools/perf/util/probe-event.h                   |    1
> >  tools/perf/util/probe-file.c                    |  226 +++++++++++++-
> >  tools/perf/util/probe-file.h                    |   24 +-
> >  tools/perf/util/symbol-elf.c                    |  252 ++++++++++++++++
> >  tools/perf/util/symbol.h                        |   22 +
> >  21 files changed, 1333 insertions(+), 65 deletions(-)
> >  create mode 100644 tools/perf/tests/sdt.c
> >
> > --
> > Masami Hiramatsu


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>