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

[Hemant Kumar <hemant@linux.vnet.ibm.com>]

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@linux.vnet.ibm.com> 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?

# 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.

> 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 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