Re: [PATCH 0/2] perf probe fixes for ppc64le
On 2016/04/11 02:41PM, Michael Ellerman wrote: > On Sat, 2016-04-09 at 19:12 +0530, Naveen N. Rao wrote: > > > > I suppose this boils down to the quirkiness of ABIv2. Though, in > > reality, I don't think most users will notice. As I stated above, users > > will most likely start with the disassembly or debuginfo and this patch > > ensures there are actually no surprises there. > > Yeah it's unfortunate that we have to handle these two cases differently. > > But I think you've chosen the right trade off. > > When we are just given the name we *must not* use the global entry point, > otherwise the probes will often not hit - because most calls go to the local > entry point and skip the global entry point entirely. > > When we're given a name and offset, it's less confusing if we use the global > entry point as the base for the offset calculation. > > So for the concept: > > Acked-by: Michael EllermanThanks, Michael. That helps. > > I don't really know this part of the perf code enough to give you an ack for > the > actual changes, I'll leave that to the perf maintainers. Sure. Arnaldo, I will send a v2 soon with a bit more testing to make sure this covers all scenarios properly (I am also trying to see if we can address debuginfo-based probing properly). - Naveen ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] perf probe fixes for ppc64le
On Sat, 2016-04-09 at 19:12 +0530, Naveen N. Rao wrote: > > I suppose this boils down to the quirkiness of ABIv2. Though, in > reality, I don't think most users will notice. As I stated above, users > will most likely start with the disassembly or debuginfo and this patch > ensures there are actually no surprises there. Yeah it's unfortunate that we have to handle these two cases differently. But I think you've chosen the right trade off. When we are just given the name we *must not* use the global entry point, otherwise the probes will often not hit - because most calls go to the local entry point and skip the global entry point entirely. When we're given a name and offset, it's less confusing if we use the global entry point as the base for the offset calculation. So for the concept: Acked-by: Michael EllermanI don't really know this part of the perf code enough to give you an ack for the actual changes, I'll leave that to the perf maintainers. cheers ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/2] perf probe fixes for ppc64le
On 2016/04/08 04:57PM, Balbir Singh wrote: > On Thu, 2016-04-07 at 14:56 +0530, Naveen N. Rao wrote: > > On 2016/04/07 06:19PM, Balbir Singh wrote: > > > > > > > > > On 06/04/16 22:32, Naveen N. Rao wrote: > > > > > > > > This patchset fixes three issues found with perf probe on ppc64le: > > > > 1. 'perf test kallsyms' failure on ppc64le (reported by Michael > > > > Ellerman). This was due to the symbols being fixed up during symbol > > > > table load. This is fixed in patch 2 by delaying symbol fixup until > > > > later. > > > > 2. perf probe function offset was being calculated from the local entry > > > > point (LEP), which does not match user expectation when trying to look > > > > at function disassembly output (reported by Ananth N). This is fixed for > > > > kallsyms in patch 1 and for symbol table in patch 2. > > > I think the bit where the offset is w.r.t LEP when using a name, but w.r.t > > > GEP when using function+offset can be confusing. > > Thanks for your review! > > > > The rationale for this is actually from the end-user perspective. The > > two use cases we are considering are: > > 1. User just wants to probe at function entry point: > > # perf probe _do_fork > > > > In this case, the user most definitely needs the local entry point, > > without which the probe won't be hit. So, for this case, we > > automatically insert the probe at the LEP. > > > > [We really only want to alter perf probe behavior in this case only, but > > we were incorrectly changing the behavior of perf with the below > > scenario as well.] > > > > 2. User wants to probe at a specific location. In this case, the user > > most likely starts by looking at the function disassembly. For instance: > > # objdump -S -d vmlinux.bak | grep -A100 \<_do_fork\>: > > c00b6a00 <_do_fork>: > > unsigned long stack_start, > > unsigned long stack_size, > > int __user *parent_tidptr, > > int __user *child_tidptr, > > unsigned long tls) > > { > > c00b6a00: f7 00 4c 3c addis r2,r12,247 > > c00b6a04: 00 86 42 38 addir2,r2,-31232 > > c00b6a08: a6 02 08 7c mflrr0 > > c00b6a0c: d0 ff 41 fb std r26,-48(r1) > > c00b6a10: 26 80 90 7d mfocrf r12,8 > > .. > > if (!(clone_flags & CLONE_UNTRACED)) { > > c00b6a54: e3 4f c7 7b rldicl. r7,r30,41,63 > > c00b6a58: 2c 00 82 40 bne c00b6a84 > ><_do_fork+0x84> > > if (clone_flags & CLONE_VFORK) > > c00b6a5c: e3 97 c8 7b rldicl. r8,r30,50,63 > > c00b6a60: a0 01 82 41 beq c00b6c00 > ><_do_fork+0x200> > > c00b6a64: 20 00 20 39 li r9,32 > > trace = PTRACE_EVENT_VFORK; > > c00b6a68: 02 00 80 3b li r28,2 > > c00b6a6c: 10 02 4d e9 ld r10,528(r13) > > > > If the user wants to probe at _do_fork+0x54, he'd do: > > # perf probe _do_fork+0x54 > > > > With the earlier approach, we would insert the probe at _do_fork+0x5c > > (0x54 from the LEP) instead, which is incorrect. > > > > In reality, user would probably just use debuginfo: > > # perf probe -L _do_fork > > <_do_fork@/root/linus/kernel/fork.c:0> > > 0 long _do_fork(unsigned long clone_flags, > > unsigned long stack_start, > > unsigned long stack_size, > > int __user *parent_tidptr, > > int __user *child_tidptr, > > unsigned long tls) > > 6 { > > struct task_struct *p; > > 8 int trace = 0; > > long nr; > > > > /* > > * Determine whether and which event to report to > >ptracer. When > > * called from kernel_thread or CLONE_UNTRACED is > >explicitly > > * requested, no event is reported; otherwise, report > >if the event > > * for the type of forking is enabled. > > */ > > 17 if (!(clone_flags & CLONE_UNTRACED)) { > > 18 if (clone_flags & CLONE_VFORK) > > 19 trace = PTRACE_EVENT_VFORK; > > 20 else if ((clone_flags & CSIGNAL) != SIGCHLD) > > 21 trace = PTRACE_EVENT_CLONE; > > > > # perf probe _do_fork:17 > > > > In this case, perf chooses the right address based on DWARF. The current > > patchset matches the behavior of perf without debuginfo with this. > > > I agree what I worry is that perf probe _do_fork sets a breakpoint after > perf probe _do_fork+0x4. I
Re: [PATCH 0/2] perf probe fixes for ppc64le
On Thu, 2016-04-07 at 14:56 +0530, Naveen N. Rao wrote: > On 2016/04/07 06:19PM, Balbir Singh wrote: > > > > > > On 06/04/16 22:32, Naveen N. Rao wrote: > > > > > > This patchset fixes three issues found with perf probe on ppc64le: > > > 1. 'perf test kallsyms' failure on ppc64le (reported by Michael > > > Ellerman). This was due to the symbols being fixed up during symbol > > > table load. This is fixed in patch 2 by delaying symbol fixup until > > > later. > > > 2. perf probe function offset was being calculated from the local entry > > > point (LEP), which does not match user expectation when trying to look > > > at function disassembly output (reported by Ananth N). This is fixed for > > > kallsyms in patch 1 and for symbol table in patch 2. > > I think the bit where the offset is w.r.t LEP when using a name, but w.r.t > > GEP when using function+offset can be confusing. > Thanks for your review! > > The rationale for this is actually from the end-user perspective. The > two use cases we are considering are: > 1. User just wants to probe at function entry point: > # perf probe _do_fork > > In this case, the user most definitely needs the local entry point, > without which the probe won't be hit. So, for this case, we > automatically insert the probe at the LEP. > > [We really only want to alter perf probe behavior in this case only, but > we were incorrectly changing the behavior of perf with the below > scenario as well.] > > 2. User wants to probe at a specific location. In this case, the user > most likely starts by looking at the function disassembly. For instance: > # objdump -S -d vmlinux.bak | grep -A100 \<_do_fork\>: > c00b6a00 <_do_fork>: > unsigned long stack_start, > unsigned long stack_size, > int __user *parent_tidptr, > int __user *child_tidptr, > unsigned long tls) > { > c00b6a00: f7 00 4c 3c addis r2,r12,247 > c00b6a04: 00 86 42 38 addir2,r2,-31232 > c00b6a08: a6 02 08 7c mflrr0 > c00b6a0c: d0 ff 41 fb std r26,-48(r1) > c00b6a10: 26 80 90 7d mfocrf r12,8 > .. > if (!(clone_flags & CLONE_UNTRACED)) { > c00b6a54: e3 4f c7 7b rldicl. r7,r30,41,63 > c00b6a58: 2c 00 82 40 bne c00b6a84 ><_do_fork+0x84> > if (clone_flags & CLONE_VFORK) > c00b6a5c: e3 97 c8 7b rldicl. r8,r30,50,63 > c00b6a60: a0 01 82 41 beq c00b6c00 ><_do_fork+0x200> > c00b6a64: 20 00 20 39 li r9,32 > trace = PTRACE_EVENT_VFORK; > c00b6a68: 02 00 80 3b li r28,2 > c00b6a6c: 10 02 4d e9 ld r10,528(r13) > > If the user wants to probe at _do_fork+0x54, he'd do: > # perf probe _do_fork+0x54 > > With the earlier approach, we would insert the probe at _do_fork+0x5c > (0x54 from the LEP) instead, which is incorrect. > > In reality, user would probably just use debuginfo: > # perf probe -L _do_fork > <_do_fork@/root/linus/kernel/fork.c:0> > 0 long _do_fork(unsigned long clone_flags, > unsigned long stack_start, > unsigned long stack_size, > int __user *parent_tidptr, > int __user *child_tidptr, > unsigned long tls) > 6 { > struct task_struct *p; > 8 int trace = 0; > long nr; > > /* > * Determine whether and which event to report to >ptracer. When > * called from kernel_thread or CLONE_UNTRACED is >explicitly > * requested, no event is reported; otherwise, report >if the event > * for the type of forking is enabled. > */ > 17 if (!(clone_flags & CLONE_UNTRACED)) { > 18 if (clone_flags & CLONE_VFORK) > 19 trace = PTRACE_EVENT_VFORK; > 20 else if ((clone_flags & CSIGNAL) != SIGCHLD) > 21 trace = PTRACE_EVENT_CLONE; > > # perf probe _do_fork:17 > > In this case, perf chooses the right address based on DWARF. The current > patchset matches the behavior of perf without debuginfo with this. I agree what I worry is that perf probe _do_fork sets a breakpoint after perf probe _do_fork+0x4. I am not sure if there is an easy solution to the problem. Balbir ___ Linuxppc-dev mailing list
Re: [PATCH 0/2] perf probe fixes for ppc64le
On 2016/04/07 06:19PM, Balbir Singh wrote: > > On 06/04/16 22:32, Naveen N. Rao wrote: > > This patchset fixes three issues found with perf probe on ppc64le: > > 1. 'perf test kallsyms' failure on ppc64le (reported by Michael > > Ellerman). This was due to the symbols being fixed up during symbol > > table load. This is fixed in patch 2 by delaying symbol fixup until > > later. > > 2. perf probe function offset was being calculated from the local entry > > point (LEP), which does not match user expectation when trying to look > > at function disassembly output (reported by Ananth N). This is fixed for > > kallsyms in patch 1 and for symbol table in patch 2. > > I think the bit where the offset is w.r.t LEP when using a name, but w.r.t > GEP when using function+offset can be confusing. Thanks for your review! The rationale for this is actually from the end-user perspective. The two use cases we are considering are: 1. User just wants to probe at function entry point: # perf probe _do_fork In this case, the user most definitely needs the local entry point, without which the probe won't be hit. So, for this case, we automatically insert the probe at the LEP. [We really only want to alter perf probe behavior in this case only, but we were incorrectly changing the behavior of perf with the below scenario as well.] 2. User wants to probe at a specific location. In this case, the user most likely starts by looking at the function disassembly. For instance: # objdump -S -d vmlinux.bak | grep -A100 \<_do_fork\>: c00b6a00 <_do_fork>: unsigned long stack_start, unsigned long stack_size, int __user *parent_tidptr, int __user *child_tidptr, unsigned long tls) { c00b6a00: f7 00 4c 3c addis r2,r12,247 c00b6a04: 00 86 42 38 addir2,r2,-31232 c00b6a08: a6 02 08 7c mflrr0 c00b6a0c: d0 ff 41 fb std r26,-48(r1) c00b6a10: 26 80 90 7d mfocrf r12,8 .. if (!(clone_flags & CLONE_UNTRACED)) { c00b6a54: e3 4f c7 7b rldicl. r7,r30,41,63 c00b6a58: 2c 00 82 40 bne c00b6a84 <_do_fork+0x84> if (clone_flags & CLONE_VFORK) c00b6a5c: e3 97 c8 7b rldicl. r8,r30,50,63 c00b6a60: a0 01 82 41 beq c00b6c00 <_do_fork+0x200> c00b6a64: 20 00 20 39 li r9,32 trace = PTRACE_EVENT_VFORK; c00b6a68: 02 00 80 3b li r28,2 c00b6a6c: 10 02 4d e9 ld r10,528(r13) If the user wants to probe at _do_fork+0x54, he'd do: # perf probe _do_fork+0x54 With the earlier approach, we would insert the probe at _do_fork+0x5c (0x54 from the LEP) instead, which is incorrect. In reality, user would probably just use debuginfo: # perf probe -L _do_fork <_do_fork@/root/linus/kernel/fork.c:0> 0 long _do_fork(unsigned long clone_flags, unsigned long stack_start, unsigned long stack_size, int __user *parent_tidptr, int __user *child_tidptr, unsigned long tls) 6 { struct task_struct *p; 8 int trace = 0; long nr; /* * Determine whether and which event to report to ptracer. When * called from kernel_thread or CLONE_UNTRACED is explicitly * requested, no event is reported; otherwise, report if the event * for the type of forking is enabled. */ 17 if (!(clone_flags & CLONE_UNTRACED)) { 18 if (clone_flags & CLONE_VFORK) 19 trace = PTRACE_EVENT_VFORK; 20 else if ((clone_flags & CSIGNAL) != SIGCHLD) 21 trace = PTRACE_EVENT_CLONE; # perf probe _do_fork:17 In this case, perf chooses the right address based on DWARF. The current patchset matches the behavior of perf without debuginfo with this. > Do we really need probe > points between GEP and LEP? All the GEP does is setup r2. The use case > could be more generic, but please clarify. There could be scenarios where having a probe point between GEP and LEP is useful - for instance, if we are only interested in calls to an in-kernel function from an external module. However, this is a secondary consideration and the more important
Re: [PATCH 0/2] perf probe fixes for ppc64le
On 06/04/16 22:32, Naveen N. Rao wrote: > This patchset fixes three issues found with perf probe on ppc64le: > 1. 'perf test kallsyms' failure on ppc64le (reported by Michael > Ellerman). This was due to the symbols being fixed up during symbol > table load. This is fixed in patch 2 by delaying symbol fixup until > later. > 2. perf probe function offset was being calculated from the local entry > point (LEP), which does not match user expectation when trying to look > at function disassembly output (reported by Ananth N). This is fixed for > kallsyms in patch 1 and for symbol table in patch 2. I think the bit where the offset is w.r.t LEP when using a name, but w.r.t GEP when using function+offset can be confusing. Do we really need probe points between GEP and LEP? All the GEP does is setup r2. The use case could be more generic, but please clarify. > 3. perf probe failure with kretprobe when using kallsyms. This was > failing as we were specifying an offset. This is fixed in patch 1. > Balbir Singh. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/2] perf probe fixes for ppc64le
This patchset fixes three issues found with perf probe on ppc64le: 1. 'perf test kallsyms' failure on ppc64le (reported by Michael Ellerman). This was due to the symbols being fixed up during symbol table load. This is fixed in patch 2 by delaying symbol fixup until later. 2. perf probe function offset was being calculated from the local entry point (LEP), which does not match user expectation when trying to look at function disassembly output (reported by Ananth N). This is fixed for kallsyms in patch 1 and for symbol table in patch 2. 3. perf probe failure with kretprobe when using kallsyms. This was failing as we were specifying an offset. This is fixed in patch 1. A few examples demonstrating the issues and the fix: Example for issue (2): # objdump -d vmlinux | grep -A8 \<_do_fork\>: c00b6a00 <_do_fork>: c00b6a00: f7 00 4c 3c addis r2,r12,247 c00b6a04: 00 86 42 38 addir2,r2,-31232 c00b6a08: a6 02 08 7c mflrr0 c00b6a0c: d0 ff 41 fb std r26,-48(r1) c00b6a10: 26 80 90 7d mfocrf r12,8 c00b6a14: d8 ff 61 fb std r27,-40(r1) c00b6a18: e0 ff 81 fb std r28,-32(r1) c00b6a1c: e8 ff a1 fb std r29,-24(r1) # perf probe -v _do_fork+4 probe-definition(0): _do_fork+4 symbol:_do_fork file:(null) line:0 offset:4 return:0 lazy:(null) 0 arguments Looking at the vmlinux_path (8 entries long) Using /proc/kcore for kernel object code Using /proc/kallsyms for symbols Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/_do_fork _text+748044 Added new event: probe:_do_fork (on _do_fork+4) You can now use it in all perf tools, such as: perf record -e probe:_do_fork -aR sleep 1 # printf "%x\n" 748044 b6a0c ^ This is offset from the LEP. With this, there is also no way to ever probe between the GEP and the LEP. With this patchset: # perf probe -v _do_fork+4 probe-definition(0): _do_fork+4 symbol:_do_fork file:(null) line:0 offset:4 return:0 lazy:(null) 0 arguments Looking at the vmlinux_path (8 entries long) Using /proc/kcore for kernel object code Using /proc/kallsyms for symbols Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/_do_fork _text+748036 Added new event: probe:_do_fork (on _do_fork+4) You can now use it in all perf tools, such as: perf record -e probe:_do_fork -aR sleep 1 # perf probe -v _do_fork probe-definition(0): _do_fork symbol:_do_fork file:(null) line:0 offset:0 return:0 lazy:(null) 0 arguments Looking at the vmlinux_path (8 entries long) Using /proc/kcore for kernel object code Using /proc/kallsyms for symbols Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: p:probe/_do_fork _text+748040 Added new event: probe:_do_fork (on _do_fork) You can now use it in all perf tools, such as: perf record -e probe:_do_fork -aR sleep 1 We only offset to the LEP if function entry is specified, otherwise, we offset from the GEP. Example for issue (3): - Before patch: # perf probe -v _do_fork:%return probe-definition(0): _do_fork:%return symbol:_do_fork file:(null) line:0 offset:0 return:1 lazy:(null) 0 arguments Looking at the vmlinux_path (8 entries long) Using /proc/kcore for kernel object code Using /proc/kallsyms for symbols Opening /sys/kernel/debug/tracing//kprobe_events write=1 Writing event: r:probe/_do_fork _do_fork+8 Failed to write event: Invalid argument Error: Failed to add events. Reason: Invalid argument (Code: -22) After patch: # perf probe _do_fork:%return Added new event: probe:_do_fork (on _do_fork%return) You can now use it in all perf tools, such as: perf record -e probe:_do_fork -aR sleep 1 Cc: Mark WielaardCc: Thiago Jung Bauermann Cc: Arnaldo Carvalho de Melo Cc: Masami Hiramatsu Cc: Michael Ellerman Cc: Ananth N Mavinakayanahalli Naveen N. Rao (2): perf/powerpc: Fix kprobe and kretprobe handling with kallsyms tools/perf: Fix kallsyms perf test on ppc64le tools/perf/arch/powerpc/util/sym-handling.c | 41 - tools/perf/util/probe-event.c | 5 ++-- tools/perf/util/probe-event.h | 3 ++- tools/perf/util/symbol-elf.c| 7 ++--- tools/perf/util/symbol.h| 3 ++- 5 files changed, 40 insertions(+), 19 deletions(-) -- 2.7.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org