From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754357AbbGXPwq (ORCPT <rfc822;w@1wt.eu>);
	Fri, 24 Jul 2015 11:52:46 -0400
Received: from mail.kernel.org ([198.145.29.136]:39230 "EHLO mail.kernel.org"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751042AbbGXPwo (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Fri, 24 Jul 2015 11:52:44 -0400
Date: Fri, 24 Jul 2015 12:52:37 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
        Hemant Kumar <hemant@linux.vnet.ibm.com>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>, linux-kernel@vger.kernel.org,
        Adrian Hunter <adrian.hunter@intel.com>,
        Ingo Molnar <mingo@redhat.com>, Jiri Olsa <jolsa@kernel.org>,
        Borislav Petkov <bp@suse.de>
Subject: Re: Re: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and
 SDT support
Message-ID: <20150724155237.GA300@kernel.org>
References: <20150715091352.8915.87480.stgit@localhost.localdomain>
 <55A7215F.40803@linux.vnet.ibm.com>
 <55A874C6.5030202@hitachi.com>
 <55AFA4E2.4040801@linux.vnet.ibm.com>
 <55B0E872.8030206@hitachi.com>
 <20150723140127.GD3152@kernel.org>
 <55B11555.9060100@hitachi.com>
 <20150724075519.GA19672@sejong>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150724075519.GA19672@sejong>
X-Url: http://acmel.wordpress.com
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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

 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.

	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.

	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?

	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:

    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.

- Arnaldo