From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas-Mich Richter Subject: Re: [PATCH 1/2] perf report fix module symbol adjustment for s390x Date: Tue, 25 Jul 2017 10:49:24 +0200 Message-ID: <8fb23780-a67f-73b1-7f37-3d57271c3e87@linux.vnet.ibm.com> References: <20170724143514.55574-1-tmricht@linux.vnet.ibm.com> <20170725061410.GA28831@sejong> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:41636 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750835AbdGYItb (ORCPT ); Tue, 25 Jul 2017 04:49:31 -0400 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v6P8mjDo050486 for ; Tue, 25 Jul 2017 04:49:31 -0400 Received: from e06smtp14.uk.ibm.com (e06smtp14.uk.ibm.com [195.75.94.110]) by mx0a-001b2d01.pphosted.com with ESMTP id 2bwrncq81u-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 25 Jul 2017 04:49:30 -0400 Received: from localhost by e06smtp14.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 25 Jul 2017 09:49:28 +0100 In-Reply-To: <20170725061410.GA28831@sejong> Content-Language: en-IE Sender: linux-perf-users-owner@vger.kernel.org List-ID: To: Namhyung Kim Cc: brueckner@linux.vnet.ibm.com, zvonko.kosic@de.ibm.com, linux-perf-users@vger.kernel.org, acme@kernel.org, kernel-team@lge.com On 07/25/2017 08:14 AM, Namhyung Kim wrote: > 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 > Okay, how do we continue with this issue? The patch I submitted uses a __weak function of s390x and does not address other platforms. > >> >> 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 > -- > 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 > -- Thomas Richter, Dept 3303, IBM LTC Boeblingen Germany -- Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294