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

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

On 05/03/2016 06:05 AM, Masami Hiramatsu wrote:
> On Tue, 03 May 2016 05:06:24 +0530
> Hemant Kumar <hemant@linux.vnet.ibm.com> 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@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?
> 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.

Right.

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

Ok. Makes sense. With a warning message then, we can make the user
aware in this case.

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

Ok. I have made the changes and also added a warning message if the
user tries to record on an sdt event, which has multiple occurences with
the same event and group name. I have sent a v2 for this patch.

-- 
Thanks,
Hemant Kumar