Re: [PATCH] perf/sdt: Directly record cached SDT events

[Masami Hiramatsu]

On Tue, 03 May 2016 05:06:24 +0530
Hemant Kumar <hemant@xxxxxxxxxxxxxxxxxx> wrote:

> Hi Masami,
> 
> On 04/30/2016 06:06 PM, Masami Hiramatsu wrote:
> > Hi Hemant,
> >
> > On Fri, 29 Apr 2016 19:10:41 +0530
> > Hemant Kumar <hemant@xxxxxxxxxxxxxxxxxx> wrote:
> >
> >> This patch adds support for directly recording SDT events which are
> >> present in the probe cache. This patch is based on current SDT
> >> enablement patchset (v5) by Masami :
> >> https://lkml.org/lkml/2016/4/27/828
> >> and it implements two points in the TODO list mentioned in the
> >> cover note :
> >> "- (perf record) Support SDT event recording directly"
> >> "- (perf record) Try to unregister SDT events after record."
> >>
> >> Without this patch, we could probe into SDT events using
> >> "perf probe" and "perf record". With this patch, we can probe
> >> the SDT events directly using "perf record".
> > Thanks! However, before looking over each part of this patch,
> > I think this is not enough for supporting SDT for perf record.
> 
> Hmm.
> 
> > If there are several SDTs which have same eventname but differnt
> > addresses (e.g. libc:memory_memalign_retry), how are those handled?
> > Currently, to support this, we'll need to enable those events
> > in different names, or just pick one of them. It could confuse
> > users in each case.
> 
> Right. But now, its the same case with a binary having multiple
> symbols with same names, isn't it?

Yes, but for the symbols or lines etc., user can not directly specify
it via perf record. And as you showed below, perf-probe expresses
there are 2 events on the probe point. So user is forced to aware of it.

> 
> # nm ./multi | grep foo
> 0000000000400530 t foo
> 0000000000400560 t foo
> 
> # perf probe -x ./multi foo
> Added new events:
>    probe_multi:foo      (on foo in /home/hemant/work/linux/tools/perf/multi)
>    probe_multi:foo_1    (on foo in /home/hemant/work/linux/tools/perf/multi)
> 
> You can now use it in all perf tools, such as:
> 
>      perf record -e probe_multi:foo_1 -aR sleep 1
> 
> 
> My point being, the user can still know, if its shown that there are two or
> more probes being placed and the o/p of perf report/script shows that
> the probes are placed at two or more different addresses.

Not only the different address, but also they will see the different
event names. That may be no good for making a script on it.

My point is, if the user only uses "perf record -e sdt_something:sdtevent",
they will think that there is one event recorded. it can easily misleading
them.

> > To solve this issue, we need to introduce multiple SDTs on single
> > ftrace event. Please read my comment on v3 patch (https://lkml.org/lkml/2015/8/15/52)
> 
> Ok. But, I think, for initial direct recording support, we can go with 
> this IMHO.

So, at least this should be noticed to users carefully. (e.g. warn if
there are more than two SDTs defined)

Thank you,


> > So, at this point, I've decided to introduce only perf-probe side.
> > If user want to record SDT, they can use perf-probe to add SDT events
> > and see what events are generated by SDT, so that they can specify those
> > events via perf-record.
> > e.g.
> >
> > # perf probe -a %sdt_libc:*
> > ...
> >    sdt_libc:lll_futex_wake_1 (on %* in /usr/lib64/libc-2.20.so)
> >    sdt_libc:lll_lock_wait_private (on %* in /usr/lib64/libc-2.20.so)
> >
> > You can now use it in all perf tools, such as:
> >
> > 	perf record -e sdt_libc:lll_lock_wait_private -aR sleep 1
> >
> > # perf record -e sdt_libc:* -a
> > ^C[ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 1.461 MB perf.data (6195 samples) ]
> >
> > # perf script
> >   plugin_audio_th 11119 [000] 16059.864654:   sdt_libc:setjmp: (7f37bf55b6c1)
> >            chrome  4650 [000] 16059.881345:   sdt_libc:setjmp: (7f37bf55b6c1)
> >            chrome  4650 [000] 16059.881350:   sdt_libc:setjmp: (7f37bf55b6c1)
> >            chrome  4650 [000] 16059.881411:   sdt_libc:setjmp: (7f37bf55b6c1)
> > ...
> >
> > BTW, see below comments on the code for implementation issues.
> 
> Will fix the issues and send a v2.
> 
> Thanks a lot for the review.
> 
> -- 
> Thanks,
> Hemant Kumar
> 


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>