Re: Re: [PATCH v4 5/5] perf/sdt: Add support to perf record to trace SDT events

[Masami Hiramatsu <masami dot hiramatsu dot pt at hitachi dot com>]

Hi,

(2014/11/04 17:06), Hemant Kumar wrote:
> Hi Namhyung,
> 
> On 11/04/2014 01:08 PM, Namhyung Kim wrote:
>> Hi Hemant,
>>
>> As you know, you need to keep an eye on how (kprobes) event cache
>> patchset from Masami settles down.  For those who aren't CC'ed, please
>> see the link below:
>>
>>    https://lkml.org/lkml/2014/10/31/207
>>
>> On Sun, 02 Nov 2014 16:26:28 +0530, Hemant Kumar wrote:
>>> This patch adds support to perf to record SDT events. When invoked,
>>> the SDT event is looked up in the sdt-cache. If its found, an entry is
>>> made silently to uprobe_events file and then recording is invoked, and
>>> then the entry for the SDT event in uprobe_events is silently discarded.
>>>
>>> The SDT events are already stored in a cache file
>>> (/var/cache/perf/perf-sdt-file.cache).
>>> Although the file_hash table helps in addition or deletion of SDT events
>>> from the cache, its not of much use when it comes to probing the actual
>>> SDT event, because the key to this hash list is a file name and not the
>>> SDT event name (which is given as an argument to perf record). So, we
>>> won't be able to hash into it.
>> It likely to be ended up with per-file or per-buildid cache files under
>> ~/.debug directory.  In this case we also need to have the (central)
>> event-to-cache table anyway IMHO.

What we are talking is to make a new caching file with buildid under
.debug/.
We already has ~/.debug/.build-id/<build-id> for string the binary
symbol maps. I think there are 2 options, one is expanding the current
build-id file format to include sdt and probe-event caches. The other is
to add ~/.debug/.build-id/<build-id>.probe and
~/.debug/.build-id/<build-id>.sdt for caching probe/sdt information.

And also, user interface is a discussion point. This series defines new
sdt-cache command, and we already have buildid-cache command. We should
have probe-cache command too? or consolidate those cache managing commands?
This question should be involving your series too.

>>> To avoid this problem, we can create another hash list "event_hash" list
>>> which will be maintained along with the file_hash list.
>>> Whenever a user invokes 'perf record -e %provider:event, perf should
>>> initialize the event_hash list and the file_hash list.
>>> The key to event_hash list is calculated from the event name and its
>>> provider name.
>> Isn't it enough just to use provide name?  I guess the provider names
>> are (should be?) unique among a system although there's no absolute
>> guarantee for that.
>>
> 
> Yes, there is no guarantee for the provider names to be unique.
> If we use only provider name with "perf record", then, what if a user 
> wants to trace
> only a specific SDT event (not all the events for that provider)?
> What do you think?

How about failing if the provider name is not unique unless user
gives the actual binary path?

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com