subject:"\[PATCH v4 2\/3\] perf\: kretprobes\: offset from reloc

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Naveen N. Rao

Masami,
Your patch works, thanks! However, I felt we could refactor and reuse
some of the code across kprobes.c for this purpose. Can you please see
if the below patch is fine?

Thanks,
Naveen

--
trace/kprobes: fix check for kretprobe offset within function entry

perf specifies an offset from _text and since this offset is fed
directly into the arch-specific helper, kprobes tracer rejects
installation of kretprobes through perf. Fix this by looking up the
actual offset from a function for the specified sym+offset.

Refactor and reuse existing routines to limit code duplication -- we
repurpose kprobe_addr() for determining final kprobe address and we
split out the function entry offset determination into a separate
generic helper.

Before patch:

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7ff]
  Probe point found: do_open+0
  Matched function: do_open [35d76dc]
  found inline addr: 0xc04ba9c4
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469776
  Failed to write event: Invalid argument
Error: Failed to add events. Reason: Invalid argument (Code: -22)
  naveen@ubuntu:~/linux/tools/perf$ dmesg | tail
  
  [   33.568656] Given offset is not valid for return probe.

After patch:

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7d6]
  Probe point found: do_open+0
  Matched function: do_open [35d76b3]
  found inline addr: 0xc04ba9e4
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469808
  Writing event: r:probe/do_open_1 _text+4956344
  Added new events:
probe:do_open(on do_open%return)
probe:do_open_1  (on do_open%return)

  You can now use it in all perf tools, such as:

  perf record -e probe:do_open_1 -aR sleep 1

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
  c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
  c04ba0b8  r  do_open+0x8[DISABLED]
  c0443430  r  do_open+0x0[DISABLED]

Signed-off-by: Naveen N. Rao 
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 40 ++--
 kernel/trace/trace_kprobe.c |  2 +-
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 862f87adcbb4..6888c9f29cb6 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -267,6 +267,7 @@ extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
 extern bool arch_function_offset_within_entry(unsigned long offset);
+extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char 
*sym, unsigned long offset);
 
 extern bool within_kprobe_blacklist(unsigned long addr);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 524766563896..49f870ea4070 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1391,21 +1391,19 @@ bool within_kprobe_blacklist(unsigned long addr)
  * This returns encoded errors if it fails to look up symbol or invalid
  * combination of parameters.
  */
-static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
+static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
+   const char *symbol_name, unsigned int offset)
 {
-   kprobe_opcode_t *addr = p->addr;
-
-   if ((p->symbol_name && p->addr) ||
-   (!p->symbol_name && !p->addr))
+   if ((symbol_name && addr) || (!symbol_name && !addr))
goto invalid;
 
-   if (p->symbol_name) {
-   kprobe_lookup_name(p->symbol_name, addr);
+   if (symbol_name) {
+   kprobe_lookup_name(symbol_name, addr);
if (!addr)
return ERR_PTR(-ENOENT);

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Naveen N. Rao

Masami,
Your patch works, thanks! However, I felt we could refactor and reuse
some of the code across kprobes.c for this purpose. Can you please see
if the below patch is fine?

Thanks,
Naveen

--
trace/kprobes: fix check for kretprobe offset within function entry

perf specifies an offset from _text and since this offset is fed
directly into the arch-specific helper, kprobes tracer rejects
installation of kretprobes through perf. Fix this by looking up the
actual offset from a function for the specified sym+offset.

Refactor and reuse existing routines to limit code duplication -- we
repurpose kprobe_addr() for determining final kprobe address and we
split out the function entry offset determination into a separate
generic helper.

Before patch:

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7ff]
  Probe point found: do_open+0
  Matched function: do_open [35d76dc]
  found inline addr: 0xc04ba9c4
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469776
  Failed to write event: Invalid argument
Error: Failed to add events. Reason: Invalid argument (Code: -22)
  naveen@ubuntu:~/linux/tools/perf$ dmesg | tail
  
  [   33.568656] Given offset is not valid for return probe.

After patch:

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7d6]
  Probe point found: do_open+0
  Matched function: do_open [35d76b3]
  found inline addr: 0xc04ba9e4
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469808
  Writing event: r:probe/do_open_1 _text+4956344
  Added new events:
probe:do_open(on do_open%return)
probe:do_open_1  (on do_open%return)

  You can now use it in all perf tools, such as:

  perf record -e probe:do_open_1 -aR sleep 1

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
  c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
  c04ba0b8  r  do_open+0x8[DISABLED]
  c0443430  r  do_open+0x0[DISABLED]

Signed-off-by: Naveen N. Rao 
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 40 ++--
 kernel/trace/trace_kprobe.c |  2 +-
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 862f87adcbb4..6888c9f29cb6 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -267,6 +267,7 @@ extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
 extern bool arch_function_offset_within_entry(unsigned long offset);
+extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char 
*sym, unsigned long offset);
 
 extern bool within_kprobe_blacklist(unsigned long addr);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 524766563896..49f870ea4070 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1391,21 +1391,19 @@ bool within_kprobe_blacklist(unsigned long addr)
  * This returns encoded errors if it fails to look up symbol or invalid
  * combination of parameters.
  */
-static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
+static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
+   const char *symbol_name, unsigned int offset)
 {
-   kprobe_opcode_t *addr = p->addr;
-
-   if ((p->symbol_name && p->addr) ||
-   (!p->symbol_name && !p->addr))
+   if ((symbol_name && addr) || (!symbol_name && !addr))
goto invalid;
 
-   if (p->symbol_name) {
-   kprobe_lookup_name(p->symbol_name, addr);
+   if (symbol_name) {
+   kprobe_lookup_name(symbol_name, addr);
if (!addr)
return ERR_PTR(-ENOENT);
}
 
-   addr =

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Masami Hiramatsu

On Mon, 6 Mar 2017 20:34:10 +0530
"Naveen N. Rao"  wrote:

> On 2017/03/04 09:49AM, Masami Hiramatsu wrote:
> > On Thu,  2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao"  wrote:
> > 
> > > We indicate support for accepting sym+offset with kretprobes through a
> > > line in ftrace README. Parse the same to identify support and choose the
> > > appropriate format for kprobe_events.
> > 
> > Could you give us an example of this change here? :)
> > for example, comment of commit 613f050d68a8 .
> > 
> > I think the code is OK, but we need actual example of result.
> 
> Sure :)
> As an example, without this perf patch, but with the ftrace changes:
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/tracing/README 
> | grep kretprobe
>   place (kretprobe): [:][+]|
>   naveen@ubuntu:~/linux/tools/perf$ 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return 
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7d8]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76b5]
>   found inline addr: 0xc04ba984
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open do_open+0
>   Writing event: r:probe/do_open_1 do_open+0
>   Added new events:
> probe:do_open(on do_open%return)
> probe:do_open_1  (on do_open%return)
> 
>   You can now use it in all perf tools, such as:
> 
> perf record -e probe:do_open_1 -aR sleep 1
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
>   c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
>   c04433d0  r  do_open+0x0[DISABLED]
>   c04433d0  r  do_open+0x0[DISABLED]
> 
> And after this patch (and the subsequent powerpc patch):
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return 
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7d8]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76b5]
>   found inline addr: 0xc04ba984
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//README write=0
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open _text+4469712
>   Writing event: r:probe/do_open_1 _text+4956248
>   Added new events:
> probe:do_open(on do_open%return)
> probe:do_open_1  (on do_open%return)
> 
>   You can now use it in all perf tools, such as:
> 
> perf record -e probe:do_open_1 -aR sleep 1
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
>   c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
>   c04433d0  r  do_open+0x0[DISABLED]
>   c04ba058  r  do_open+0x8[DISABLED]

Ok, with this usage example.

Acked-by: Masami Hiramatsu 

Thanks,

> 
> 
> Thanks,
> - Naveen
> 


-- 
Masami Hiramatsu

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Masami Hiramatsu

On Mon, 6 Mar 2017 20:34:10 +0530
"Naveen N. Rao"  wrote:

> On 2017/03/04 09:49AM, Masami Hiramatsu wrote:
> > On Thu,  2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao"  wrote:
> > 
> > > We indicate support for accepting sym+offset with kretprobes through a
> > > line in ftrace README. Parse the same to identify support and choose the
> > > appropriate format for kprobe_events.
> > 
> > Could you give us an example of this change here? :)
> > for example, comment of commit 613f050d68a8 .
> > 
> > I think the code is OK, but we need actual example of result.
> 
> Sure :)
> As an example, without this perf patch, but with the ftrace changes:
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/tracing/README 
> | grep kretprobe
>   place (kretprobe): [:][+]|
>   naveen@ubuntu:~/linux/tools/perf$ 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return 
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7d8]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76b5]
>   found inline addr: 0xc04ba984
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open do_open+0
>   Writing event: r:probe/do_open_1 do_open+0
>   Added new events:
> probe:do_open(on do_open%return)
> probe:do_open_1  (on do_open%return)
> 
>   You can now use it in all perf tools, such as:
> 
> perf record -e probe:do_open_1 -aR sleep 1
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
>   c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
>   c04433d0  r  do_open+0x0[DISABLED]
>   c04433d0  r  do_open+0x0[DISABLED]
> 
> And after this patch (and the subsequent powerpc patch):
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return 
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7d8]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76b5]
>   found inline addr: 0xc04ba984
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//README write=0
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open _text+4469712
>   Writing event: r:probe/do_open_1 _text+4956248
>   Added new events:
> probe:do_open(on do_open%return)
> probe:do_open_1  (on do_open%return)
> 
>   You can now use it in all perf tools, such as:
> 
> perf record -e probe:do_open_1 -aR sleep 1
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
>   c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
>   c04433d0  r  do_open+0x0[DISABLED]
>   c04ba058  r  do_open+0x8[DISABLED]

Ok, with this usage example.

Acked-by: Masami Hiramatsu 

Thanks,

> 
> 
> Thanks,
> - Naveen
> 


-- 
Masami Hiramatsu

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Masami Hiramatsu

On Mon,  6 Mar 2017 23:19:09 +0530
"Naveen N. Rao"  wrote:

> Masami,
> Your patch works, thanks! However, I felt we could refactor and reuse
> some of the code across kprobes.c for this purpose. Can you please see
> if the below patch is fine?

OK, looks good to me:)

Acked-by: Masami Hiramatsu 

Thanks!

> 
> Thanks,
> Naveen
> 
> --
> trace/kprobes: fix check for kretprobe offset within function entry
> 
> perf specifies an offset from _text and since this offset is fed
> directly into the arch-specific helper, kprobes tracer rejects
> installation of kretprobes through perf. Fix this by looking up the
> actual offset from a function for the specified sym+offset.
> 
> Refactor and reuse existing routines to limit code duplication -- we
> repurpose kprobe_addr() for determining final kprobe address and we
> split out the function entry offset determination into a separate
> generic helper.
> 
> Before patch:
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7ff]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76dc]
>   found inline addr: 0xc04ba9c4
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//README write=0
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open _text+4469776
>   Failed to write event: Invalid argument
> Error: Failed to add events. Reason: Invalid argument (Code: -22)
>   naveen@ubuntu:~/linux/tools/perf$ dmesg | tail
>   
>   [   33.568656] Given offset is not valid for return probe.
> 
> After patch:
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7d6]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76b3]
>   found inline addr: 0xc04ba9e4
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//README write=0
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open _text+4469808
>   Writing event: r:probe/do_open_1 _text+4956344
>   Added new events:
> probe:do_open(on do_open%return)
> probe:do_open_1  (on do_open%return)
> 
>   You can now use it in all perf tools, such as:
> 
> perf record -e probe:do_open_1 -aR sleep 1
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
>   c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
>   c04ba0b8  r  do_open+0x8[DISABLED]
>   c0443430  r  do_open+0x0[DISABLED]
> 
> Signed-off-by: Naveen N. Rao 
> ---
>  include/linux/kprobes.h |  1 +
>  kernel/kprobes.c| 40 ++--
>  kernel/trace/trace_kprobe.c |  2 +-
>  3 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 862f87adcbb4..6888c9f29cb6 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -267,6 +267,7 @@ extern void show_registers(struct pt_regs *regs);
>  extern void kprobes_inc_nmissed_count(struct kprobe *p);
>  extern bool arch_within_kprobe_blacklist(unsigned long addr);
>  extern bool arch_function_offset_within_entry(unsigned long offset);
> +extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char 
> *sym, unsigned long offset);
>  
>  extern bool within_kprobe_blacklist(unsigned long addr);
>  
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 524766563896..49f870ea4070 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1391,21 +1391,19 @@ bool within_kprobe_blacklist(unsigned long addr)
>   * This returns encoded errors if it fails to look up symbol or invalid
>   * combination of parameters.
>   */
> -static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
> +static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
> + const char *symbol_name, unsigned int offset)
>  {
> - kprobe_opcode_t *addr = p->addr;
> -
> - if

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Masami Hiramatsu

On Mon,  6 Mar 2017 23:19:09 +0530
"Naveen N. Rao"  wrote:

> Masami,
> Your patch works, thanks! However, I felt we could refactor and reuse
> some of the code across kprobes.c for this purpose. Can you please see
> if the below patch is fine?

OK, looks good to me:)

Acked-by: Masami Hiramatsu 

Thanks!

> 
> Thanks,
> Naveen
> 
> --
> trace/kprobes: fix check for kretprobe offset within function entry
> 
> perf specifies an offset from _text and since this offset is fed
> directly into the arch-specific helper, kprobes tracer rejects
> installation of kretprobes through perf. Fix this by looking up the
> actual offset from a function for the specified sym+offset.
> 
> Refactor and reuse existing routines to limit code duplication -- we
> repurpose kprobe_addr() for determining final kprobe address and we
> split out the function entry offset determination into a separate
> generic helper.
> 
> Before patch:
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7ff]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76dc]
>   found inline addr: 0xc04ba9c4
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//README write=0
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open _text+4469776
>   Failed to write event: Invalid argument
> Error: Failed to add events. Reason: Invalid argument (Code: -22)
>   naveen@ubuntu:~/linux/tools/perf$ dmesg | tail
>   
>   [   33.568656] Given offset is not valid for return probe.
> 
> After patch:
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
>   probe-definition(0): do_open%return
>   symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /boot/vmlinux for symbols
>   Open Debuginfo file: /boot/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: do_open [2d0c7d6]
>   Probe point found: do_open+0
>   Matched function: do_open [35d76b3]
>   found inline addr: 0xc04ba9e4
>   Failed to find "do_open%return",
>because do_open is an inlined function and has no return point.
>   An error occurred in debuginfo analysis (-22).
>   Trying to use symbols.
>   Opening /sys/kernel/debug/tracing//README write=0
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Writing event: r:probe/do_open _text+4469808
>   Writing event: r:probe/do_open_1 _text+4956344
>   Added new events:
> probe:do_open(on do_open%return)
> probe:do_open_1  (on do_open%return)
> 
>   You can now use it in all perf tools, such as:
> 
> perf record -e probe:do_open_1 -aR sleep 1
> 
>   naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
>   c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
>   c04ba0b8  r  do_open+0x8[DISABLED]
>   c0443430  r  do_open+0x0[DISABLED]
> 
> Signed-off-by: Naveen N. Rao 
> ---
>  include/linux/kprobes.h |  1 +
>  kernel/kprobes.c| 40 ++--
>  kernel/trace/trace_kprobe.c |  2 +-
>  3 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index 862f87adcbb4..6888c9f29cb6 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -267,6 +267,7 @@ extern void show_registers(struct pt_regs *regs);
>  extern void kprobes_inc_nmissed_count(struct kprobe *p);
>  extern bool arch_within_kprobe_blacklist(unsigned long addr);
>  extern bool arch_function_offset_within_entry(unsigned long offset);
> +extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char 
> *sym, unsigned long offset);
>  
>  extern bool within_kprobe_blacklist(unsigned long addr);
>  
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 524766563896..49f870ea4070 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1391,21 +1391,19 @@ bool within_kprobe_blacklist(unsigned long addr)
>   * This returns encoded errors if it fails to look up symbol or invalid
>   * combination of parameters.
>   */
> -static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
> +static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
> + const char *symbol_name, unsigned int offset)
>  {
> - kprobe_opcode_t *addr = p->addr;
> -
> - if ((p->symbol_name && p->addr) ||
> - (!p->symbol_name && !p->addr))
> + if

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Naveen N. Rao

On 2017/03/04 01:34PM, Masami Hiramatsu wrote:
> On Sat, 4 Mar 2017 11:35:51 +0900
> Masami Hiramatsu  wrote:
> 
> > On Sat, 4 Mar 2017 09:49:11 +0900
> > Masami Hiramatsu  wrote:
> > 
> > > On Thu,  2 Mar 2017 23:25:06 +0530
> > > "Naveen N. Rao"  wrote:
> > > 
> > > > We indicate support for accepting sym+offset with kretprobes through a
> > > > line in ftrace README. Parse the same to identify support and choose the
> > > > appropriate format for kprobe_events.
> > > 
> > > Could you give us an example of this change here? :)
> > > for example, comment of commit 613f050d68a8 .
> > > 
> > > I think the code is OK, but we need actual example of result.
> > 
> > Hi Naveen,
> > 
> > I've tried following commands
> > 
> > $ grep "[Tt] user_read$" /proc/kallsyms  
> >  T user_read
> >  t user_read
> > $ sudo ./perf probe -D user_read%return
> > r:probe/user_read _text+3539616
> > r:probe/user_read_1 _text+3653408
> > 
> > OK, looks good. However, when I set the retprobes, I got an error.
> > 
> > $ sudo ./perf probe -a user_read%return
> > Failed to write event: Invalid argument
> >   Error: Failed to add events.
> > 
> > And kernel rejected that.
> > 
> > $ dmesg -k | tail -n 1
> > [  850.315068] Given offset is not valid for return probe.
> > 
> > Hmm, curious..
> 
> Ah, I see.
> 
> static int create_trace_kprobe(int argc, char **argv)
> ...
> } else {
> /* a symbol specified */
> symbol = argv[1];
> /* TODO: support .init module functions */
> ret = traceprobe_split_symbol_offset(symbol, );
> if (ret) {
> pr_info("Failed to parse symbol.\n");
> return ret;
> }
> if (offset && is_return &&
> !arch_function_offset_within_entry(offset)) {
> pr_info("Given offset is not valid for return 
> probe.\n");
> return -EINVAL;
> }
> }
> 
> So, actually, traceprobe_split_symbol_offset() just split out symbol
> and offset from symbol string (e.g. "_text+3539616").
> So, you should use kallsyms_lookup_size_offset() here again to check
> offset.

Ah, nice catch! I should have tested Steven's patch...

> 
> Please try attached patch (I've already tested on x86-64).
> 
> $ sudo ./perf probe -a user_read%return
> Added new events:
>   probe:user_read  (on user_read%return)
>   probe:user_read_1(on user_read%return)
> 
> You can now use it in all perf tools, such as:
> 
>   perf record -e probe:user_read_1 -aR sleep 1
> 
> $ sudo ./perf probe -l
>   probe:user_read  (on user_read%return@security/keys/user_defined.c)
>   probe:user_read_1(on user_read%return@selinux/ss/policydb.c)
> $ sudo cat /sys/kernel/debug/kprobes/list
> 9637bf70  r  user_read+0x0[DISABLED][FTRACE]
> 963602f0  r  user_read+0x0[DISABLED][FTRACE]

Thanks, I will test this.


- Naveen

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Naveen N. Rao

On 2017/03/04 01:34PM, Masami Hiramatsu wrote:
> On Sat, 4 Mar 2017 11:35:51 +0900
> Masami Hiramatsu  wrote:
> 
> > On Sat, 4 Mar 2017 09:49:11 +0900
> > Masami Hiramatsu  wrote:
> > 
> > > On Thu,  2 Mar 2017 23:25:06 +0530
> > > "Naveen N. Rao"  wrote:
> > > 
> > > > We indicate support for accepting sym+offset with kretprobes through a
> > > > line in ftrace README. Parse the same to identify support and choose the
> > > > appropriate format for kprobe_events.
> > > 
> > > Could you give us an example of this change here? :)
> > > for example, comment of commit 613f050d68a8 .
> > > 
> > > I think the code is OK, but we need actual example of result.
> > 
> > Hi Naveen,
> > 
> > I've tried following commands
> > 
> > $ grep "[Tt] user_read$" /proc/kallsyms  
> >  T user_read
> >  t user_read
> > $ sudo ./perf probe -D user_read%return
> > r:probe/user_read _text+3539616
> > r:probe/user_read_1 _text+3653408
> > 
> > OK, looks good. However, when I set the retprobes, I got an error.
> > 
> > $ sudo ./perf probe -a user_read%return
> > Failed to write event: Invalid argument
> >   Error: Failed to add events.
> > 
> > And kernel rejected that.
> > 
> > $ dmesg -k | tail -n 1
> > [  850.315068] Given offset is not valid for return probe.
> > 
> > Hmm, curious..
> 
> Ah, I see.
> 
> static int create_trace_kprobe(int argc, char **argv)
> ...
> } else {
> /* a symbol specified */
> symbol = argv[1];
> /* TODO: support .init module functions */
> ret = traceprobe_split_symbol_offset(symbol, );
> if (ret) {
> pr_info("Failed to parse symbol.\n");
> return ret;
> }
> if (offset && is_return &&
> !arch_function_offset_within_entry(offset)) {
> pr_info("Given offset is not valid for return 
> probe.\n");
> return -EINVAL;
> }
> }
> 
> So, actually, traceprobe_split_symbol_offset() just split out symbol
> and offset from symbol string (e.g. "_text+3539616").
> So, you should use kallsyms_lookup_size_offset() here again to check
> offset.

Ah, nice catch! I should have tested Steven's patch...

> 
> Please try attached patch (I've already tested on x86-64).
> 
> $ sudo ./perf probe -a user_read%return
> Added new events:
>   probe:user_read  (on user_read%return)
>   probe:user_read_1(on user_read%return)
> 
> You can now use it in all perf tools, such as:
> 
>   perf record -e probe:user_read_1 -aR sleep 1
> 
> $ sudo ./perf probe -l
>   probe:user_read  (on user_read%return@security/keys/user_defined.c)
>   probe:user_read_1(on user_read%return@selinux/ss/policydb.c)
> $ sudo cat /sys/kernel/debug/kprobes/list
> 9637bf70  r  user_read+0x0[DISABLED][FTRACE]
> 963602f0  r  user_read+0x0[DISABLED][FTRACE]

Thanks, I will test this.


- Naveen

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Naveen N. Rao

On 2017/03/04 09:49AM, Masami Hiramatsu wrote:
> On Thu,  2 Mar 2017 23:25:06 +0530
> "Naveen N. Rao"  wrote:
> 
> > We indicate support for accepting sym+offset with kretprobes through a
> > line in ftrace README. Parse the same to identify support and choose the
> > appropriate format for kprobe_events.
> 
> Could you give us an example of this change here? :)
> for example, comment of commit 613f050d68a8 .
> 
> I think the code is OK, but we need actual example of result.

Sure :)
As an example, without this perf patch, but with the ftrace changes:

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/tracing/README | 
grep kretprobe
  place (kretprobe): [:][+]|
  naveen@ubuntu:~/linux/tools/perf$ 
  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return 
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7d8]
  Probe point found: do_open+0
  Matched function: do_open [35d76b5]
  found inline addr: 0xc04ba984
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open do_open+0
  Writing event: r:probe/do_open_1 do_open+0
  Added new events:
probe:do_open(on do_open%return)
probe:do_open_1  (on do_open%return)

  You can now use it in all perf tools, such as:

  perf record -e probe:do_open_1 -aR sleep 1

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
  c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
  c04433d0  r  do_open+0x0[DISABLED]
  c04433d0  r  do_open+0x0[DISABLED]

And after this patch (and the subsequent powerpc patch):

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return 
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7d8]
  Probe point found: do_open+0
  Matched function: do_open [35d76b5]
  found inline addr: 0xc04ba984
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469712
  Writing event: r:probe/do_open_1 _text+4956248
  Added new events:
probe:do_open(on do_open%return)
probe:do_open_1  (on do_open%return)

  You can now use it in all perf tools, such as:

  perf record -e probe:do_open_1 -aR sleep 1

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
  c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
  c04433d0  r  do_open+0x0[DISABLED]
  c04ba058  r  do_open+0x8[DISABLED]

Thanks,
- Naveen

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-06 Thread Naveen N. Rao

On 2017/03/04 09:49AM, Masami Hiramatsu wrote:
> On Thu,  2 Mar 2017 23:25:06 +0530
> "Naveen N. Rao"  wrote:
> 
> > We indicate support for accepting sym+offset with kretprobes through a
> > line in ftrace README. Parse the same to identify support and choose the
> > appropriate format for kprobe_events.
> 
> Could you give us an example of this change here? :)
> for example, comment of commit 613f050d68a8 .
> 
> I think the code is OK, but we need actual example of result.

Sure :)
As an example, without this perf patch, but with the ftrace changes:

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/tracing/README | 
grep kretprobe
  place (kretprobe): [:][+]|
  naveen@ubuntu:~/linux/tools/perf$ 
  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return 
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7d8]
  Probe point found: do_open+0
  Matched function: do_open [35d76b5]
  found inline addr: 0xc04ba984
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open do_open+0
  Writing event: r:probe/do_open_1 do_open+0
  Added new events:
probe:do_open(on do_open%return)
probe:do_open_1  (on do_open%return)

  You can now use it in all perf tools, such as:

  perf record -e probe:do_open_1 -aR sleep 1

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
  c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
  c04433d0  r  do_open+0x0[DISABLED]
  c04433d0  r  do_open+0x0[DISABLED]

And after this patch (and the subsequent powerpc patch):

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return 
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7d8]
  Probe point found: do_open+0
  Matched function: do_open [35d76b5]
  found inline addr: 0xc04ba984
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469712
  Writing event: r:probe/do_open_1 _text+4956248
  Added new events:
probe:do_open(on do_open%return)
probe:do_open_1  (on do_open%return)

  You can now use it in all perf tools, such as:

  perf record -e probe:do_open_1 -aR sleep 1

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
  c0041370  k  kretprobe_trampoline+0x0[OPTIMIZED]
  c04433d0  r  do_open+0x0[DISABLED]
  c04ba058  r  do_open+0x8[DISABLED]

Thanks,
- Naveen

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Sat, 4 Mar 2017 11:35:51 +0900
Masami Hiramatsu  wrote:

> On Sat, 4 Mar 2017 09:49:11 +0900
> Masami Hiramatsu  wrote:
> 
> > On Thu,  2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao"  wrote:
> > 
> > > We indicate support for accepting sym+offset with kretprobes through a
> > > line in ftrace README. Parse the same to identify support and choose the
> > > appropriate format for kprobe_events.
> > 
> > Could you give us an example of this change here? :)
> > for example, comment of commit 613f050d68a8 .
> > 
> > I think the code is OK, but we need actual example of result.
> 
> Hi Naveen,
> 
> I've tried following commands
> 
> $ grep "[Tt] user_read$" /proc/kallsyms  
>  T user_read
>  t user_read
> $ sudo ./perf probe -D user_read%return
> r:probe/user_read _text+3539616
> r:probe/user_read_1 _text+3653408
> 
> OK, looks good. However, when I set the retprobes, I got an error.
> 
> $ sudo ./perf probe -a user_read%return
> Failed to write event: Invalid argument
>   Error: Failed to add events.
> 
> And kernel rejected that.
> 
> $ dmesg -k | tail -n 1
> [  850.315068] Given offset is not valid for return probe.
> 
> Hmm, curious..

Ah, I see.

static int create_trace_kprobe(int argc, char **argv)
...
} else {
/* a symbol specified */
symbol = argv[1];
/* TODO: support .init module functions */
ret = traceprobe_split_symbol_offset(symbol, );
if (ret) {
pr_info("Failed to parse symbol.\n");
return ret;
}
if (offset && is_return &&
!arch_function_offset_within_entry(offset)) {
pr_info("Given offset is not valid for return 
probe.\n");
return -EINVAL;
}
}

So, actually, traceprobe_split_symbol_offset() just split out symbol
and offset from symbol string (e.g. "_text+3539616").
So, you should use kallsyms_lookup_size_offset() here again to check
offset.

Please try attached patch (I've already tested on x86-64).

$ sudo ./perf probe -a user_read%return
Added new events:
  probe:user_read  (on user_read%return)
  probe:user_read_1(on user_read%return)

You can now use it in all perf tools, such as:

perf record -e probe:user_read_1 -aR sleep 1

$ sudo ./perf probe -l
  probe:user_read  (on user_read%return@security/keys/user_defined.c)
  probe:user_read_1(on user_read%return@selinux/ss/policydb.c)
$ sudo cat /sys/kernel/debug/kprobes/list
9637bf70  r  user_read+0x0[DISABLED][FTRACE]
963602f0  r  user_read+0x0[DISABLED][FTRACE]

Thank you,


-- 
Masami Hiramatsu 


tracing-kprobe-check-kretprobe
Description: Binary data

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Sat, 4 Mar 2017 11:35:51 +0900
Masami Hiramatsu  wrote:

> On Sat, 4 Mar 2017 09:49:11 +0900
> Masami Hiramatsu  wrote:
> 
> > On Thu,  2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao"  wrote:
> > 
> > > We indicate support for accepting sym+offset with kretprobes through a
> > > line in ftrace README. Parse the same to identify support and choose the
> > > appropriate format for kprobe_events.
> > 
> > Could you give us an example of this change here? :)
> > for example, comment of commit 613f050d68a8 .
> > 
> > I think the code is OK, but we need actual example of result.
> 
> Hi Naveen,
> 
> I've tried following commands
> 
> $ grep "[Tt] user_read$" /proc/kallsyms  
>  T user_read
>  t user_read
> $ sudo ./perf probe -D user_read%return
> r:probe/user_read _text+3539616
> r:probe/user_read_1 _text+3653408
> 
> OK, looks good. However, when I set the retprobes, I got an error.
> 
> $ sudo ./perf probe -a user_read%return
> Failed to write event: Invalid argument
>   Error: Failed to add events.
> 
> And kernel rejected that.
> 
> $ dmesg -k | tail -n 1
> [  850.315068] Given offset is not valid for return probe.
> 
> Hmm, curious..

Ah, I see.

static int create_trace_kprobe(int argc, char **argv)
...
} else {
/* a symbol specified */
symbol = argv[1];
/* TODO: support .init module functions */
ret = traceprobe_split_symbol_offset(symbol, );
if (ret) {
pr_info("Failed to parse symbol.\n");
return ret;
}
if (offset && is_return &&
!arch_function_offset_within_entry(offset)) {
pr_info("Given offset is not valid for return 
probe.\n");
return -EINVAL;
}
}

So, actually, traceprobe_split_symbol_offset() just split out symbol
and offset from symbol string (e.g. "_text+3539616").
So, you should use kallsyms_lookup_size_offset() here again to check
offset.

Please try attached patch (I've already tested on x86-64).

$ sudo ./perf probe -a user_read%return
Added new events:
  probe:user_read  (on user_read%return)
  probe:user_read_1(on user_read%return)

You can now use it in all perf tools, such as:

perf record -e probe:user_read_1 -aR sleep 1

$ sudo ./perf probe -l
  probe:user_read  (on user_read%return@security/keys/user_defined.c)
  probe:user_read_1(on user_read%return@selinux/ss/policydb.c)
$ sudo cat /sys/kernel/debug/kprobes/list
9637bf70  r  user_read+0x0[DISABLED][FTRACE]
963602f0  r  user_read+0x0[DISABLED][FTRACE]

Thank you,


-- 
Masami Hiramatsu 


tracing-kprobe-check-kretprobe
Description: Binary data

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Sat, 4 Mar 2017 09:49:11 +0900
Masami Hiramatsu  wrote:

> On Thu,  2 Mar 2017 23:25:06 +0530
> "Naveen N. Rao"  wrote:
> 
> > We indicate support for accepting sym+offset with kretprobes through a
> > line in ftrace README. Parse the same to identify support and choose the
> > appropriate format for kprobe_events.
> 
> Could you give us an example of this change here? :)
> for example, comment of commit 613f050d68a8 .
> 
> I think the code is OK, but we need actual example of result.

Hi Naveen,

I've tried following commands

$ grep "[Tt] user_read$" /proc/kallsyms  
 T user_read
 t user_read
$ sudo ./perf probe -D user_read%return
r:probe/user_read _text+3539616
r:probe/user_read_1 _text+3653408

OK, looks good. However, when I set the retprobes, I got an error.

$ sudo ./perf probe -a user_read%return
Failed to write event: Invalid argument
  Error: Failed to add events.

And kernel rejected that.

$ dmesg -k | tail -n 1
[  850.315068] Given offset is not valid for return probe.

Hmm, curious..

I tried normal probes

$ sudo ./perf probe -D user_read
p:probe/user_read _text+3539616
p:probe/user_read_1 _text+3653408
$ sudo ./perf probe -a user_read
Added new events:
  probe:user_read  (on user_read)
  probe:user_read_1(on user_read)

You can now use it in all perf tools, such as:

perf record -e probe:user_read_1 -aR sleep 1

It works!

$ sudo ./perf probe -l
  probe:user_read  (on user_read@security/keys/user_defined.c)
  probe:user_read_1(on user_read@selinux/ss/policydb.c)
$ sudo cat /sys/kernel/debug/kprobes/list
9237bf20  k  user_read+0x0[DISABLED][FTRACE]
923602a0  k  user_read+0x0[DISABLED][FTRACE]

So, the both "_text+3539616" and "_text+3653408" are correctly located
on the entry address of user_read functions. It seems kernel-side
symbol+offset check is wrong.

Thank you,


> 
> Thanks,
> 
> > 
> > Signed-off-by: Naveen N. Rao 
> > ---
> >  tools/perf/util/probe-event.c | 12 +---
> >  tools/perf/util/probe-file.c  |  7 +++
> >  tools/perf/util/probe-file.h  |  1 +
> >  3 files changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 35f5b7b7715c..faf5789902f5 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -757,7 +757,9 @@ post_process_kernel_probe_trace_events(struct 
> > probe_trace_event *tevs,
> > }
> >  
> > for (i = 0; i < ntevs; i++) {
> > -   if (!tevs[i].point.address || tevs[i].point.retprobe)
> > +   if (!tevs[i].point.address)
> > +   continue;
> > +   if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
> > continue;
> > /* If we found a wrong one, mark it by NULL symbol */
> > if (kprobe_warn_out_range(tevs[i].point.symbol,
> > @@ -1528,11 +1530,6 @@ static int parse_perf_probe_point(char *arg, struct 
> > perf_probe_event *pev)
> > return -EINVAL;
> > }
> >  
> > -   if (pp->retprobe && !pp->function) {
> > -   semantic_error("Return probe requires an entry function.\n");
> > -   return -EINVAL;
> > -   }
> > -
> > if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
> > semantic_error("Offset/Line/Lazy pattern can't be used with "
> >"return probe.\n");
> > @@ -2841,7 +2838,8 @@ static int find_probe_trace_events_from_map(struct 
> > perf_probe_event *pev,
> > }
> >  
> > /* Note that the symbols in the kmodule are not relocated */
> > -   if (!pev->uprobes && !pp->retprobe && !pev->target) {
> > +   if (!pev->uprobes && !pev->target &&
> > +   (!pp->retprobe || kretprobe_offset_is_supported())) {
> > reloc_sym = kernel_get_ref_reloc_sym();
> > if (!reloc_sym) {
> > pr_warning("Relocated base symbol is not found!\n");
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index 8a219cd831b7..1542cd0d6799 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -879,6 +879,7 @@ int probe_cache__show_all_caches(struct strfilter 
> > *filter)
> >  
> >  enum ftrace_readme {
> > FTRACE_README_PROBE_TYPE_X = 0,
> > +   FTRACE_README_KRETPROBE_OFFSET,
> > FTRACE_README_END,
> >  };
> >  
> > @@ -889,6 +890,7 @@ static struct {
> >  #define DEFINE_TYPE(idx, pat)  \
> > [idx] = {.pattern = pat, .avail = false}
> > DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
> > +   DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
> >  };
> >  
> >  static bool scan_ftrace_readme(enum ftrace_readme type)
> > @@ -939,3 +941,8 @@ bool probe_type_is_available(enum probe_type type)
> >  
> > return true;
> >

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Sat, 4 Mar 2017 09:49:11 +0900
Masami Hiramatsu  wrote:

> On Thu,  2 Mar 2017 23:25:06 +0530
> "Naveen N. Rao"  wrote:
> 
> > We indicate support for accepting sym+offset with kretprobes through a
> > line in ftrace README. Parse the same to identify support and choose the
> > appropriate format for kprobe_events.
> 
> Could you give us an example of this change here? :)
> for example, comment of commit 613f050d68a8 .
> 
> I think the code is OK, but we need actual example of result.

Hi Naveen,

I've tried following commands

$ grep "[Tt] user_read$" /proc/kallsyms  
 T user_read
 t user_read
$ sudo ./perf probe -D user_read%return
r:probe/user_read _text+3539616
r:probe/user_read_1 _text+3653408

OK, looks good. However, when I set the retprobes, I got an error.

$ sudo ./perf probe -a user_read%return
Failed to write event: Invalid argument
  Error: Failed to add events.

And kernel rejected that.

$ dmesg -k | tail -n 1
[  850.315068] Given offset is not valid for return probe.

Hmm, curious..

I tried normal probes

$ sudo ./perf probe -D user_read
p:probe/user_read _text+3539616
p:probe/user_read_1 _text+3653408
$ sudo ./perf probe -a user_read
Added new events:
  probe:user_read  (on user_read)
  probe:user_read_1(on user_read)

You can now use it in all perf tools, such as:

perf record -e probe:user_read_1 -aR sleep 1

It works!

$ sudo ./perf probe -l
  probe:user_read  (on user_read@security/keys/user_defined.c)
  probe:user_read_1(on user_read@selinux/ss/policydb.c)
$ sudo cat /sys/kernel/debug/kprobes/list
9237bf20  k  user_read+0x0[DISABLED][FTRACE]
923602a0  k  user_read+0x0[DISABLED][FTRACE]

So, the both "_text+3539616" and "_text+3653408" are correctly located
on the entry address of user_read functions. It seems kernel-side
symbol+offset check is wrong.

Thank you,


> 
> Thanks,
> 
> > 
> > Signed-off-by: Naveen N. Rao 
> > ---
> >  tools/perf/util/probe-event.c | 12 +---
> >  tools/perf/util/probe-file.c  |  7 +++
> >  tools/perf/util/probe-file.h  |  1 +
> >  3 files changed, 13 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 35f5b7b7715c..faf5789902f5 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -757,7 +757,9 @@ post_process_kernel_probe_trace_events(struct 
> > probe_trace_event *tevs,
> > }
> >  
> > for (i = 0; i < ntevs; i++) {
> > -   if (!tevs[i].point.address || tevs[i].point.retprobe)
> > +   if (!tevs[i].point.address)
> > +   continue;
> > +   if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
> > continue;
> > /* If we found a wrong one, mark it by NULL symbol */
> > if (kprobe_warn_out_range(tevs[i].point.symbol,
> > @@ -1528,11 +1530,6 @@ static int parse_perf_probe_point(char *arg, struct 
> > perf_probe_event *pev)
> > return -EINVAL;
> > }
> >  
> > -   if (pp->retprobe && !pp->function) {
> > -   semantic_error("Return probe requires an entry function.\n");
> > -   return -EINVAL;
> > -   }
> > -
> > if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
> > semantic_error("Offset/Line/Lazy pattern can't be used with "
> >"return probe.\n");
> > @@ -2841,7 +2838,8 @@ static int find_probe_trace_events_from_map(struct 
> > perf_probe_event *pev,
> > }
> >  
> > /* Note that the symbols in the kmodule are not relocated */
> > -   if (!pev->uprobes && !pp->retprobe && !pev->target) {
> > +   if (!pev->uprobes && !pev->target &&
> > +   (!pp->retprobe || kretprobe_offset_is_supported())) {
> > reloc_sym = kernel_get_ref_reloc_sym();
> > if (!reloc_sym) {
> > pr_warning("Relocated base symbol is not found!\n");
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index 8a219cd831b7..1542cd0d6799 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -879,6 +879,7 @@ int probe_cache__show_all_caches(struct strfilter 
> > *filter)
> >  
> >  enum ftrace_readme {
> > FTRACE_README_PROBE_TYPE_X = 0,
> > +   FTRACE_README_KRETPROBE_OFFSET,
> > FTRACE_README_END,
> >  };
> >  
> > @@ -889,6 +890,7 @@ static struct {
> >  #define DEFINE_TYPE(idx, pat)  \
> > [idx] = {.pattern = pat, .avail = false}
> > DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
> > +   DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
> >  };
> >  
> >  static bool scan_ftrace_readme(enum ftrace_readme type)
> > @@ -939,3 +941,8 @@ bool probe_type_is_available(enum probe_type type)
> >  
> > return true;
> >  }
> > +
> > +bool kretprobe_offset_is_supported(void)
> > +{
> > +   return

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Sat, 4 Mar 2017 11:35:51 +0900
Masami Hiramatsu  wrote:

> On Sat, 4 Mar 2017 09:49:11 +0900
> Masami Hiramatsu  wrote:
> 
> > On Thu,  2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao"  wrote:
> > 
> > > We indicate support for accepting sym+offset with kretprobes through a
> > > line in ftrace README. Parse the same to identify support and choose the
> > > appropriate format for kprobe_events.
> > 
> > Could you give us an example of this change here? :)
> > for example, comment of commit 613f050d68a8 .
> > 
> > I think the code is OK, but we need actual example of result.
> 
> Hi Naveen,
> 
> I've tried following commands
> 
> $ grep "[Tt] user_read$" /proc/kallsyms  
>  T user_read
>  t user_read
> $ sudo ./perf probe -D user_read%return
> r:probe/user_read _text+3539616
> r:probe/user_read_1 _text+3653408
> 
> OK, looks good. However, when I set the retprobes, I got an error.
> 
> $ sudo ./perf probe -a user_read%return
> Failed to write event: Invalid argument
>   Error: Failed to add events.
> 
> And kernel rejected that.
> 
> $ dmesg -k | tail -n 1
> [  850.315068] Given offset is not valid for return probe.
> 
> Hmm, curious..
> 
> I tried normal probes
> 
> $ sudo ./perf probe -D user_read
> p:probe/user_read _text+3539616
> p:probe/user_read_1 _text+3653408
> $ sudo ./perf probe -a user_read
> Added new events:
>   probe:user_read  (on user_read)
>   probe:user_read_1(on user_read)
> 
> You can now use it in all perf tools, such as:
> 
>   perf record -e probe:user_read_1 -aR sleep 1
> 
> It works!
> 
> $ sudo ./perf probe -l
>   probe:user_read  (on user_read@security/keys/user_defined.c)
>   probe:user_read_1(on user_read@selinux/ss/policydb.c)
> $ sudo cat /sys/kernel/debug/kprobes/list
> 9237bf20  k  user_read+0x0[DISABLED][FTRACE]
> 923602a0  k  user_read+0x0[DISABLED][FTRACE]
> 
> So, the both "_text+3539616" and "_text+3653408" are correctly located
> on the entry address of user_read functions. It seems kernel-side
> symbol+offset check is wrong.

FYI, without this patch, perf probe returns same place for same-name
functions. So this patch itself looks good.

$ sudo ./perf probe -D user_read%return
r:probe/user_read user_read+0
r:probe/user_read_1 user_read+0


Thanks,


-- 
Masami Hiramatsu

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Sat, 4 Mar 2017 11:35:51 +0900
Masami Hiramatsu  wrote:

> On Sat, 4 Mar 2017 09:49:11 +0900
> Masami Hiramatsu  wrote:
> 
> > On Thu,  2 Mar 2017 23:25:06 +0530
> > "Naveen N. Rao"  wrote:
> > 
> > > We indicate support for accepting sym+offset with kretprobes through a
> > > line in ftrace README. Parse the same to identify support and choose the
> > > appropriate format for kprobe_events.
> > 
> > Could you give us an example of this change here? :)
> > for example, comment of commit 613f050d68a8 .
> > 
> > I think the code is OK, but we need actual example of result.
> 
> Hi Naveen,
> 
> I've tried following commands
> 
> $ grep "[Tt] user_read$" /proc/kallsyms  
>  T user_read
>  t user_read
> $ sudo ./perf probe -D user_read%return
> r:probe/user_read _text+3539616
> r:probe/user_read_1 _text+3653408
> 
> OK, looks good. However, when I set the retprobes, I got an error.
> 
> $ sudo ./perf probe -a user_read%return
> Failed to write event: Invalid argument
>   Error: Failed to add events.
> 
> And kernel rejected that.
> 
> $ dmesg -k | tail -n 1
> [  850.315068] Given offset is not valid for return probe.
> 
> Hmm, curious..
> 
> I tried normal probes
> 
> $ sudo ./perf probe -D user_read
> p:probe/user_read _text+3539616
> p:probe/user_read_1 _text+3653408
> $ sudo ./perf probe -a user_read
> Added new events:
>   probe:user_read  (on user_read)
>   probe:user_read_1(on user_read)
> 
> You can now use it in all perf tools, such as:
> 
>   perf record -e probe:user_read_1 -aR sleep 1
> 
> It works!
> 
> $ sudo ./perf probe -l
>   probe:user_read  (on user_read@security/keys/user_defined.c)
>   probe:user_read_1(on user_read@selinux/ss/policydb.c)
> $ sudo cat /sys/kernel/debug/kprobes/list
> 9237bf20  k  user_read+0x0[DISABLED][FTRACE]
> 923602a0  k  user_read+0x0[DISABLED][FTRACE]
> 
> So, the both "_text+3539616" and "_text+3653408" are correctly located
> on the entry address of user_read functions. It seems kernel-side
> symbol+offset check is wrong.

FYI, without this patch, perf probe returns same place for same-name
functions. So this patch itself looks good.

$ sudo ./perf probe -D user_read%return
r:probe/user_read user_read+0
r:probe/user_read_1 user_read+0


Thanks,


-- 
Masami Hiramatsu

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Thu,  2 Mar 2017 23:25:06 +0530
"Naveen N. Rao"  wrote:

> We indicate support for accepting sym+offset with kretprobes through a
> line in ftrace README. Parse the same to identify support and choose the
> appropriate format for kprobe_events.

Could you give us an example of this change here? :)
for example, comment of commit 613f050d68a8 .

I think the code is OK, but we need actual example of result.

Thanks,

> 
> Signed-off-by: Naveen N. Rao 
> ---
>  tools/perf/util/probe-event.c | 12 +---
>  tools/perf/util/probe-file.c  |  7 +++
>  tools/perf/util/probe-file.h  |  1 +
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 35f5b7b7715c..faf5789902f5 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -757,7 +757,9 @@ post_process_kernel_probe_trace_events(struct 
> probe_trace_event *tevs,
>   }
>  
>   for (i = 0; i < ntevs; i++) {
> - if (!tevs[i].point.address || tevs[i].point.retprobe)
> + if (!tevs[i].point.address)
> + continue;
> + if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
>   continue;
>   /* If we found a wrong one, mark it by NULL symbol */
>   if (kprobe_warn_out_range(tevs[i].point.symbol,
> @@ -1528,11 +1530,6 @@ static int parse_perf_probe_point(char *arg, struct 
> perf_probe_event *pev)
>   return -EINVAL;
>   }
>  
> - if (pp->retprobe && !pp->function) {
> - semantic_error("Return probe requires an entry function.\n");
> - return -EINVAL;
> - }
> -
>   if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
>   semantic_error("Offset/Line/Lazy pattern can't be used with "
>  "return probe.\n");
> @@ -2841,7 +2838,8 @@ static int find_probe_trace_events_from_map(struct 
> perf_probe_event *pev,
>   }
>  
>   /* Note that the symbols in the kmodule are not relocated */
> - if (!pev->uprobes && !pp->retprobe && !pev->target) {
> + if (!pev->uprobes && !pev->target &&
> + (!pp->retprobe || kretprobe_offset_is_supported())) {
>   reloc_sym = kernel_get_ref_reloc_sym();
>   if (!reloc_sym) {
>   pr_warning("Relocated base symbol is not found!\n");
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 8a219cd831b7..1542cd0d6799 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -879,6 +879,7 @@ int probe_cache__show_all_caches(struct strfilter *filter)
>  
>  enum ftrace_readme {
>   FTRACE_README_PROBE_TYPE_X = 0,
> + FTRACE_README_KRETPROBE_OFFSET,
>   FTRACE_README_END,
>  };
>  
> @@ -889,6 +890,7 @@ static struct {
>  #define DEFINE_TYPE(idx, pat)\
>   [idx] = {.pattern = pat, .avail = false}
>   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
> + DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
>  };
>  
>  static bool scan_ftrace_readme(enum ftrace_readme type)
> @@ -939,3 +941,8 @@ bool probe_type_is_available(enum probe_type type)
>  
>   return true;
>  }
> +
> +bool kretprobe_offset_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
> +}
> diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
> index a17a82eff8a0..dbf95a00864a 100644
> --- a/tools/perf/util/probe-file.h
> +++ b/tools/perf/util/probe-file.h
> @@ -65,6 +65,7 @@ struct probe_cache_entry *probe_cache__find_by_name(struct 
> probe_cache *pcache,
>   const char *group, const char *event);
>  int probe_cache__show_all_caches(struct strfilter *filter);
>  bool probe_type_is_available(enum probe_type type);
> +bool kretprobe_offset_is_supported(void);
>  #else/* ! HAVE_LIBELF_SUPPORT */
>  static inline struct probe_cache *probe_cache__new(const char *tgt 
> __maybe_unused)
>  {
> -- 
> 2.11.1
> 


-- 
Masami Hiramatsu

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-03 Thread Masami Hiramatsu

On Thu,  2 Mar 2017 23:25:06 +0530
"Naveen N. Rao"  wrote:

> We indicate support for accepting sym+offset with kretprobes through a
> line in ftrace README. Parse the same to identify support and choose the
> appropriate format for kprobe_events.

Could you give us an example of this change here? :)
for example, comment of commit 613f050d68a8 .

I think the code is OK, but we need actual example of result.

Thanks,

> 
> Signed-off-by: Naveen N. Rao 
> ---
>  tools/perf/util/probe-event.c | 12 +---
>  tools/perf/util/probe-file.c  |  7 +++
>  tools/perf/util/probe-file.h  |  1 +
>  3 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 35f5b7b7715c..faf5789902f5 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -757,7 +757,9 @@ post_process_kernel_probe_trace_events(struct 
> probe_trace_event *tevs,
>   }
>  
>   for (i = 0; i < ntevs; i++) {
> - if (!tevs[i].point.address || tevs[i].point.retprobe)
> + if (!tevs[i].point.address)
> + continue;
> + if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
>   continue;
>   /* If we found a wrong one, mark it by NULL symbol */
>   if (kprobe_warn_out_range(tevs[i].point.symbol,
> @@ -1528,11 +1530,6 @@ static int parse_perf_probe_point(char *arg, struct 
> perf_probe_event *pev)
>   return -EINVAL;
>   }
>  
> - if (pp->retprobe && !pp->function) {
> - semantic_error("Return probe requires an entry function.\n");
> - return -EINVAL;
> - }
> -
>   if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
>   semantic_error("Offset/Line/Lazy pattern can't be used with "
>  "return probe.\n");
> @@ -2841,7 +2838,8 @@ static int find_probe_trace_events_from_map(struct 
> perf_probe_event *pev,
>   }
>  
>   /* Note that the symbols in the kmodule are not relocated */
> - if (!pev->uprobes && !pp->retprobe && !pev->target) {
> + if (!pev->uprobes && !pev->target &&
> + (!pp->retprobe || kretprobe_offset_is_supported())) {
>   reloc_sym = kernel_get_ref_reloc_sym();
>   if (!reloc_sym) {
>   pr_warning("Relocated base symbol is not found!\n");
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 8a219cd831b7..1542cd0d6799 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -879,6 +879,7 @@ int probe_cache__show_all_caches(struct strfilter *filter)
>  
>  enum ftrace_readme {
>   FTRACE_README_PROBE_TYPE_X = 0,
> + FTRACE_README_KRETPROBE_OFFSET,
>   FTRACE_README_END,
>  };
>  
> @@ -889,6 +890,7 @@ static struct {
>  #define DEFINE_TYPE(idx, pat)\
>   [idx] = {.pattern = pat, .avail = false}
>   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
> + DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
>  };
>  
>  static bool scan_ftrace_readme(enum ftrace_readme type)
> @@ -939,3 +941,8 @@ bool probe_type_is_available(enum probe_type type)
>  
>   return true;
>  }
> +
> +bool kretprobe_offset_is_supported(void)
> +{
> + return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
> +}
> diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
> index a17a82eff8a0..dbf95a00864a 100644
> --- a/tools/perf/util/probe-file.h
> +++ b/tools/perf/util/probe-file.h
> @@ -65,6 +65,7 @@ struct probe_cache_entry *probe_cache__find_by_name(struct 
> probe_cache *pcache,
>   const char *group, const char *event);
>  int probe_cache__show_all_caches(struct strfilter *filter);
>  bool probe_type_is_available(enum probe_type type);
> +bool kretprobe_offset_is_supported(void);
>  #else/* ! HAVE_LIBELF_SUPPORT */
>  static inline struct probe_cache *probe_cache__new(const char *tgt 
> __maybe_unused)
>  {
> -- 
> 2.11.1
> 


-- 
Masami Hiramatsu

[PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-02 Thread Naveen N. Rao

We indicate support for accepting sym+offset with kretprobes through a
line in ftrace README. Parse the same to identify support and choose the
appropriate format for kprobe_events.

Signed-off-by: Naveen N. Rao 
---
 tools/perf/util/probe-event.c | 12 +---
 tools/perf/util/probe-file.c  |  7 +++
 tools/perf/util/probe-file.h  |  1 +
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 35f5b7b7715c..faf5789902f5 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -757,7 +757,9 @@ post_process_kernel_probe_trace_events(struct 
probe_trace_event *tevs,
}
 
for (i = 0; i < ntevs; i++) {
-   if (!tevs[i].point.address || tevs[i].point.retprobe)
+   if (!tevs[i].point.address)
+   continue;
+   if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
continue;
/* If we found a wrong one, mark it by NULL symbol */
if (kprobe_warn_out_range(tevs[i].point.symbol,
@@ -1528,11 +1530,6 @@ static int parse_perf_probe_point(char *arg, struct 
perf_probe_event *pev)
return -EINVAL;
}
 
-   if (pp->retprobe && !pp->function) {
-   semantic_error("Return probe requires an entry function.\n");
-   return -EINVAL;
-   }
-
if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
semantic_error("Offset/Line/Lazy pattern can't be used with "
   "return probe.\n");
@@ -2841,7 +2838,8 @@ static int find_probe_trace_events_from_map(struct 
perf_probe_event *pev,
}
 
/* Note that the symbols in the kmodule are not relocated */
-   if (!pev->uprobes && !pp->retprobe && !pev->target) {
+   if (!pev->uprobes && !pev->target &&
+   (!pp->retprobe || kretprobe_offset_is_supported())) {
reloc_sym = kernel_get_ref_reloc_sym();
if (!reloc_sym) {
pr_warning("Relocated base symbol is not found!\n");
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 8a219cd831b7..1542cd0d6799 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -879,6 +879,7 @@ int probe_cache__show_all_caches(struct strfilter *filter)
 
 enum ftrace_readme {
FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_KRETPROBE_OFFSET,
FTRACE_README_END,
 };
 
@@ -889,6 +890,7 @@ static struct {
 #define DEFINE_TYPE(idx, pat)  \
[idx] = {.pattern = pat, .avail = false}
DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
+   DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
 };
 
 static bool scan_ftrace_readme(enum ftrace_readme type)
@@ -939,3 +941,8 @@ bool probe_type_is_available(enum probe_type type)
 
return true;
 }
+
+bool kretprobe_offset_is_supported(void)
+{
+   return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index a17a82eff8a0..dbf95a00864a 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -65,6 +65,7 @@ struct probe_cache_entry *probe_cache__find_by_name(struct 
probe_cache *pcache,
const char *group, const char *event);
 int probe_cache__show_all_caches(struct strfilter *filter);
 bool probe_type_is_available(enum probe_type type);
+bool kretprobe_offset_is_supported(void);
 #else  /* ! HAVE_LIBELF_SUPPORT */
 static inline struct probe_cache *probe_cache__new(const char *tgt 
__maybe_unused)
 {
-- 
2.11.1

[PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-02 Thread Naveen N. Rao

We indicate support for accepting sym+offset with kretprobes through a
line in ftrace README. Parse the same to identify support and choose the
appropriate format for kprobe_events.

Signed-off-by: Naveen N. Rao 
---
 tools/perf/util/probe-event.c | 12 +---
 tools/perf/util/probe-file.c  |  7 +++
 tools/perf/util/probe-file.h  |  1 +
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 35f5b7b7715c..faf5789902f5 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -757,7 +757,9 @@ post_process_kernel_probe_trace_events(struct 
probe_trace_event *tevs,
}
 
for (i = 0; i < ntevs; i++) {
-   if (!tevs[i].point.address || tevs[i].point.retprobe)
+   if (!tevs[i].point.address)
+   continue;
+   if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
continue;
/* If we found a wrong one, mark it by NULL symbol */
if (kprobe_warn_out_range(tevs[i].point.symbol,
@@ -1528,11 +1530,6 @@ static int parse_perf_probe_point(char *arg, struct 
perf_probe_event *pev)
return -EINVAL;
}
 
-   if (pp->retprobe && !pp->function) {
-   semantic_error("Return probe requires an entry function.\n");
-   return -EINVAL;
-   }
-
if ((pp->offset || pp->line || pp->lazy_line) && pp->retprobe) {
semantic_error("Offset/Line/Lazy pattern can't be used with "
   "return probe.\n");
@@ -2841,7 +2838,8 @@ static int find_probe_trace_events_from_map(struct 
perf_probe_event *pev,
}
 
/* Note that the symbols in the kmodule are not relocated */
-   if (!pev->uprobes && !pp->retprobe && !pev->target) {
+   if (!pev->uprobes && !pev->target &&
+   (!pp->retprobe || kretprobe_offset_is_supported())) {
reloc_sym = kernel_get_ref_reloc_sym();
if (!reloc_sym) {
pr_warning("Relocated base symbol is not found!\n");
diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 8a219cd831b7..1542cd0d6799 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -879,6 +879,7 @@ int probe_cache__show_all_caches(struct strfilter *filter)
 
 enum ftrace_readme {
FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_KRETPROBE_OFFSET,
FTRACE_README_END,
 };
 
@@ -889,6 +890,7 @@ static struct {
 #define DEFINE_TYPE(idx, pat)  \
[idx] = {.pattern = pat, .avail = false}
DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
+   DEFINE_TYPE(FTRACE_README_KRETPROBE_OFFSET, "*place (kretprobe): *"),
 };
 
 static bool scan_ftrace_readme(enum ftrace_readme type)
@@ -939,3 +941,8 @@ bool probe_type_is_available(enum probe_type type)
 
return true;
 }
+
+bool kretprobe_offset_is_supported(void)
+{
+   return scan_ftrace_readme(FTRACE_README_KRETPROBE_OFFSET);
+}
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index a17a82eff8a0..dbf95a00864a 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -65,6 +65,7 @@ struct probe_cache_entry *probe_cache__find_by_name(struct 
probe_cache *pcache,
const char *group, const char *event);
 int probe_cache__show_all_caches(struct strfilter *filter);
 bool probe_type_is_available(enum probe_type type);
+bool kretprobe_offset_is_supported(void);
 #else  /* ! HAVE_LIBELF_SUPPORT */
 static inline struct probe_cache *probe_cache__new(const char *tgt 
__maybe_unused)
 {
-- 
2.11.1

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

Re: [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

[PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

[PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it

20 matches

Site Navigation

Mail list logo

Footer information