Messages in this thread | | | From | Namhyung Kim <> | Date | Mon, 5 Feb 2024 18:21:46 -0800 | Subject | Re: [PATCH 2/2] perf symbols: Slightly improve module file executable section mappings |
| |
On Sun, Feb 4, 2024 at 10:58 PM Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 3/02/24 03:44, Namhyung Kim wrote: > > Hi Adrian, > > > > On Fri, Feb 02, 2024 at 01:01:30PM +0200, Adrian Hunter wrote: > >> Currently perf does not record module section addresses except for > >> the .text section. In general that means perf cannot get module section > >> mappings correct (except for .text) when loading symbols from a kernel > >> module file. (Note using --kcore does not have this issue) > >> > >> Improve that situation slightly by identifying executable sections that > >> use the same mapping as the .text section. That happens when an > >> executable section comes directly after the .text section, both in memory > >> and on file, something that can be determined by following the same layout > >> rules used by the kernel, refer kernel layout_sections(). Note whether > >> that happens is somewhat arbitrary, so this is not a final solution. > >> > >> Example from tracing a virtual machine process: > >> > >> Before: > >> > >> $ perf script | grep unknown > >> CPU 0/KVM 1718 203.511270: 318341 cpu-cycles:P: ffffffffc13e8a70 [unknown] (/lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko) > >> $ perf script -vvv 2>&1 >/dev/null | grep kvm.intel | grep 'noinstr.text\|ffff' > >> Map: 0-7e0 41430 [kvm_intel].noinstr.text > >> Map: ffffffffc13a7000-ffffffffc1421000 a0 /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko > >> > >> After: > >> > >> $ perf script | grep 203.511270 > >> CPU 0/KVM 1718 203.511270: 318341 cpu-cycles:P: ffffffffc13e8a70 vmx_vmexit+0x0 (/lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko) > >> $ perf script -vvv 2>&1 >/dev/null | grep kvm.intel | grep 'noinstr.text\|ffff' > >> Map: ffffffffc13a7000-ffffffffc1421000 a0 /lib/modules/6.7.2-local/kernel/arch/x86/kvm/kvm-intel.ko > >> > >> Reported-by: Like Xu <like.xu.linux@gmail.com> > >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > >> --- > >> tools/perf/util/symbol-elf.c | 75 +++++++++++++++++++++++++++++++++++- > >> 1 file changed, 73 insertions(+), 2 deletions(-) > >> > >> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elfc > >> index 9e7eeaf616b8..98bf0881aaf6 100644 > >> --- a/tools/perf/util/symbol-elf.c > >> +++ b/tools/perf/util/symbol-elf.c > >> @@ -23,6 +23,7 @@ > >> #include <linux/ctype.h> > >> #include <linux/kernel.h> > >> #include <linux/zalloc.h> > >> +#include <linux/string.h> > >> #include <symbol/kallsyms.h> > >> #include <internal/lib.h> > >> > >> @@ -1329,6 +1330,58 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name, > >> return -1; > >> } > >> > >> +static bool is_exe_text(int flags) > >> +{ > >> + return (flags & (SHF_ALLOC | SHF_EXECINSTR)) == (SHF_ALLOC | SHF_EXECINSTR); > >> +} > >> + > >> +/* > >> + * Some executable module sections like .noinstr.text might be laid out with > >> + * .text so they can use the same mapping (memory address to file offset). > >> + * Check if that is the case. Refer to kernel layout_sections(). Return the > >> + * maximum offset. > >> + */ > >> +static u64 max_text_section(Elf *elf, GElf_Ehdr *ehdr) > >> +{ > >> + Elf_Scn *sec = NULL; > >> + GElf_Shdr shdr; > >> + u64 offs = 0; > >> + > >> + /* Doesn't work for some arch */ > >> + if (ehdr->e_machine == EM_PARISC || > >> + ehdr->e_machine == EM_ALPHA) > >> + return 0; > >> + > >> + /* ELF is corrupted/truncated, avoid calling elf_strptr. */ > >> + if (!elf_rawdata(elf_getscn(elf, ehdr->e_shstrndx), NULL)) > >> + return 0; > >> + > >> + while ((sec = elf_nextscn(elf, sec)) != NULL) { > >> + char *sec_name; > >> + > >> + if (!gelf_getshdr(sec, &shdr)) > >> + break; > >> + > >> + if (!is_exe_text(shdr.sh_flags)) > >> + continue; > >> + > >> + /* .init and .exit sections are not placed with .text */ > >> + sec_name = elf_strptr(elf, ehdr->e_shstrndx, shdr.sh_name); > >> + if (!sec_name || > >> + strstarts(sec_name, ".init") || > >> + strstarts(sec_name, ".exit")) > >> + break; > > > > Do we really need this? It seems my module has .init.text section > > next to .text. > > > > $ readelf -SW /lib/modules/`uname -r`/kernel/fs/ext4/ext4.ko > > There are 77 section headers, starting at offset 0x252e90: > > > > Section Headers: > > [Nr] Name Type Address Off Size ES Flg Lk Inf Al > > [ 0] NULL 0000000000000000 000000 000000 00 0 0 0 > > [ 1] .text PROGBITS 0000000000000000 000040 079fa7 00 AX 0 0 16 > > [ 2] .rela.text RELA 0000000000000000 13c348 04f0c8 18 I 74 1 8 > > [ 3] .init.text PROGBITS 0000000000000000 079ff0 00060c 00 AX 0 0 16 > > ... > > > > > > ALIGN(0x40 + 0x79fa7, 16) = 0x79ff0, right? > > But not in memory e.g. > > Section Headers: > [Nr] Name Type Address Off Size ES Flg Lk Inf Al > [ 3] .text PROGBITS 0000000000000000 0000a0 071719 00 AX 0 0 16 > [ 5] .text.unlikely PROGBITS 0000000000000000 0717b9 000a59 00 AX 0 0 1 > [ 7] .init.text PROGBITS 0000000000000000 072212 0004fe 00 AX 0 0 1 > [ 9] .altinstr_replacement PROGBITS 0000000000000000 072710 000004 00 AX 0 0 1 > [10] .static_call.text PROGBITS 0000000000000000 072714 000388 00 AX 0 0 4 > [12] .exit.text PROGBITS 0000000000000000 072a9c 000078 00 AX 0 0 1 > > > /sys/module/ext4/sections/.text: 0xffffffffc0453000 > /sys/module/ext4/sections/.text.unlikely: 0xffffffffc04c4719 > /sys/module/ext4/sections/.init.text: 0xffffffffc053e000 > /sys/module/ext4/sections/.altinstr_replacement: 0xffffffffc04c5172 > /sys/module/ext4/sections/.static_call.text: 0xffffffffc04c5178 > /sys/module/ext4/sections/.exit.text: 0xffffffffc04c5500 > > Need to have: > > section address - offset == .text address - .text offset > > perf does not record the section address, but the kernel > layout_sections() lays out executable sections in order > starting with .text *until* it gets to .init* or .exit*.
Ok, thanks for the explanation!
Namhyung
| |