[PATCH] Make kretprobe_trampoline symbol look like a function.

Subsystems: linux for powerpc (32-bit and 64-bit), the rest

STALE3806d

15 messages, 3 authors, 2016-03-31 · open the first message on its own page

[PATCH] Make kretprobe_trampoline symbol look like a function.

From: Thiago Jung Bauermann <hidden>
Date: 2016-03-24 17:17:32

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 <redacted>
---
 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

Re: Make kretprobe_trampoline symbol look like a function.

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-03-25 08:24:59

On Thu, 2016-24-03 at 17:17:04 UTC, Thiago Jung Bauermann wrote:
quoted hunk
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

[PATCH] Remove kretprobe_trampoline_holder.

From: Thiago Jung Bauermann <hidden>
Date: 2016-03-28 20:07:03

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 <redacted>
---

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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Thiago Jung Bauermann <hidden>
Date: 2016-03-28 20:29:51

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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-03-28 23:45:57

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:
quoted
/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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-03-29 03:31:34

On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote:
quoted hunk
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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Thiago Jung Bauermann <hidden>
Date: 2016-03-29 18:34:37

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:
quoted
If I do s/_do_fork/._do_fork/ in kprobe_ftrace.tc then all ftrace k=
probe
quoted
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 <redacted>
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 <redacted>
---
 .../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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Thiago Jung Bauermann <hidden>
Date: 2016-03-29 23:35:45

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:
quoted
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.
quoted hunk
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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-03-30 00:09:04

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:
quoted
On Mon, 2016-03-28 at 17:29 -0300, Thiago Jung Bauermann wrote:
quoted
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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Naveen N. Rao <hidden>
Date: 2016-03-30 08:07:14

On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote:
Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman:
quoted
On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote:
quoted
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.
quoted
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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Naveen N. Rao <hidden>
Date: 2016-03-30 08:49:38

On 2016/03/30 01:34PM, Naveen N Rao wrote:
On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote:
quoted
Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman:
quoted
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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2016-03-30 09:09:36

On Wed, 2016-03-30 at 13:34 +0530, Naveen N. Rao wrote:
On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote:
quoted
Am Dienstag, 29 März 2016, 14:31:34 schrieb Michael Ellerman:
quoted
On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote:
quoted
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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Thiago Jung Bauermann <hidden>
Date: 2016-03-30 18:39:31

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:
quoted
On 2016/03/29 08:35PM, Thiago Jung Bauermann wrote:
quoted
Am Dienstag, 29 M=E4rz 2016, 14:31:34 schrieb Michael Ellerman:
quoted
On Mon, 2016-03-28 at 17:06 -0300, Thiago Jung Bauermann wrote:=
quoted
quoted
quoted
quoted
With this patch, all vmlinux symbols match /proc/kallsyms and=
 the
quoted
quoted
quoted
quoted
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

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Naveen N. Rao <hidden>
Date: 2016-03-31 08:24:38

On 2016/03/28 05:06PM, Thiago Jung Bauermann wrote:
quoted hunk
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 <redacted>


- Naveen

Re: [PATCH] Remove kretprobe_trampoline_holder.

From: Thiago Jung Bauermann <hidden>
Date: 2016-03-31 20:16:40

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 <redacted>
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help