| Message ID | 1459195592-6559-1-git-send-email-bauerman@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
Am Montag, 28 März 2016, 17:06:32 schrieb Thiago Jung Bauermann: > /home/bauermann/src/linux/tools/testing/selftests/ftrace/test.d/kprobe/kp > robe_ftrace.tc: line 9: echo: write error: Invalid argument > > But even without my patch that testcase fails in the same way, so this > is not a problem introduced by the patch. That failure is on this line of kprobe_ftrace.tc: echo _do_fork > set_ftrace_filter This fails because on powerpc the function symbol has a dot prepended to its name: # cat available_filter_functions | grep _do_fork ._do_fork If I do s/_do_fork/._do_fork/ in kprobe_ftrace.tc then all ftrace kprobe tests pass: $ sudo ./ftracetest test.d/kprobe/ === Ftrace unit tests === [1] Kprobe dynamic event - adding and removing [PASS] [2] Kprobe dynamic event - busy event check [PASS] [3] Kprobe dynamic event with arguments [PASS] [4] Kprobe dynamic event with function tracer [PASS] [5] Kretprobe dynamic event with arguments [PASS] # of passed: 5 # of failed: 0 # of unresolved: 0 # of untested: 0 # of unsupported: 0 # of xfailed: 0 # of undefined(test bug): 0
On Mon, 2016-03-28 at 17:29 -0300, Thiago Jung Bauermann wrote: > Am Montag, 28 März 2016, 17:06:32 schrieb Thiago Jung Bauermann: > > /home/bauermann/src/linux/tools/testing/selftests/ftrace/test.d/kprobe/kp > > robe_ftrace.tc: line 9: echo: write error: Invalid argument > > > > But even without my patch that testcase fails in the same way, so this > > is not a problem introduced by the patch. > > That failure is on this line of kprobe_ftrace.tc: > > echo _do_fork > set_ftrace_filter > > This fails because on powerpc the function symbol has a dot prepended to its > name: Only on big endian powerpc. (Or actually ABI < ELFv2, but in practice that means BE) > # cat available_filter_functions | grep _do_fork > ._do_fork > > If I do s/_do_fork/._do_fork/ in kprobe_ftrace.tc then all ftrace kprobe > tests pass: > > $ sudo ./ftracetest test.d/kprobe/ > === Ftrace unit tests === > [1] Kprobe dynamic event - adding and removing [PASS] > [2] Kprobe dynamic event - busy event check [PASS] > [3] Kprobe dynamic event with arguments [PASS] > [4] Kprobe dynamic event with function tracer [PASS] > [5] Kretprobe dynamic event with arguments [PASS] OK. We fixed that in 'perf probe', but not if you're using the sysfs file directly. Do you want to write a patch for ftracetest to try and handle it? I guess you'd try "_do_fork" and if that fails then try "._do_fork", and maybe only if uname -m says you're running on ppc64? cheers
On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote: > Fixes the following testsuite failure: > > $ sudo ./perf test -v kallsyms > 1: vmlinux symtab matches kallsyms : > --- start --- > test child forked, pid 12489 > Using /proc/kcore for kernel object code > Looking at the vmlinux_path (8 entries long) > Using /boot/vmlinux for symbols > 0xc00000000003d300: diff name v: .kretprobe_trampoline_holder k: kretprobe_trampoline > Maps only in vmlinux: > c00000000086ca38-c000000000879b6c 87ca38 [kernel].text.unlikely > c000000000879b6c-c000000000bf0000 889b6c [kernel].meminit.text > c000000000bf0000-c000000000c53264 c00000 [kernel].init.text > c000000000c53264-d000000004250000 c63264 [kernel].exit.text > d000000004250000-d000000004450000 0 [libcrc32c] > d000000004450000-d000000004620000 0 [xfs] > d000000004620000-d000000004680000 0 [autofs4] > d000000004680000-d0000000046e0000 0 [x_tables] > d0000000046e0000-d000000004780000 0 [ip_tables] > d000000004780000-d0000000047e0000 0 [rng_core] > d0000000047e0000-ffffffffffffffff 0 [pseries_rng] > Maps in vmlinux with a different name in kallsyms: > Maps only in kallsyms: > d000000000000000-f000000000000000 1000000000010000 [kernel.kallsyms] > f000000000000000-ffffffffffffffff 3000000000010000 [kernel.kallsyms] > test child finished with -1 > ---- end ---- > vmlinux symtab matches kallsyms: FAILED! > > The problem is that the kretprobe_trampoline symbol looks like this: > > $ eu-readelf -s /boot/vmlinux G kretprobe_trampoline > 2431: c000000001302368 24 NOTYPE LOCAL DEFAULT 37 kretprobe_trampoline_holder > 2432: c00000000003d300 8 FUNC LOCAL DEFAULT 1 .kretprobe_trampoline_holder > 97543: c00000000003d300 0 NOTYPE GLOBAL DEFAULT 1 kretprobe_trampoline > > Its type is NOTYPE, and its size is 0, and this is a problem because > symbol-elf.c:dso__load_sym skips function symbols that are not STT_FUNC > or STT_GNU_IFUNC (this is determined by elf_sym__is_function). Even > if the type is changed to STT_FUNC, when dso__load_sym calls > symbols__fixup_duplicate, the kretprobe_trampoline symbol is dropped in > favour of .kretprobe_trampoline_holder because the latter has non-zero > size (as determined by choose_best_symbol). > > With this patch, all vmlinux symbols match /proc/kallsyms and the > testcase passes. Have you tested this on an LE system? I did a quick test and it appears to be completely broken. Basically every symbol is not found, eg: 1: vmlinux symtab matches kallsyms : --- start --- test child forked, pid 16222 Using /proc/kcore for kernel object code Looking at the vmlinux_path (8 entries long) Using /boot/vmlinux-4.5.0-11318-gdf01bc5cf4ea for symbols 0xc00000000000b428: run_init_process not on kallsyms 0xc00000000000b478: try_to_run_init_process not on kallsyms 0xc00000000000b4f8: do_one_initcall not on kallsyms 0xc00000000000b768: match_dev_by_uuid not on kallsyms ... 0xc0000000009846b8: write_mem not on kallsyms 0xc000000000984728: do_fp_store not on kallsyms 0xc000000000984828: emulate_step not on kallsyms $ sudo grep emulate_step /proc/kallsyms c000000000984820 T emulate_step Notice it's off by 8. That's because of the local/global entry point on LE. So that will need to be fixed somewhere. cheers
Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman: > On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote: > > With this patch, all vmlinux symbols match /proc/kallsyms and the > > testcase passes. > > Have you tested this on an LE system? No, I was focusing on ppc64 BE. > I did a quick test and it appears to > be completely broken. Basically every symbol is not found, eg: > > 1: vmlinux symtab matches kallsyms : > --- start --- > test child forked, pid 16222 > Using /proc/kcore for kernel object code > Looking at the vmlinux_path (8 entries long) > Using /boot/vmlinux-4.5.0-11318-gdf01bc5cf4ea for symbols > 0xc00000000000b428: run_init_process not on kallsyms > 0xc00000000000b478: try_to_run_init_process not on kallsyms > 0xc00000000000b4f8: do_one_initcall not on kallsyms > 0xc00000000000b768: match_dev_by_uuid not on kallsyms > ... > 0xc0000000009846b8: write_mem not on kallsyms > 0xc000000000984728: do_fp_store not on kallsyms > 0xc000000000984828: emulate_step not on kallsyms > > $ sudo grep emulate_step /proc/kallsyms > c000000000984820 T emulate_step > > > Notice it's off by 8. That's because of the local/global entry point on > LE. So that will need to be fixed somewhere. Symbols loaded from the vmlinux file get adjusted to the local entry point because symbol-elf.c:dso__load_sym calls arch__elf_sym_adjust for each function symbol, which on ppc64le adjusts the address to the local entry point. On the other hand, when symbols are loaded from /proc/kallsyms they are used as-is (i.e., with the global entry point address). This seems inconsistent: dso__load_kernel_sym can end up loading symbols from either vmlinux or /proc/kallsyms, so it seems symbols should look the same wherever they are loaded from. I am still trying to understand what the rest of the perf code expects. If I comment out the call to arch__elf_sym_adjust in dso__load_sym, then all symbols are found in the vmlinux-kallsyms.c test (the test still fails because two symbols have different names. I haven’t checked why). Also, the code-reading.c test is able to read object code for a lot (but not all) of the sample events, whereas in the adjusted symbols case it can’t read object code for any sample event. The other perf tests are unaffected by the change. I thought that if there’s no vmlinux available, then perf would have to rely only on /proc/kallsyms and I would see some difference in the test results compared to when perf uses vmlinux and adjusts the symbols to the local entry point, but I saw no difference at all. So at first glance, it looks like perf is better off using symbols that point to the global entry point...
On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote: > Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman: > > On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote: > > > With this patch, all vmlinux symbols match /proc/kallsyms and the > > > testcase passes. > > > > Have you tested this on an LE system? > > No, I was focusing on ppc64 BE. Which kernel did you use? I don't see this with the latest 4.6.0-rc1. > > > I did a quick test and it appears to > > be completely broken. Basically every symbol is not found, eg: > > > > 1: vmlinux symtab matches kallsyms : > > --- start --- > > test child forked, pid 16222 > > Using /proc/kcore for kernel object code > > Looking at the vmlinux_path (8 entries long) > > Using /boot/vmlinux-4.5.0-11318-gdf01bc5cf4ea for symbols > > 0xc00000000000b428: run_init_process not on kallsyms > > 0xc00000000000b478: try_to_run_init_process not on kallsyms > > 0xc00000000000b4f8: do_one_initcall not on kallsyms > > 0xc00000000000b768: match_dev_by_uuid not on kallsyms > > ... > > 0xc0000000009846b8: write_mem not on kallsyms > > 0xc000000000984728: do_fp_store not on kallsyms > > 0xc000000000984828: emulate_step not on kallsyms > > > > $ sudo grep emulate_step /proc/kallsyms > > c000000000984820 T emulate_step > > > > > > Notice it's off by 8. That's because of the local/global entry point on > > LE. So that will need to be fixed somewhere. > > Symbols loaded from the vmlinux file get adjusted to the local entry point > because symbol-elf.c:dso__load_sym calls arch__elf_sym_adjust for each > function symbol, which on ppc64le adjusts the address to the local entry > point. On the other hand, when symbols are loaded from /proc/kallsyms > they are used as-is (i.e., with the global entry point address). > > This seems inconsistent: dso__load_kernel_sym can end up loading symbols > from either vmlinux or /proc/kallsyms, so it seems symbols should look the > same wherever they are loaded from. I am still trying to understand what the > rest of the perf code expects. The LEP is used since probing on that catches calls to both the GEP and the LEP. I did fix 'perf probe' to fixup the kallsyms address as well. See commit 7b6ff0bdb. > > If I comment out the call to arch__elf_sym_adjust in dso__load_sym, then all > symbols are found in the vmlinux-kallsyms.c test (the test still fails > because two symbols have different names. I haven’t checked why). Also, the > code-reading.c test is able to read object code for a lot (but not all) of > the sample events, whereas in the adjusted symbols case it can’t read object > code for any sample event. The other perf tests are unaffected by the > change. > > I thought that if there’s no vmlinux available, then perf would have to rely > only on /proc/kallsyms and I would see some difference in the test results > compared to when perf uses vmlinux and adjusts the symbols to the local > entry point, but I saw no difference at all. > > So at first glance, it looks like perf is better off using symbols that > point to the global entry point... See above - we need to use the LEP. I suspect we don't fixup all the symbols in kallsyms -- just the ones being probed which explains why there is a mismatch. Let me take a look. - Naveen
On 2016/03/30 01:34PM, Naveen N Rao wrote: > On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote: > > Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman: > > > On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote: > > So at first glance, it looks like perf is better off using symbols > > that point to the global entry point... > > See above - we need to use the LEP. I suspect we don't fixup all the > symbols in kallsyms -- just the ones being probed which explains why > there is a mismatch. Let me take a look. Indeed. With vmlinux, we use the symbol table to identify symbols with a LEP, and we adjust those symbols during load time. However, with kallsyms, we don't have any data around which symbols have a LEP and which don't. So, we can't fixup all the entries globally. - Naveen
On Wed, 2016-03-30 at 13:34 +0530, Naveen N. Rao wrote: > On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote: > > Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman: > > > On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote: > > > > With this patch, all vmlinux symbols match /proc/kallsyms and the > > > > testcase passes. > > > > > > Have you tested this on an LE system? > > > > No, I was focusing on ppc64 BE. > > Which kernel did you use? I don't see this with the latest 4.6.0-rc1. Yeah I can't reproduce the perf test failure either. Not sure what's going on? cheers
Am Mittwoch, 30 März 2016, 20:09:36 schrieb Michael Ellerman: > On Wed, 2016-03-30 at 13:34 +0530, Naveen N. Rao wrote: > > On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote: > > > Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman: > > > > On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote: > > > > > With this patch, all vmlinux symbols match /proc/kallsyms and the > > > > > testcase passes. > > > > > > > > Have you tested this on an LE system? > > > > > > No, I was focusing on ppc64 BE. > > > > Which kernel did you use? I don't see this with the latest 4.6.0-rc1. > > Yeah I can't reproduce the perf test failure either. I can still reproduce on 4.6.0-rc1. > Not sure what's going on? Try disabling CONFIG_FUNCTION_TRACER. The test failure happens when .kretprobe_trampoline_holder and kretprobe_trampoline have the same address. If that option is enabled, the symbols have different addresses: $ eu-readelf -s /boot/vmlinux-4.6.0-rc1-dirty | grep kretprobe_tramp 2479: c000000000ef1d68 24 NOTYPE LOCAL DEFAULT 37 kretprobe_trampoline_holder 2480: c000000000043ab0 40 FUNC LOCAL DEFAULT 1 .kretprobe_trampoline_holder 86903: c000000000043ac4 0 NOTYPE GLOBAL DEFAULT 1 kretprobe_trampoline $ grep kretprobe_tramp /proc/kallsyms c000000000043ab0 t .kretprobe_trampoline_holder c000000000043ac4 T kretprobe_trampoline c000000000ef1d68 d kretprobe_trampoline_holder
On 2016/03/28 05:06PM, Thiago Jung Bauermann wrote: > Fixes the following testsuite failure: > > $ sudo ./perf test -v kallsyms > 1: vmlinux symtab matches kallsyms : > --- start --- > test child forked, pid 12489 > Using /proc/kcore for kernel object code > Looking at the vmlinux_path (8 entries long) > Using /boot/vmlinux for symbols > 0xc00000000003d300: diff name v: .kretprobe_trampoline_holder k: kretprobe_trampoline > Maps only in vmlinux: > c00000000086ca38-c000000000879b6c 87ca38 [kernel].text.unlikely > c000000000879b6c-c000000000bf0000 889b6c [kernel].meminit.text > c000000000bf0000-c000000000c53264 c00000 [kernel].init.text > c000000000c53264-d000000004250000 c63264 [kernel].exit.text > d000000004250000-d000000004450000 0 [libcrc32c] > d000000004450000-d000000004620000 0 [xfs] > d000000004620000-d000000004680000 0 [autofs4] > d000000004680000-d0000000046e0000 0 [x_tables] > d0000000046e0000-d000000004780000 0 [ip_tables] > d000000004780000-d0000000047e0000 0 [rng_core] > d0000000047e0000-ffffffffffffffff 0 [pseries_rng] > Maps in vmlinux with a different name in kallsyms: > Maps only in kallsyms: > d000000000000000-f000000000000000 1000000000010000 [kernel.kallsyms] > f000000000000000-ffffffffffffffff 3000000000010000 [kernel.kallsyms] > test child finished with -1 > ---- end ---- > vmlinux symtab matches kallsyms: FAILED! You should indent the above output -- in this specific case, the start marker interferes with git am. Apart from that, I have tested this patch and kretprobe works fine. A minor nit I had is that we end up with a non-dot function in .text without a corresponding function descriptor for kretprobe_trampoline. But, since this is a trampoline, I think that is good. So, for this patch: Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> - Naveen
Am Donnerstag, 31 März 2016, 13:53:11 schrieb Naveen N. Rao: > You should indent the above output -- in this specific case, the start > marker interferes with git am. > > Apart from that, I have tested this patch and kretprobe works fine. A > minor nit I had is that we end up with a non-dot function in .text > without a corresponding function descriptor for kretprobe_trampoline. > But, since this is a trampoline, I think that is good. So, for this > patch: > Reviewed-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com> Thanks for the tips and the review. I fixed the issue you mentioned and sent a new patch with your Reviewed-by. Also thanks for fixing the vmlinux-kallsyms issue in ppc64le. I didn’t comment on the patches because I’m not very familiar with the code in question.
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c index 7c053f2..417c0ea 100644 --- a/arch/powerpc/kernel/kprobes.c +++ b/arch/powerpc/kernel/kprobes.c @@ -278,12 +278,11 @@ no_kprobe: * - When the probed function returns, this probe * causes the handlers to fire */ -static void __used kretprobe_trampoline_holder(void) -{ - asm volatile(".global kretprobe_trampoline\n" - "kretprobe_trampoline:\n" - "nop\n"); -} +asm(".global kretprobe_trampoline\n" + ".type kretprobe_trampoline, @function\n" + "kretprobe_trampoline:\n" + "nop\n" + ".size kretprobe_trampoline, .-kretprobe_trampoline\n"); /* * Called when the probe at kretprobe trampoline is hit
Fixes the following testsuite failure: $ sudo ./perf test -v kallsyms 1: vmlinux symtab matches kallsyms : --- start --- test child forked, pid 12489 Using /proc/kcore for kernel object code Looking at the vmlinux_path (8 entries long) Using /boot/vmlinux for symbols 0xc00000000003d300: diff name v: .kretprobe_trampoline_holder k: kretprobe_trampoline Maps only in vmlinux: c00000000086ca38-c000000000879b6c 87ca38 [kernel].text.unlikely c000000000879b6c-c000000000bf0000 889b6c [kernel].meminit.text c000000000bf0000-c000000000c53264 c00000 [kernel].init.text c000000000c53264-d000000004250000 c63264 [kernel].exit.text d000000004250000-d000000004450000 0 [libcrc32c] d000000004450000-d000000004620000 0 [xfs] d000000004620000-d000000004680000 0 [autofs4] d000000004680000-d0000000046e0000 0 [x_tables] d0000000046e0000-d000000004780000 0 [ip_tables] d000000004780000-d0000000047e0000 0 [rng_core] d0000000047e0000-ffffffffffffffff 0 [pseries_rng] Maps in vmlinux with a different name in kallsyms: Maps only in kallsyms: d000000000000000-f000000000000000 1000000000010000 [kernel.kallsyms] f000000000000000-ffffffffffffffff 3000000000010000 [kernel.kallsyms] test child finished with -1 ---- end ---- vmlinux symtab matches kallsyms: FAILED! The problem is that the kretprobe_trampoline symbol looks like this: $ eu-readelf -s /boot/vmlinux G kretprobe_trampoline 2431: c000000001302368 24 NOTYPE LOCAL DEFAULT 37 kretprobe_trampoline_holder 2432: c00000000003d300 8 FUNC LOCAL DEFAULT 1 .kretprobe_trampoline_holder 97543: c00000000003d300 0 NOTYPE GLOBAL DEFAULT 1 kretprobe_trampoline Its type is NOTYPE, and its size is 0, and this is a problem because symbol-elf.c:dso__load_sym skips function symbols that are not STT_FUNC or STT_GNU_IFUNC (this is determined by elf_sym__is_function). Even if the type is changed to STT_FUNC, when dso__load_sym calls symbols__fixup_duplicate, the kretprobe_trampoline symbol is dropped in favour of .kretprobe_trampoline_holder because the latter has non-zero size (as determined by choose_best_symbol). With this patch, all vmlinux symbols match /proc/kallsyms and the testcase passes. Commit c1c355c gets rid of kretprobe_trampoline_holder altogether on x86. This commit does the same on powerpc. Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> --- Notes: With this patch, all ftrace startup tests still pass. There's one ftrace testcase failure: [4] Kprobe dynamic event with function tracer [FAIL] execute: /home/bauermann/src/linux/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_ftrace.tc + . /home/bauermann/src/linux/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_ftrace.tc ++ '[' -f kprobe_events ']' ++ grep function available_tracers blk function_graph wakeup_dl wakeup_rt wakeup function nop ++ echo nop ++ echo _do_fork /home/bauermann/src/linux/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_ftrace.tc: line 9: echo: write error: Invalid argument But even without my patch that testcase fails in the same way, so this is not a problem introduced by the patch. arch/powerpc/kernel/kprobes.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)