Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964785AbbGYAwE (ORCPT ); Fri, 24 Jul 2015 20:52:04 -0400 Received: from mail9.hitachi.co.jp ([133.145.228.44]:42788 "EHLO mail9.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932075AbbGYAwB (ORCPT ); Fri, 24 Jul 2015 20:52:01 -0400 Message-ID: <55B2DDA8.3010700@hitachi.com> Date: Sat, 25 Jul 2015 09:51:52 +0900 From: Masami Hiramatsu Organization: Hitachi, Ltd., Japan User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Arnaldo Carvalho de Melo , Namhyung Kim CC: Hemant Kumar , Peter Zijlstra , linux-kernel@vger.kernel.org, Adrian Hunter , Ingo Molnar , Jiri Olsa , Borislav Petkov Subject: Re: [RFC PATCH perf/core v2 00/16] perf-probe --cache and SDT support 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> <20150724155237.GA300@kernel.org> In-Reply-To: <20150724155237.GA300@kernel.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5998 Lines: 167 On 2015/07/25 0:52, 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. >>> >>> - : >>> simple, but could easily clash with others. >>> - probe_: >>> - sdt_: >>> also simple and similar to current solution. but fragile against >>> clash among SDTs. >>> - probe_:_ >>> 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. >>> - _/ >>> 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. I agree that the short path is useful, but we know only full path how to make it short? (only show the differences?) > > 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? OK, but the problem is that the k/uprobe_event doesn't support multiple probe on one event yet. This means that if we set those 3 events, it will be translated to "sdt_foo:bar", "sdt_foo:bar_1", and "sdt_foo:bar_2". So we need to enhance k/uprobe_event too. Note that, if the event-name clash happens among events with different type of arguments, we can not bail it out... It is better to warn user if that happened. > 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? Ah, that's nice :) I like '@'. > 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. BTW, will we show it as "[User SDT event]" instead of "[Tracepoint]"? In that case, after setting the event, same name event will appear under tracefs/events/. I guess it conflicts with above SDT event. e.g. $ 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] And enables on libfoo2.so $ perf record -e sdt_foo:bar@libfoo2.so What the perf list shows $ perf list sdt_foo:bar sdt_foo:bar@libfoo2.so [Tracepoint] sdt_foo:bar@dir1/libfoo1.so [User SDT event] sdt_foo:bar@dir2/libfoo1.so [User SDT event] Is this OK? Or, we'll need to distinguish sdt_* from other events. 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/