From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755162AbbCFSGI (ORCPT ); Fri, 6 Mar 2015 13:06:08 -0500 Received: from mail.kernel.org ([198.145.29.136]:42333 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755058AbbCFSGF (ORCPT ); Fri, 6 Mar 2015 13:06:05 -0500 Date: Fri, 6 Mar 2015 15:06:04 -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: <20150306180604.GC5187@kernel.org> References: <20150306073118.6904.72740.stgit@localhost.localdomain> <20150306073127.6904.3232.stgit@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150306073127.6904.3232.stgit@localhost.localdomain> 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 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... - Arnaldo > Acked-by: Masami Hiramatsu > Signed-off-by: Namhyung Kim > --- > tools/perf/util/machine.c | 2 +- > tools/perf/util/map.c | 6 +++--- > tools/perf/util/map.h | 8 +++++++- > tools/perf/util/symbol-elf.c | 5 +++-- > tools/perf/util/symbol-minimal.c | 2 +- > tools/perf/util/symbol.c | 8 +++++--- > tools/perf/util/symbol.h | 5 +++-- > 7 files changed, 23 insertions(+), 13 deletions(-) > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > index 24f8c97..01ba9b6 100644 > --- a/tools/perf/util/machine.c > +++ b/tools/perf/util/machine.c > @@ -1128,7 +1128,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine, > * preload dso of guest kernel and modules > */ > dso__load(kernel, machine->vmlinux_maps[MAP__FUNCTION], > - NULL); > + NULL, false); > } > } > return 0; > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c > index 62ca9f2..711e072 100644 > --- a/tools/perf/util/map.c > +++ b/tools/perf/util/map.c > @@ -248,7 +248,7 @@ void map__fixup_end(struct map *map) > > #define DSO__DELETED "(deleted)" > > -int map__load(struct map *map, symbol_filter_t filter) > +int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias) > { > const char *name = map->dso->long_name; > int nr; > @@ -256,7 +256,7 @@ int map__load(struct map *map, symbol_filter_t filter) > if (dso__loaded(map->dso, map->type)) > return 0; > > - nr = dso__load(map->dso, map, filter); > + nr = dso__load(map->dso, map, filter, allow_alias); > if (nr < 0) { > if (map->dso->has_build_id) { > char sbuild_id[BUILD_ID_SIZE * 2 + 1]; > @@ -304,7 +304,7 @@ struct symbol *map__find_symbol(struct map *map, u64 addr, > struct symbol *map__find_symbol_by_name(struct map *map, const char *name, > symbol_filter_t filter) > { > - if (map__load(map, filter) < 0) > + if (__map__load(map, filter, true) < 0) > return NULL; > > if (!dso__sorted_by_name(map->dso, map->type)) > diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h > index 0e42438..ba15607 100644 > --- a/tools/perf/util/map.h > +++ b/tools/perf/util/map.h > @@ -149,7 +149,13 @@ size_t map__fprintf_dsoname(struct map *map, FILE *fp); > int map__fprintf_srcline(struct map *map, u64 addr, const char *prefix, > FILE *fp); > > -int map__load(struct map *map, symbol_filter_t filter); > +int __map__load(struct map *map, symbol_filter_t filter, bool allow_alias); > + > +static inline int map__load(struct map *map, symbol_filter_t filter) > +{ > + return __map__load(map, filter, false); > +} > + > struct symbol *map__find_symbol(struct map *map, > u64 addr, symbol_filter_t filter); > struct symbol *map__find_symbol_by_name(struct map *map, const char *name, > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index ada1676..fb630f8 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -754,7 +754,7 @@ static bool want_demangle(bool is_kernel_sym) > > int dso__load_sym(struct dso *dso, struct map *map, > struct symsrc *syms_ss, struct symsrc *runtime_ss, > - symbol_filter_t filter, int kmodule) > + symbol_filter_t filter, int kmodule, bool allow_alias) > { > struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL; > struct map *curr_map = map; > @@ -1048,7 +1048,8 @@ new_symbol: > * For misannotated, zeroed, ASM function sizes. > */ > if (nr > 0) { > - symbols__fixup_duplicate(&dso->symbols[map->type]); > + if (!allow_alias) > + symbols__fixup_duplicate(&dso->symbols[map->type]); > symbols__fixup_end(&dso->symbols[map->type]); > if (kmap) { > /* > diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c > index d7efb03..fefeeb3 100644 > --- a/tools/perf/util/symbol-minimal.c > +++ b/tools/perf/util/symbol-minimal.c > @@ -334,7 +334,7 @@ int dso__load_sym(struct dso *dso, struct map *map __maybe_unused, > struct symsrc *ss, > struct symsrc *runtime_ss __maybe_unused, > symbol_filter_t filter __maybe_unused, > - int kmodule __maybe_unused) > + int kmodule __maybe_unused, bool allow_alias __maybe_unused) > { > unsigned char *build_id[BUILD_ID_SIZE]; > int ret; > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index a690668..de72a16 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c > @@ -1345,7 +1345,8 @@ static bool dso__is_compatible_symtab_type(struct dso *dso, bool kmod, > } > } > > -int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter) > +int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter, > + bool allow_alias) > { > char *name; > int ret = -1; > @@ -1458,7 +1459,8 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter) > runtime_ss = syms_ss; > > if (syms_ss) > - ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod); > + ret = dso__load_sym(dso, map, syms_ss, runtime_ss, filter, kmod, > + allow_alias); > else > ret = -1; > > @@ -1516,7 +1518,7 @@ int dso__load_vmlinux(struct dso *dso, struct map *map, > if (symsrc__init(&ss, dso, symfs_vmlinux, symtab_type)) > return -1; > > - err = dso__load_sym(dso, map, &ss, &ss, filter, 0); > + err = dso__load_sym(dso, map, &ss, &ss, filter, 0, false); > symsrc__destroy(&ss); > > if (err > 0) { > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h > index 1650dcb..8299038 100644 > --- a/tools/perf/util/symbol.h > +++ b/tools/perf/util/symbol.h > @@ -218,7 +218,8 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name, > bool symsrc__has_symtab(struct symsrc *ss); > bool symsrc__possibly_runtime(struct symsrc *ss); > > -int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter); > +int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter, > + bool allow_alias); > int dso__load_vmlinux(struct dso *dso, struct map *map, > const char *vmlinux, bool vmlinux_allocated, > symbol_filter_t filter); > @@ -262,7 +263,7 @@ bool symbol__is_idle(struct symbol *sym); > > int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss, > struct symsrc *runtime_ss, symbol_filter_t filter, > - int kmodule); > + int kmodule, bool allow_alias); > int dso__synthesize_plt_symbols(struct dso *dso, struct symsrc *ss, > struct map *map, symbol_filter_t filter); >