Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1190909 > unrolled thread

Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and  SDT support Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-07-23 15:20 +0200
    Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT  support Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-07-23 16:10 +0200
      Re: Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and  SDT support Namhyung Kim <namhyung@kernel.org> - 2015-07-24 10:10 +0200
        Re: Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and  SDT support Namhyung Kim <namhyung@kernel.org> - 2015-07-27 16:10 +0200
          Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT support Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> - 2015-07-28 02:50 +0200

#1190909 — Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT support

On 2015/07/22 23:12, Hemant Kumar wrote:
> Hi Masami,
> 
> Apologies for the delayed response.
> 
> On 07/17/2015 08:51 AM, Masami Hiramatsu wrote:
>> Hi Hemant,
>>
>> On 2015/07/16 12:13, Hemant Kumar wrote:
>>> Hi Masami,
>>>
>>> On 07/15/2015 02:43 PM, Masami Hiramatsu wrote:
>>>> Hi,
>>>>
>>>> Here is the 2nd version of the patchset for probe-cache and
>>>> initial SDT support which are going to be perf-cache finally.
>>> Thanks for adding the SDT support.
>>>
>>>> The perf-probe is useful for debugging, but it strongly depends
>>>> on the debuginfo. Without debuginfo, it is just a frontend of
>>>> ftrace's dynamic events. This can usually happen in server
>>>> farms or on cloud system, since no one wants to distribute
>>>> big debuginfo packages.
>>>>
>>>> To solve this issue, I had tried to make a pre-analyzed probes
>>>> ( https://lkml.org/lkml/2014/10/31/207 ) but it has a problm
>>>> that we can't ensure the probed binary is same as what we analyzed.
>>>> Arnaldo gave me an idea to reuse build-id cache for that perpose
>>>> and this series is the first prototype of that.
>>>>
>>>> At the same time, Hemant has started to support SDT probes which
>>>> also use the cache file of SDT info. So I decided to merge this
>>>> into the same build-id cache.
>>>> In this version, SDT support is still very limited, it works
>>>> as a part of probe-cache.
>>>>
>>>> In this version, perf probe supports --cache option which means
>>>> that perf probe manipulate probe caches, for example,
>>>>
>>>>     # perf probe --cache --add "probe-desc"
>>>>
>>>> does not only add probe events but also add "probe-desc" and
>>>> it's result on the cache. (Note that the cached entry is always
>>>> referred even without --cache)
>>>> The --list and --del commands also support --cache. Note that
>>>> both are only manipulate caches, not real events.
>>>>
>>>> To use SDT, we have to scan the target binary at first by using
>>>> perf-buildid-cache, e.g.
>>>>
>>>>     # perf buildid-cache --add /lib/libc-2.17.so
>>>>
>>>> And perf probe --cache --list shows what SDTs are scanned.
>>>>
>>>>     # perf probe --cache --list
>>>>     /usr/lib/libc-2.17.so (a6fb821bdf53660eb2c29f778757aef294d3d392):
>>>>     libc:setjmp=setjmp
>>>>     libc:longjmp=longjmp
>>>>     libc:longjmp_target=longjmp_target
>>>>     libc:memory_heap_new=memory_heap_new
>>>>     libc:memory_sbrk_less=memory_sbrk_less
>>>>     libc:memory_arena_reuse_free_list=memory_arena_reuse_free_list
>>>>     libc:memory_arena_reuse=memory_arena_reuse
>>>>     ...
>>>>
>>>> To use the SDT events, perf probe -x BIN %SDTEVENT allows you to
>>>> add a probe on SDTEVENT@BIN.
>>>>
>>>>     # perf probe -x /lib/libc-2.17.so %memory_heap_new
>>>>
>>>> If you define a cached probe with event name, you can also reuse
>>>> it as same as SDT events.
>>>>
>>>>     # perf probe -x ./perf --cache -n 'myevent=dso__load $params'
>>>>
>>>> (Note that "-n" option only updates caches)
>>>> To use the above "myevent", you just have to add "%myevent".
>>>>
>>>>     # perf probe -x ./perf %myevent
>>>>
>>>>
>>>> TODOs:
>>>>    - Show available cached/SDT events by perf-list
>>>>    - Allow perf-record to use cached/SDT events directly
>>> As I was already working on SDT events' recording
>>> https://lkml.org/lkml/2014/11/2/73,
>>> I can re-spin the patches on top of your patchset and make the
>>> required changes to implement the above TODOs.
>> Sounds great! :)
>> Note that you'll need to re-implement almost from scratch, since
>> now the SDT is implemented on buildid-cache. Maybe I have to work
>> on the buildid-cache one more to filter out binaries which are gone
>> or different version from current running one (e.g. old vmlinux).
>> It could help you to get available SDTs when showing it via perf-list.
> 
> Sure. That would be great.
> 
>>> What would you suggest?
>> Now I'm thinking that we should avoid using %event syntax for perf-list
>> and perf-record to avoid confusion. For example, suppose that we have
>> "libfoo:bar" SDT event, when we just scanned the libfoo binary and
>> use it via perf-record, we'll run perf record -e "%libfoo:bar".
>> However, after we set the probe via perf-probe, we have to run
>> perf record -e "libfoo:bar". That difference looks no good.
>> So, I think in both case it should accept -e "libfoo:bar" syntax.
> 
> Although I agree to have "perf record" as a higher level tool and not bother
> this tool to distinguish between its events, but that way we end up looking
> into kprobe_events, uprobe_events, kernel tracepoints and then the entire
> cache for any event (which may or may not be an SDT event or even a valid
> event) lookup. Right?

Yeah, right.

> 
> The idea behind '%' was to identify the SDT events and take a different path
> to lookup through the cache, put a probe, record and then delete the probe.
> Or, do you want "perf record" to record any event this way (not just an sdt
> event).

I see, but I think that is not good by following reasons,

- when we record event with "-e %provider:event", it will be shown as
  "provider:event"
- if perf-list shows the SDT(cached) events as "%provider:event", that
  will not match the recorded result.
- it is somewhat fragile that we temporary add the SDT event and remove it
  after record, because the event will not hide from ftrace users (this
  means that we'll fail removing the event by -EBUSY if someone use it
  via ftrace)
- if we set SDT events perf-probe, it will be shown as "provider:event" name
  because "%" will be rejected by ftrace. In that case, what the perf-list show
  those events, both of %provider:event and provider:event ?

thus I pushed the "%" as a "special remembering mark" only for looking
up the event from cache by perf-probe.

So I'd like to suggest that the following behavior

1) perf-list shows the cached-with-name and SDT events as Tracepoint events
  even if it is not yet probed.

# perf list

List of pre-defined events (to be used in -e):
...
  libc:memory_heap_new                             [Tracepoint event]
...
  probes:myevent                                   [Tracepoint event]
...

2) perf-record -e with no-probed event should try to set up the given probe
 by using perf-probe. It is possible to remove that the probe after recording,
 but also ignore if it fails by -EBUSY. (anyway, there is no difference for
 users)

This rule will solve the contradiction between the event name on recorded
data and listed events. However, as we discussed there are other clashes.

A) clash among binaries: Since the binary builders can freely use the
provider name, it is possible to clash to other binaries' SDTs.

B) clash among different versions: Of course the different versions of binaries
can be co-exist on the system. Those usually have the same SDTs and same
basename, just different build-ids.

These issues are not solved by using "%" because it happens among SDTs.
So we need to find another way to distinguish the SDTs.

Thank you,

> 
> Please correct me if I missed something.
> 
>> In this series I've introduced %event syntax only to recall cached event
>> setting explicitly, because perf-probe is a lower layer tool to set up
>> new event. IMO, perf-list and perf-record should be higher tools which
>> handle abstract events.
>>
>> Thanks!
>>
>>
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1190945 — Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT support

Em Thu, Jul 23, 2015 at 10:13:22PM +0900, Masami Hiramatsu escreveu:
> On 2015/07/22 23:12, Hemant Kumar wrote:
> > On 07/17/2015 08:51 AM, Masami Hiramatsu wrote:
> >> On 2015/07/16 12:13, Hemant Kumar wrote:
> > The idea behind '%' was to identify the SDT events and take a different path
> > to lookup through the cache, put a probe, record and then delete the probe.
> > Or, do you want "perf record" to record any event this way (not just an sdt
> > event).
 
> I see, but I think that is not good by following reasons,
 
> - when we record event with "-e %provider:event", it will be shown as
>   "provider:event"
> - if perf-list shows the SDT(cached) events as "%provider:event", that
>   will not match the recorded result.
> - it is somewhat fragile that we temporary add the SDT event and remove it
>   after record, because the event will not hide from ftrace users (this
>   means that we'll fail removing the event by -EBUSY if someone use it
>   via ftrace)

We should avoid that, if we say record event "foo:bar", then we should
consistently show "foo:bar" everywhere this is referenced.

> - if we set SDT events perf-probe, it will be shown as "provider:event" name
>   because "%" will be rejected by ftrace. In that case, what the perf-list show
>   those events, both of %provider:event and provider:event ?
 
> thus I pushed the "%" as a "special remembering mark" only for looking
> up the event from cache by perf-probe.
 
> So I'd like to suggest that the following behavior
 
> 1) perf-list shows the cached-with-name and SDT events as Tracepoint events
>   even if it is not yet probed.

I can agree with 'perf list' showing what can be used as events, and
SDTs, AFAIK, match that definition, i.e. we have somewhere (in the DSOs,
right?) information about where to ask for an event to be enabled.

If there are details on how that needs to be obtained, then passed to
the kernel somehow to then become really, really accessible, then these
details are completely internal.

> # perf list
> 
> List of pre-defined events (to be used in -e):
> ...
>   libc:memory_heap_new                             [Tracepoint event]

Is it like this or is it like [ku]probes where we already have a
namespace qualifier, i.e.:

[root@zoo ~]# perf probe icmp_rcv
Added new event:
  probe:icmp_rcv       (on icmp_rcv)

You can now use it in all perf tools, such as:

	perf record -e probe:icmp_rcv -aR sleep 1

[root@zoo ~]#

[root@zoo ~]# perf probe /lib64/libc-2.20.so malloc
Added new events:
  probe_libc:malloc    (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_1  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_2  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_3  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_4  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_5  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_6  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_7  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_8  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_9  (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_10 (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_11 (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_12 (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_13 (on malloc in /lib64/libc-2.20.so)
  probe_libc:malloc_14 (on malloc in /lib64/libc-2.20.so)

You can now use it in all perf tools, such as:

	perf record -e probe_libc:malloc_14 -aR sleep 1

[root@zoo ~]#

"probe" for kernel events, "probe_%s" % DSO basename for userspace
events.

Why not continue with that and have SDTs use the probe_%s: namespace?
Sorry if this was already discussed here...

If there is some ambiguity, that can be resolved by explicitely setting
a new name, 'perf probe' has provision for that, right? I.e.:

[root@zoo ~]# perf probe /lib64/libc-2.20.so another_name=malloc
Added new events:
<SNIP>
  probe_libc:another_name_14 (on malloc in /lib64/libc-2.20.so)

You can now use it in all perf tools, such as:

	perf record -e probe_libc:another_name_14 -aR sleep 1

[root@zoo ~]#

> ...
>   probes:myevent                                   [Tracepoint event]
> ...
 
> 2) perf-record -e with no-probed event should try to set up the given probe
>  by using perf-probe. It is possible to remove that the probe after recording,
>  but also ignore if it fails by -EBUSY. (anyway, there is no difference for
>  users)

Right, being able to add new probes _without_ calling 'perf probe', but
instead functions used by 'perf probe' is something the eBPF does (I'm
almost getting there... :) ) and that I want to do in other tools, like
'trace' as well...

There are permission problems about how to add new probes and how to
_enable_ them, i.e. I think it is ok to allow users to ask for
probe:foo, if they are monitoring just their workloads...

[root@zoo ~]# perf probe 'vfs_getname=getname_flags:72 pathname=filename:string'
Added new event:
  probe:vfs_getname    (on getname_flags:72 with pathname=filename:string)

You can now use it in all perf tools, such as:

	perf record -e probe:vfs_getname -aR sleep 1

[root@zoo ~]#

Then, as !root:

  [acme@zoo linux]$ trace cat /etc/passwd
  Error:	No permissions to read /sys/kernel/debug/tracing/events/raw_syscalls/sys_(enter|exit)
  Hint:	Try 'sudo mount -o remount,mode=755 /sys/kernel/debug'

By default, !root users can't see debugfs, so, no dice in trying to use
the raw_syscalls tracepoints, bummer, then:

  [acme@zoo linux]$ sudo mount -o remount,mode=755 /sys/kernel/debug
  [sudo] password for acme: 
  [acme@zoo linux]$ trace cat /etc/passwd
  Error:	No permissions to read /sys/kernel/debug/tracing/events/raw_syscalls/sys_(enter|exit)
  Hint:	Try 'sudo mount -o remount,mode=755 /sys/kernel/debug/tracing'

Ouch, now this tracefs thing inside debugfs, at least sudo doesn't asks me for the
password this time!

But then, since 'perf trace' knows there is a probe:vfs_getname syscall in place, it
will try to use it, to monitor a workload it is about to start, but:

  [acme@zoo linux]$ sudo mount -o remount,mode=755 /sys/kernel/debug/tracing
  [acme@zoo linux]$ trace cat /etc/passwd
  Error:	Operation not permitted.
  Hint:	Check /proc/sys/kernel/perf_event_paranoid setting.
  Hint:	For system wide tracing it needs to be set to -1.
  Hint:	Try: 'sudo sh -c "echo -1 > /proc/sys/kernel/perf_event_paranoid"'
  Hint:	The current value is 1.
  [acme@zoo linux]$

This needs refining, i.e. I should just warn that albeit "probe:vfs_getname" is available,
it can't be used, unless we open the doors wide.

Sorry for the digression, but these permission issues will hit us with SDT as well, no?
 
> This rule will solve the contradiction between the event name on recorded
> data and listed events. However, as we discussed there are other clashes.

> A) clash among binaries: Since the binary builders can freely use the
> provider name, it is possible to clash to other binaries' SDTs.

Yes, one way to disambiguate is to use the buildid, i.e. content based, when/if
the need arises. We should _always_ store the build-id, together with any probe
used, so that we can bail out when trying to run those with a non matching DSO
(kernel, module, library, whatever).

When specifying some SDT that is ambiguous, we should bail out and ask for further
namespacing, like:

[acme@zoo linux]$ git show --oneline 25b6
error: short SHA1 25b6 is ambiguous.
error: short SHA1 25b6 is ambiguous.
fatal: ambiguous argument '25b6': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
[acme@zoo linux]$ git show --oneline 25b67
error: short SHA1 25b67 is ambiguous.
error: short SHA1 25b67 is ambiguous.
fatal: ambiguous argument '25b67': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'

It can be via build-id or by asking for the full pathname to the DSO, etc.
 
> B) clash among different versions: Of course the different versions of binaries
> can be co-exist on the system. Those usually have the same SDTs and same
> basename, just different build-ids.

Right, in that case the build-id or the full pathname needs to be specified somehow

> These issues are not solved by using "%" because it happens among SDTs.
> So we need to find another way to distinguish the SDTs.

From what I've read so far (not much): agreed.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1191584

Hi Masami and Arnaldo,

On Fri, Jul 24, 2015 at 01:24:53AM +0900, Masami Hiramatsu wrote:
> On 2015/07/23 23:01, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jul 23, 2015 at 10:13:22PM +0900, Masami Hiramatsu escreveu:
> >> # perf list
> >>
> >> List of pre-defined events (to be used in -e):
> >> ...
> >>   libc:memory_heap_new                             [Tracepoint event]
> > 
> > Is it like this or is it like [ku]probes where we already have a
> > namespace qualifier, i.e.:
> > 
> > [root@zoo ~]# perf probe icmp_rcv
> > Added new event:
> >   probe:icmp_rcv       (on icmp_rcv)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe:icmp_rcv -aR sleep 1
> > 
> > [root@zoo ~]#
> > 
> > [root@zoo ~]# perf probe /lib64/libc-2.20.so malloc
> > Added new events:
> >   probe_libc:malloc    (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_1  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_2  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_3  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_4  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_5  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_6  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_7  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_8  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_9  (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_10 (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_11 (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_12 (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_13 (on malloc in /lib64/libc-2.20.so)
> >   probe_libc:malloc_14 (on malloc in /lib64/libc-2.20.so)
> > 
> > You can now use it in all perf tools, such as:
> > 
> > 	perf record -e probe_libc:malloc_14 -aR sleep 1
> > 
> > [root@zoo ~]#
> > 
> > "probe" for kernel events, "probe_%s" % DSO basename for userspace
> > events.
> > 
> > Why not continue with that and have SDTs use the probe_%s: namespace?
> > Sorry if this was already discussed here...
> 
> :) We are discussing about that in another thread, anyway, probe_%s can
> solve a little part of the clash of names.
> 
> > 
> > If there is some ambiguity, that can be resolved by explicitely setting
> > a new name, 'perf probe' has provision for that, right? I.e.:
> 
> Yes, but that means we'll have to give new names before using that.
> 
> Actually, SDT has "provider-name", "event-name" and "probe location" (also
> have arguments, but not supported). And provider name is not always same
> as the binary name. (actually, the application developers can use any
> name for it...)
> So adding something special prefix or detect clash before using will
> be the option.
> 
> The following patterns we've discussed.
> 
>  - <provider>:<name>
> 	simple, but could easily clash with others.
>  - probe_<provider>:<name>
>  - sdt_<provider>:<name>
> 	also simple and similar to current solution. but fragile against
> 	clash among SDTs.
>  - probe_<binary>:<provider>_<name>
> 	also simple, but if provider or/and name has '_', it is hard to
> 	split the provider and name. and fragile against clash among SDTs too.
>  - <provider>_<buildid>/<name>
> 	possible, but ugly since buildid is a random long xdigits(maybe cut up
> 	to 8 or 12 bytes).

As I said, we might allow name clashes as they're rare.  I don't want
to make it complex just for an uncommon case.  I think such a
duplicate name is fine as long as 'perf list' indicates it and 'perf
record' enable them all.

If we agreed to extend the event format, I'd like to keep it simple
and to make it optional to add more info (separated by colon?).

Maybe something like below.  Suppose we have 3 SDT events with a same
name:

 /some/where/dir1/libfoo1.so (build-id: 0x1234...) -->  foo:bar
 /some/where/dir2/libfoo1.so (build-id: 0x5678...) -->  foo:bar
 /some/where/dir2/libfoo2.so (build-id: 0xabcd...) -->  foo:bar

So perf list shows the single name, but also says it has 3 events.

  $ perf list sdt_foo:bar
  
  sdt_foo:bar (total 3 events)            [User SDT event]


  $ perf list -v sdt_foo:bar
  
  sdt_foo:bar:libfoo1.so:0x1234...        [User SDT event]
  sdt_foo:bar:libfoo1.so:0x5678...        [User SDT event]
  sdt_foo:bar:libfoo2.so:0xabcd...        [User SDT event]


Now perf record can accept any of these forms..

  # record all 3 events
  $ perf record -e 'sdt_foo:bar'

  # record 2 events from libfoo1.so
  $ perf record -e 'sdt_foo:bar:libfoo1.so'

  # record only 1 event
  $ perf record -e 'sdt_foo:bar:libfoo1.so:0x1234...'


What do you think?

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193138

Hi Arnaldo,

On Fri, Jul 24, 2015 at 12:52:37PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jul 24, 2015 at 04:55:19PM +0900, Namhyung Kim escreveu:
> > On Fri, Jul 24, 2015 at 01:24:53AM +0900, Masami Hiramatsu wrote:
> > > On 2015/07/23 23:01, Arnaldo Carvalho de Melo wrote:
> > > > Em Thu, Jul 23, 2015 at 10:13:22PM +0900, Masami Hiramatsu escreveu:
> 
> > > The following patterns we've discussed.
> > > 
> > >  - <provider>:<name>
> > > 	simple, but could easily clash with others.
> > >  - probe_<provider>:<name>
> > >  - sdt_<provider>:<name>
> > > 	also simple and similar to current solution. but fragile against
> > > 	clash among SDTs.
> > >  - probe_<binary>:<provider>_<name>
> > > 	also simple, but if provider or/and name has '_', it is hard to
> > > 	split the provider and name. and fragile against clash among SDTs too.
> > >  - <provider>_<buildid>/<name>
> > > 	possible, but ugly since buildid is a random long xdigits(maybe cut up
> > > 	to 8 or 12 bytes).
>  
> > As I said, we might allow name clashes as they're rare.  I don't want
> > to make it complex just for an uncommon case.  I think such a
> > duplicate name is fine as long as 'perf list' indicates it and 'perf
> > record' enable them all.
> 
> I made some comments about enabling it all by default, look below.

OK.


>  
> > If we agreed to extend the event format, I'd like to keep it simple
> > and to make it optional to add more info (separated by colon?).
> 
> Reading this again after writing what is below: my suggestion is to use
> @, see rationale below.

I'm fine with using @.


>  
> > Maybe something like below.  Suppose we have 3 SDT events with a same
> > name:
> > 
> >  /some/where/dir1/libfoo1.so (build-id: 0x1234...) -->  foo:bar
> >  /some/where/dir2/libfoo1.so (build-id: 0x5678...) -->  foo:bar
> >  /some/where/dir2/libfoo2.so (build-id: 0xabcd...) -->  foo:bar
> > 
> > So perf list shows the single name, but also says it has 3 events.
> > 
> >   $ perf list sdt_foo:bar
> >   
> >   sdt_foo:bar (total 3 events)            [User SDT event]
> 
> I would show what desambiguates them in non verbose mode, i.e., the
> above would be:
> 
>    $ perf list sdt_foo:bar
> 
>    sdt_foo:bar:dir1/libfoo1.so   [User SDT event]
>    sdt_foo:bar:dir2/libfoo1.so   [User SDT event]
>    sdt_foo:bar:libfoo2.so        [User SDT event]

Then it should use @ here too.


> 
>  The -v one would should both the full path and the buildid, but this
> is just polishing up the default output a bit to make it more
> informative.

Fair enough.


> 
> 	Now what should be the default when one does:
> 
>    perf record -e sdt_foo:bar
> 
>         Will it enable all events or bail out and state that multiple
> events with that name matches, requiring a '--all-matches' to really
> apply it to all events with the same name?
> 
> 	Humm, this probably will not be that common, so perhaps just
> use all matches by default while telling the user that all those places
> were used and if the user wants just one of them, be more precise,
> adding somehow a disambiguator.

Either is fine to me.  Mayb we can add a config option to select the
default bahavior.. :)


> 
> 	That would be something like this:
> 
>     perf record -e sdt_foo:bar:0x1234
> 
> 	Or perhaps:
> 
>     perf record -e sdt_foo:bar@0x1234
> 
> 	Because in this case the 'at' meaning of '@' makes sense, i.e.
> use the std_foo:bar event at the DSO with a 0x1234 buildid?

IMHO @ looks perfect for pathnames but I don't know about build-id as
it can be thought as some address.  Anyway I still think @ is a good
choice though. ;-)


> 
> 	Additionally, for people that don't want to mess with buildids
> because its environment is deemed well controlled and this works and is
> unambiguous, looking at the LD_LIBRARY_PATH or equivalent:

Ah, good idea.


> 
>     perf record -e sdt_foo:bar@libfoo2
> 
> 	Full paths could be used as well.
> > 
> >   $ perf list -v sdt_foo:bar
> >   
> >   sdt_foo:bar:libfoo1.so:0x1234...        [User SDT event]
> >   sdt_foo:bar:libfoo1.so:0x5678...        [User SDT event]
> >   sdt_foo:bar:libfoo2.so:0xabcd...        [User SDT event]
> 
> > 
> > Now perf record can accept any of these forms..
> > 
> >   # record all 3 events
> >   $ perf record -e 'sdt_foo:bar'
> > 
> >   # record 2 events from libfoo1.so
> >   $ perf record -e 'sdt_foo:bar:libfoo1.so'
> > 
> >   # record only 1 event
> >   $ perf record -e 'sdt_foo:bar:libfoo1.so:0x1234...'
> > 
> > 
> > What do you think?
> 
> If nothing prevents using @ with the meaning of "event at LOCATION"
> where LOCATION is a buildid (noticed because it starts with 0x) or
> a library name or pathname, then that looks more natural.

Agreed.

Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1193666 — Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT support

On 2015/07/28 0:16, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jul 27, 2015 at 11:03:20PM +0900, Namhyung Kim escreveu:
>> On Fri, Jul 24, 2015 at 12:52:37PM -0300, Arnaldo Carvalho de Melo wrote:
>>>> If we agreed to extend the event format, I'd like to keep it simple
>>>> and to make it optional to add more info (separated by colon?).
>>>
>>> Reading this again after writing what is below: my suggestion is to use
>>> @, see rationale below.
>>
>> I'm fine with using @.
>  
>>> I would show what desambiguates them in non verbose mode, i.e., the
>>> above would be:
>>>
>>>    $ perf list sdt_foo:bar
>>>
>>>    sdt_foo:bar:dir1/libfoo1.so   [User SDT event]
>>>    sdt_foo:bar:dir2/libfoo1.so   [User SDT event]
>>>    sdt_foo:bar:libfoo2.so        [User SDT event]
>>
>> Then it should use @ here too.
> 
> Right.
>  
> <SNIP>
> 
>>> 	That would be something like this:
> 
>>>     perf record -e sdt_foo:bar@0x1234
> 
>>> 	Because in this case the 'at' meaning of '@' makes sense, i.e.
>>> use the std_foo:bar event at the DSO with a 0x1234 buildid?
>>
>> IMHO @ looks perfect for pathnames but I don't know about build-id as
>> it can be thought as some address.  Anyway I still think @ is a good
>> choice though. ;-)
> 
> Yeah, perhaps we need further clarification? I.e. something like:
> 
> 	sdt_foo:bar:libfoo1.so@buildid(0x1234)
> 
> Or something else, perhaps shorter, that clarifies that it is a buildid?

Hmm, Do we really need such additional buildid? Even though, I think
the build id should have different delimiter, like '%', as below.

sdt_foo:bar@libfoo1.so%buildid


Thank you,

-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web