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

[Hemant Kumar]

On 05/03/2016 06:05 AM, Masami Hiramatsu wrote:
> 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.

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