From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755531AbbCFSc6 (ORCPT ); Fri, 6 Mar 2015 13:32:58 -0500 Received: from mail.kernel.org ([198.145.29.136]:43956 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753979AbbCFScw (ORCPT ); Fri, 6 Mar 2015 13:32:52 -0500 Date: Fri, 6 Mar 2015 15:32:52 -0300 From: Arnaldo Carvalho de Melo To: Masami Hiramatsu Cc: Naohiro Aota , Peter Zijlstra , Linux Kernel Mailing List , David Ahern , namhyung@kernel.org, Jiri Olsa , Ingo Molnar Subject: Re: [PATCH perf/core v2 4/5] perf symbols: Allow symbol alias when loading map for symbol name Message-ID: <20150306183252.GE5187@kernel.org> References: <20150306073118.6904.72740.stgit@localhost.localdomain> <20150306073127.6904.3232.stgit@localhost.localdomain> <20150306180604.GC5187@kernel.org> <20150306182630.GD5187@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150306182630.GD5187@kernel.org> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Fri, Mar 06, 2015 at 03:26:30PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Mar 06, 2015 at 03:06:04PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Fri, Mar 06, 2015 at 04:31:27PM +0900, Masami Hiramatsu escreveu: > > > From: Namhyung Kim > > > > > > When perf probe tries to add a probe in a binary using symbol name, it > > > sometimes failed since some symbols were discard during loading dso. > > > When it resolves an address to symbol, it'd be better to have just one > > > symbol at given address. But for finding address from symbol, it'd be > > > better to keep all names (including aliases). > > > > > > Add and propagate a new allow_alias argument to dso (and map) load > > > functions so that it can keep those duplicate symbol aliases. > > > > Isn't this a global knob, i.e. one that a tool, such as 'perf probe' > > flips at startup and leave it turned on while other tools don't need to > > care about it? > > > > If so, we should use symbol_conf for that, no? I.e. > > symbol_conf.allow_aliases? > > > > Looking at doing that now... > > Ugh, I was looking at when was that knob to flipped on in this patch, it > is when one calls map__find_symbol_by_name(), when it assumes that the > map is not loaded yet and thus asks for duplicates not to be eliminated. > > But what happens if the map was already loaded by, say, > map__find_symbol(), i.e. to find by address? Then that 'true' in the > __map__load() call from map__find_symbol_by_name will be a nop. > > So I think it is really better for this to be a > symbol_conf.allow_aliases that tools set at startup, and perhaps even > this could even be governed by symbol_conf.sort_by_name, but I'll leave > alow_aliases for now, its more flexible. For reference, below is what I have now in my tree, if you are OK with it that is what I'll push to Ingo, - Arnaldo commit 3dae901ca3eddbd76bc20f47e0019c712ade7d14 Author: Namhyung Kim Date: Fri Mar 6 16:31:27 2015 +0900 perf symbols: Allow symbol alias when loading map for symbol name When perf probe tries to add a probe in a binary using symbol name, it sometimes failed since some symbols were discard during loading dso. When it resolves an address to symbol, it'd be better to have just one symbol at given address. But for finding address from symbol, it'd be better to keep all names (including aliases). So allow tools to state that they want to allow aliases via symbol_conf.allow_aliases. Signed-off-by: Namhyung Kim Acked-by: Masami Hiramatsu Cc: David Ahern Cc: Ingo Molnar Cc: Jiri Olsa Cc: Naohiro Aota Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/20150306073127.6904.3232.stgit@localhost.localdomain [ Original patch passwd allow_alias to many functions, use symbol_conf.allow_aliases instead ] Signed-off-by: Arnaldo Carvalho de Melo diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index c379ea0edfd5..9feba0e3343e 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -80,6 +80,7 @@ static int init_symbol_maps(bool user_only) int ret; symbol_conf.sort_by_name = true; + symbol_conf.allow_aliases = true; ret = symbol__init(NULL); if (ret < 0) { pr_debug("Failed to init symbol map.\n"); diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index ada16762fac2..62742e46c010 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -1048,7 +1048,8 @@ new_symbol: * For misannotated, zeroed, ASM function sizes. */ if (nr > 0) { - symbols__fixup_duplicate(&dso->symbols[map->type]); + if (!symbol_conf.allow_aliases) + symbols__fixup_duplicate(&dso->symbols[map->type]); symbols__fixup_end(&dso->symbols[map->type]); if (kmap) { /* diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h index 1650dcb3a67b..efdaaa544041 100644 --- a/tools/perf/util/symbol.h +++ b/tools/perf/util/symbol.h @@ -87,6 +87,7 @@ struct symbol_conf { ignore_vmlinux_buildid, show_kernel_path, use_modules, + allow_aliases, sort_by_name, show_nr_samples, show_total_period,