* [PATCH] Make kretprobe_trampoline symbol look like a function.
@ 2016-03-24 17:17 Thiago Jung Bauermann
2016-03-25 8:24 ` Michael Ellerman
0 siblings, 1 reply; 15+ messages in thread
From: Thiago Jung Bauermann @ 2016-03-24 17:17 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ananth, Thiago Jung Bauermann
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, during symbol deduplication kretprobe_trampoline is
used instead of .kretprobe_trampoline_holder, making all vmlinux symbols match
/proc/kallsyms and the testcase passes.
Alternatively, I just noticed that commit c1c355c was merged and it
gets rid of kretprobe_trampoline_holder altogether on x86. I believe that
would fix this problem as well, but I don't know enough about kprobes to
know whether kretprobe_trampoline_holder has any use on powerpc.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
arch/powerpc/kernel/kprobes.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 7c053f2..068f2f9 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -281,8 +281,10 @@ no_kprobe:
static void __used kretprobe_trampoline_holder(void)
{
asm volatile(".global kretprobe_trampoline\n"
+ ".type kretprobe_trampoline, @function\n"
"kretprobe_trampoline:\n"
- "nop\n");
+ "nop\n"
+ ".size kretprobe_trampoline, .-kretprobe_trampoline\n");
}
/*
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: Make kretprobe_trampoline symbol look like a function.
2016-03-24 17:17 [PATCH] Make kretprobe_trampoline symbol look like a function Thiago Jung Bauermann
@ 2016-03-25 8:24 ` Michael Ellerman
2016-03-28 20:06 ` [PATCH] Remove kretprobe_trampoline_holder Thiago Jung Bauermann
0 siblings, 1 reply; 15+ messages in thread
From: Michael Ellerman @ 2016-03-25 8:24 UTC (permalink / raw)
To: Thiago Jung Bauermann, linuxppc-dev; +Cc: Thiago Jung Bauermann
On Thu, 2016-24-03 at 17:17:04 UTC, 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, during symbol deduplication kretprobe_trampoline is
> used instead of .kretprobe_trampoline_holder, making all vmlinux symbols match
> /proc/kallsyms and the testcase passes.
Thanks, good change log.
> Alternatively, I just noticed that commit c1c355c was merged and it
> gets rid of kretprobe_trampoline_holder altogether on x86. I believe that
> would fix this problem as well, but I don't know enough about kprobes to
> know whether kretprobe_trampoline_holder has any use on powerpc.
I don't see any reason why we need it on powerpc.
So can you try removing kretprobe_trampoline_holder and check that kretprobes
still work. And then confirm that it also fixes this bug?
cheers
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] Remove kretprobe_trampoline_holder.
2016-03-25 8:24 ` Michael Ellerman
@ 2016-03-28 20:06 ` Thiago Jung Bauermann
2016-03-28 20:29 ` Thiago Jung Bauermann
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Thiago Jung Bauermann @ 2016-03-28 20:06 UTC (permalink / raw)
To: linuxppc-dev; +Cc: ananth, mpe, Thiago Jung Bauermann
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(-)
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
--
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-28 20:06 ` [PATCH] Remove kretprobe_trampoline_holder Thiago Jung Bauermann
@ 2016-03-28 20:29 ` Thiago Jung Bauermann
2016-03-28 23:45 ` Michael Ellerman
2016-03-29 3:31 ` Michael Ellerman
2016-03-31 8:23 ` Naveen N. Rao
2 siblings, 1 reply; 15+ messages in thread
From: Thiago Jung Bauermann @ 2016-03-28 20:29 UTC (permalink / raw)
To: linuxppc-dev
Am Montag, 28 M=E4rz 2016, 17:06:32 schrieb Thiago Jung Bauermann:
> /home/bauermann/src/linux/tools/testing/selftests/ftrace/test.d/kprob=
e/kp
> robe_ftrace.tc: line 9: echo: write error: Invalid argument
>=20
> 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 t=
o its=20
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 kprob=
e=20
tests pass:
$ sudo ./ftracetest test.d/kprobe/
=3D=3D=3D Ftrace unit tests =3D=3D=3D
[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
--=20
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-28 20:29 ` Thiago Jung Bauermann
@ 2016-03-28 23:45 ` Michael Ellerman
2016-03-29 18:34 ` Thiago Jung Bauermann
0 siblings, 1 reply; 15+ messages in thread
From: Michael Ellerman @ 2016-03-28 23:45 UTC (permalink / raw)
To: Thiago Jung Bauermann, linuxppc-dev
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-28 20:06 ` [PATCH] Remove kretprobe_trampoline_holder Thiago Jung Bauermann
2016-03-28 20:29 ` Thiago Jung Bauermann
@ 2016-03-29 3:31 ` Michael Ellerman
2016-03-29 23:35 ` Thiago Jung Bauermann
2016-03-31 8:23 ` Naveen N. Rao
2 siblings, 1 reply; 15+ messages in thread
From: Michael Ellerman @ 2016-03-29 3:31 UTC (permalink / raw)
To: Thiago Jung Bauermann, linuxppc-dev
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-28 23:45 ` Michael Ellerman
@ 2016-03-29 18:34 ` Thiago Jung Bauermann
2016-03-30 0:09 ` Michael Ellerman
0 siblings, 1 reply; 15+ messages in thread
From: Thiago Jung Bauermann @ 2016-03-29 18:34 UTC (permalink / raw)
To: linuxppc-dev
Am Dienstag, 29 M=C3=A4rz 2016, 10:45:57 schrieb Michael Ellerman:
> On Mon, 2016-03-28 at 17:29 -0300, Thiago Jung Bauermann wrote:
> > If I do s/_do_fork/._do_fork/ in kprobe_ftrace.tc then all ftrace k=
probe
> > tests pass:
>=20
> OK. We fixed that in 'perf probe', but not if you're using the sysfs =
file
> directly.
>=20
> Do you want to write a patch for ftracetest to try and handle it? I g=
uess
> you'd try "_do_fork" and if that fails then try "._do_fork", and mayb=
e
> only if uname -m says you're running on ppc64?
I did write a patch yesterday (included below for reference), but then =
I
noticed that the other ftrace tests use _do_fork and they work fine (I =
guess
because of the fix you mentioned). I think that ideally the ftrace filt=
er
mechanism should work with dot symbols as well as regular symbols.
I think this could work by creating a mechanism analogous to the
ARCH_HAS_SYSCALL_MATCH_SYM_NAME one in trace_syscalls.c. Ftrace_match_r=
ecord
could call it to adjust the symbol name (like kprobe_lookup_name) befor=
e
calling ftrace_match.
But I=E2=80=99m wondering if it=E2=80=99s really worth the effort and m=
aybe patching the
testcase is enough? Also, I don=E2=80=99t know whether my idea would ha=
ve any
side effects.
--=20
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
>From 2ea9b3545906932b9aa213506ec0dff03cffdbe5 Mon Sep 17 00:00:00 2001
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Date: Mon, 28 Mar 2016 18:39:56 -0300
Subject: [PATCH] selftests: kprobe: Fix kprobe_ftrace.tc on ppc64
On ppc64 (but not ppc64le), symbols for function entry points start wit=
h
a dot. There's no _do_fork but there is a ._do_fork.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
.../selftests/ftrace/test.d/kprobe/kprobe_ftrace.tc | 20 ++++++++++++=
+-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_ftrace=
.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_ftrace.tc
index d6f2f49..c3ec5c2 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_ftrace.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_ftrace.tc
@@ -4,33 +4,39 @@
[ -f kprobe_events ] || exit_unsupported # this is configurable
grep function available_tracers || exit_unsupported # this is configur=
able
=20
+if [ "$(uname -m)" =3D "ppc64" ]; then
+ FILTER_FUNCTION=3D"._do_fork"
+else
+ FILTER_FUNCTION=3D"_do_fork"
+fi
+
# prepare
echo nop > current_tracer
-echo _do_fork > set_ftrace_filter
+echo $FILTER_FUNCTION > set_ftrace_filter
echo 0 > events/enable
echo > kprobe_events
-echo 'p:testprobe _do_fork' > kprobe_events
+echo "p:testprobe $FILTER_FUNCTION" > kprobe_events
=20
# kprobe on / ftrace off
echo 1 > events/kprobes/testprobe/enable
echo > trace
( echo "forked")
grep testprobe trace
-! grep '_do_fork <-' trace
+! grep "$FILTER_FUNCTION <-" trace
=20
# kprobe on / ftrace on
echo function > current_tracer
echo > trace
( echo "forked")
grep testprobe trace
-grep '_do_fork <-' trace
+grep "$FILTER_FUNCTION <-" trace
=20
# kprobe off / ftrace on
echo 0 > events/kprobes/testprobe/enable
echo > trace
( echo "forked")
! grep testprobe trace
-grep '_do_fork <-' trace
+grep "$FILTER_FUNCTION <-" trace
=20
# kprobe on / ftrace on
echo 1 > events/kprobes/testprobe/enable
@@ -38,14 +44,14 @@ echo function > current_tracer
echo > trace
( echo "forked")
grep testprobe trace
-grep '_do_fork <-' trace
+grep "$FILTER_FUNCTION <-" trace
=20
# kprobe on / ftrace off
echo nop > current_tracer
echo > trace
( echo "forked")
grep testprobe trace
-! grep '_do_fork <-' trace
+! grep "$FILTER_FUNCTION <-" trace
=20
# cleanup
echo nop > current_tracer
--=20
1.9.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-29 3:31 ` Michael Ellerman
@ 2016-03-29 23:35 ` Thiago Jung Bauermann
2016-03-30 8:04 ` Naveen N. Rao
0 siblings, 1 reply; 15+ messages in thread
From: Thiago Jung Bauermann @ 2016-03-29 23:35 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, ananth
Am Dienstag, 29 M=C3=A4rz 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.
>=20
> 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:
>=20
> 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
>=20
> $ sudo grep emulate_step /proc/kallsyms
> c000000000984820 T emulate_step
>=20
>=20
> 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 po=
int=20
because symbol-elf.c:dso__load_sym calls arch__elf_sym_adjust for each=20=
function symbol, which on ppc64le adjusts the address to the local entr=
y=20
point. On the other hand, when symbols are loaded from /proc/kallsyms t=
hey=20
are used as-is (i.e., with the global entry point address).
This seems inconsistent: dso__load_kernel_sym can end up loading symbol=
s=20
from either vmlinux or /proc/kallsyms, so it seems symbols should look =
the=20
same wherever they are loaded from. I am still trying to understand wha=
t the=20
rest of the perf code expects.
If I comment out the call to arch__elf_sym_adjust in dso__load_sym, the=
n all=20
symbols are found in the vmlinux-kallsyms.c test (the test still fails=20=
because two symbols have different names. I haven=E2=80=99t checked why=
). Also, the=20
code-reading.c test is able to read object code for a lot (but not all)=
of=20
the sample events, whereas in the adjusted symbols case it can=E2=80=99=
t read object=20
code for any sample event. The other perf tests are unaffected by the=20=
change.
I thought that if there=E2=80=99s no vmlinux available, then perf would=
have to rely=20
only on /proc/kallsyms and I would see some difference in the test resu=
lts=20
compared to when perf uses vmlinux and adjusts the symbols to the local=
=20
entry point, but I saw no difference at all.
So at first glance, it looks like perf is better off using symbols that=
=20
point to the global entry point...
--=20
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-29 18:34 ` Thiago Jung Bauermann
@ 2016-03-30 0:09 ` Michael Ellerman
0 siblings, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2016-03-30 0:09 UTC (permalink / raw)
To: Thiago Jung Bauermann, linuxppc-dev
On Tue, 2016-03-29 at 15:34 -0300, Thiago Jung Bauermann wrote:
> Am Dienstag, 29 März 2016, 10:45:57 schrieb Michael Ellerman:
> > On Mon, 2016-03-28 at 17:29 -0300, Thiago Jung Bauermann wrote:
> > > If I do s/_do_fork/._do_fork/ in kprobe_ftrace.tc then all ftrace kprobe
> > > tests 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?
>
> I did write a patch yesterday (included below for reference), but then I
> noticed that the other ftrace tests use _do_fork and they work fine (I guess
> because of the fix you mentioned). I think that ideally the ftrace filter
> mechanism should work with dot symbols as well as regular symbols.
>
> I think this could work by creating a mechanism analogous to the
> ARCH_HAS_SYSCALL_MATCH_SYM_NAME one in trace_syscalls.c. Ftrace_match_record
> could call it to adjust the symbol name (like kprobe_lookup_name) before
> calling ftrace_match.
>
> But I’m wondering if it’s really worth the effort and maybe patching the
> testcase is enough? Also, I don’t know whether my idea would have any
> side effects.
It'd be nice if it worked properly. Reusing kprobe_lookup_name() looks like it
would be the right fix, given this is "kprobe events".
cheers
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-29 23:35 ` Thiago Jung Bauermann
@ 2016-03-30 8:04 ` Naveen N. Rao
2016-03-30 8:46 ` Naveen N. Rao
2016-03-30 9:09 ` Michael Ellerman
0 siblings, 2 replies; 15+ messages in thread
From: Naveen N. Rao @ 2016-03-30 8:04 UTC (permalink / raw)
To: Thiago Jung Bauermann; +Cc: Michael Ellerman, linuxppc-dev
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-30 8:04 ` Naveen N. Rao
@ 2016-03-30 8:46 ` Naveen N. Rao
2016-03-30 9:09 ` Michael Ellerman
1 sibling, 0 replies; 15+ messages in thread
From: Naveen N. Rao @ 2016-03-30 8:46 UTC (permalink / raw)
To: Thiago Jung Bauermann; +Cc: linuxppc-dev
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-30 8:04 ` Naveen N. Rao
2016-03-30 8:46 ` Naveen N. Rao
@ 2016-03-30 9:09 ` Michael Ellerman
2016-03-30 18:38 ` Thiago Jung Bauermann
1 sibling, 1 reply; 15+ messages in thread
From: Michael Ellerman @ 2016-03-30 9:09 UTC (permalink / raw)
To: Naveen N. Rao, Thiago Jung Bauermann; +Cc: linuxppc-dev
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-30 9:09 ` Michael Ellerman
@ 2016-03-30 18:38 ` Thiago Jung Bauermann
0 siblings, 0 replies; 15+ messages in thread
From: Thiago Jung Bauermann @ 2016-03-30 18:38 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Naveen N. Rao, linuxppc-dev
Am Mittwoch, 30 M=E4rz 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=E4rz 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.
> > > >=20
> > > > Have you tested this on an LE system?
> > >=20
> > > No, I was focusing on ppc64 BE.
> >=20
> > Which kernel did you use? I don't see this with the latest 4.6.0-rc=
1.
>=20
> 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 .kr=
etprobe_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 =
=20
2479: c000000000ef1d68 24 NOTYPE LOCAL DEFAULT 37 kretprob=
e_trampoline_holder
2480: c000000000043ab0 40 FUNC LOCAL DEFAULT 1 .kretpro=
be_trampoline_holder
86903: c000000000043ac4 0 NOTYPE GLOBAL DEFAULT 1 kretprob=
e_trampoline
$ grep kretprobe_tramp /proc/kallsyms
c000000000043ab0 t .kretprobe_trampoline_holder
c000000000043ac4 T kretprobe_trampoline
c000000000ef1d68 d kretprobe_trampoline_holder
--=20
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-28 20:06 ` [PATCH] Remove kretprobe_trampoline_holder Thiago Jung Bauermann
2016-03-28 20:29 ` Thiago Jung Bauermann
2016-03-29 3:31 ` Michael Ellerman
@ 2016-03-31 8:23 ` Naveen N. Rao
2016-03-31 20:16 ` Thiago Jung Bauermann
2 siblings, 1 reply; 15+ messages in thread
From: Naveen N. Rao @ 2016-03-31 8:23 UTC (permalink / raw)
To: Thiago Jung Bauermann; +Cc: linuxppc-dev
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] Remove kretprobe_trampoline_holder.
2016-03-31 8:23 ` Naveen N. Rao
@ 2016-03-31 20:16 ` Thiago Jung Bauermann
0 siblings, 0 replies; 15+ messages in thread
From: Thiago Jung Bauermann @ 2016-03-31 20:16 UTC (permalink / raw)
To: Naveen N. Rao; +Cc: linuxppc-dev
Am Donnerstag, 31 M=C3=A4rz 2016, 13:53:11 schrieb Naveen N. Rao:
> You should indent the above output -- in this specific case, the star=
t
> marker interferes with git am.
>=20
> 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=20
a new patch with your Reviewed-by.
Also thanks for fixing the vmlinux-kallsyms issue in ppc64le. I didn=E2=
=80=99t=20
comment on the patches because I=E2=80=99m not very familiar with the c=
ode in=20
question.
--=20
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-03-31 20:16 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-24 17:17 [PATCH] Make kretprobe_trampoline symbol look like a function Thiago Jung Bauermann
2016-03-25 8:24 ` Michael Ellerman
2016-03-28 20:06 ` [PATCH] Remove kretprobe_trampoline_holder Thiago Jung Bauermann
2016-03-28 20:29 ` Thiago Jung Bauermann
2016-03-28 23:45 ` Michael Ellerman
2016-03-29 18:34 ` Thiago Jung Bauermann
2016-03-30 0:09 ` Michael Ellerman
2016-03-29 3:31 ` Michael Ellerman
2016-03-29 23:35 ` Thiago Jung Bauermann
2016-03-30 8:04 ` Naveen N. Rao
2016-03-30 8:46 ` Naveen N. Rao
2016-03-30 9:09 ` Michael Ellerman
2016-03-30 18:38 ` Thiago Jung Bauermann
2016-03-31 8:23 ` Naveen N. Rao
2016-03-31 20:16 ` Thiago Jung Bauermann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.