From mboxrd@z Thu Jan 1 00:00:00 1970 From: Namhyung Kim Subject: Re: [PATCH 1/2] perf report fix module symbol adjustment for s390x Date: Tue, 25 Jul 2017 15:14:10 +0900 Message-ID: <20170725061410.GA28831@sejong> References: <20170724143514.55574-1-tmricht@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Received: from LGEAMRELO11.lge.com ([156.147.23.51]:57654 "EHLO lgeamrelo11.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750743AbdGYGON (ORCPT ); Tue, 25 Jul 2017 02:14:13 -0400 Content-Disposition: inline In-Reply-To: <20170724143514.55574-1-tmricht@linux.vnet.ibm.com> Sender: linux-perf-users-owner@vger.kernel.org List-ID: To: Thomas Richter Cc: brueckner@linux.vnet.ibm.com, zvonko.kosic@de.ibm.com, linux-perf-users@vger.kernel.org, acme@kernel.org, kernel-team@lge.com Hello, On Mon, Jul 24, 2017 at 04:35:13PM +0200, Thomas Richter wrote: > The perf report tool does not display the addresses of kernel > module symbols correctly. > > For example symbol qeth_send_ipa_cmd in kernel module qeth.ko > has this relative address for function qeth_send_ipa_cmd() > [root@s8360047 linux]# nm -g drivers/s390/net/qeth.ko | fgrep send_ipa_cmd > 0000000000013088 T qeth_send_ipa_cmd > > The module is loaded at address > [root@s8360047 linux]# cat /sys/module/qeth/sections/.text > 0x000003ff80296d20 > [root@s8360047 linux]# > > This should result in a start address of > 0x13088 + 0x3ff80296d20 = 0x3ff802a9da8 > > Using crash to verify the address on a live system: > [root@s8360046 linux]# crash vmlinux > > crash 7.1.9++ > Copyright (C) 2002-2016 Red Hat, Inc. > Copyright (C) 2004, 2005, 2006, 2010 IBM Corporation > > [...] > > crash> mod -s qeth drivers/s390/net/qeth.ko > MODULE NAME SIZE OBJECT FILE > 3ff8028d700 qeth 151552 drivers/s390/net/qeth.ko > crash> sym qeth_send_ipa_cmd > 3ff802a9da8 (T) qeth_send_ipa_cmd [qeth] /root/linux/drivers/s390/net/qeth_core_main.c: 2944 > crash> > > Now perf report displays the address of symbol qeth_send_ipa_cmd: > symbol__new: qeth_send_ipa_cmd 0x130f0-0x132ce > > There is a difference of 0x68 between the entry in the > symbol table (see nm command above) and perf. The difference is from > the offset the .text segment of qeth.ko: > > [root@s8360047 perf]# readelf -a drivers/s390/net/qeth.ko > Section Headers: > [Nr] Name Type Address Offset > Size EntSize Flags Link Info Align > [ 0] NULL 0000000000000000 00000000 > 0000000000000000 0000000000000000 0 0 0 > [ 1] .note.gnu.build-i NOTE 0000000000000000 00000040 > 0000000000000024 0000000000000000 A 0 0 4 > [ 2] .text PROGBITS 0000000000000000 00000068 > 000000000001c8a0 0000000000000000 AX 0 0 8 > > As seen the .text segment has an offset of 0x68 with start address 0x0. > Therefore 0x68 is added to the address of qeth_send_ipa_cmd and thus > 0x13088 + 0x68 = 0x130f0 is displayed. > > This is wrong, perf report needs to display the start address of > symbol qeth_send_ipa_cmd at 0x13088 + qeth.ko.text section start address. > > The qeth.ko module .text start address is available in the qeth.ko > DSO map. Just identify the kernel module symbols and correct the > addresses. > > With the fix I see this correct address for symbol: > symbol__new: qeth_send_ipa_cmd 0x3ff802a9da8-0x3ff802a9f86 > > I am not sure if this patch uses the perfect approach. > Hints and sugguests welcome. I saw this problem on x86_64 too and I don't know why we add sh_offset to symbols. To me, current symbol code is somewhat hacky and needs some love. Thanks, Namhyung > > Signed-off-by: Thomas Richter > --- > tools/perf/arch/s390/util/sym-handling.c | 7 +++++++ > tools/perf/util/symbol-elf.c | 8 +++++++- > tools/perf/util/symbol.h | 3 +++ > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/arch/s390/util/sym-handling.c b/tools/perf/arch/s390/util/sym-handling.c > index b6cd056..79bf238 100644 > --- a/tools/perf/arch/s390/util/sym-handling.c > +++ b/tools/perf/arch/s390/util/sym-handling.c > @@ -19,4 +19,11 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) > return ehdr.e_type == ET_REL || ehdr.e_type == ET_DYN; > } > > +u64 arch__sym_set_address(GElf_Sym *sym, GElf_Shdr *shdr __maybe_unused, > + struct map *map) > +{ > + if (map->type == MAP__FUNCTION) > + sym->st_value += map->start; > + return sym->st_value; > +} > #endif > diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c > index 502505c..d3521d4 100644 > --- a/tools/perf/util/symbol-elf.c > +++ b/tools/perf/util/symbol-elf.c > @@ -793,6 +793,12 @@ static u64 ref_reloc(struct kmap *kmap) > void __weak arch__sym_update(struct symbol *s __maybe_unused, > GElf_Sym *sym __maybe_unused) { } > > +u64 __weak arch__sym_set_address(GElf_Sym *sym, GElf_Shdr *shdr, > + struct map *map __maybe_unused) > +{ > + return sym->st_value - (shdr->sh_addr - shdr->sh_offset); > +} > + > int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss, > struct symsrc *runtime_ss, int kmodule) > { > @@ -973,7 +979,7 @@ int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss, > > /* Adjust symbol to map to file offset */ > if (adjust_kernel_syms) > - sym.st_value -= shdr.sh_addr - shdr.sh_offset; > + sym.st_value = arch__sym_set_address(&sym, &shdr, map); > > if (strcmp(section_name, > (curr_dso->short_name + > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h > index 41ebba9..beb8737 100644 > --- a/tools/perf/util/symbol.h > +++ b/tools/perf/util/symbol.h > @@ -343,6 +343,9 @@ int setup_intlist(struct intlist **list, const char *list_str, > #ifdef HAVE_LIBELF_SUPPORT > bool elf__needs_adjust_symbols(GElf_Ehdr ehdr); > void arch__sym_update(struct symbol *s, GElf_Sym *sym); > +u64 arch__sym_set_address(GElf_Sym *sym, > + GElf_Shdr *shdr __maybe_unused, > + struct map *map __maybe_unused); > #endif > > #define SYMBOL_A 0 > -- > 2.9.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html