Re: [RESEND PATCH 1/6] trace/kprobes: fix check for kretprobe offset within function entry

2017-03-08 Thread Naveen N. Rao
On 2017/03/07 03:47PM, Steven Rostedt wrote:
> 
> Please start a new thread. When sending patches as replies to other
> patch threads, especially this deep into the thread, they will most
> likely get ignored.

Sorry, got carried off. I will re-post in a new series.

- Naveen



Re: [RESEND PATCH 1/6] trace/kprobes: fix check for kretprobe offset within function entry

2017-03-08 Thread Naveen N. Rao
On 2017/03/07 03:47PM, Steven Rostedt wrote:
> 
> Please start a new thread. When sending patches as replies to other
> patch threads, especially this deep into the thread, they will most
> likely get ignored.

Sorry, got carried off. I will re-post in a new series.

- Naveen



[PATCH v5 0/5] kretprobe fixes

2017-03-08 Thread Naveen N. Rao
Now that I've been carried back in (ugh!), please find the remaining
patches from the earlier series (*) here. Patches 1-4 are the same as in
v4. Patch 5 in the previous series was dropped and the previous patch 6
has been updated accordingly.

- Naveen

(*) https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1347013.html

--
Naveen N. Rao (5):
  trace/kprobes: fix check for kretprobe offset within function entry
  powerpc: kretprobes: override default function entry offset
  perf: probe: factor out the ftrace README scanning
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: powerpc: choose local entry point with kretprobes

 arch/powerpc/kernel/kprobes.c   |  9 
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 40 +--
 kernel/trace/trace_kprobe.c |  2 +-
 tools/perf/arch/powerpc/util/sym-handling.c | 14 --
 tools/perf/util/probe-event.c   | 12 ++---
 tools/perf/util/probe-file.c| 77 -
 tools/perf/util/probe-file.h|  1 +
 8 files changed, 97 insertions(+), 59 deletions(-)

-- 
2.11.1



[PATCH v5 0/5] kretprobe fixes

2017-03-08 Thread Naveen N. Rao
Now that I've been carried back in (ugh!), please find the remaining
patches from the earlier series (*) here. Patches 1-4 are the same as in
v4. Patch 5 in the previous series was dropped and the previous patch 6
has been updated accordingly.

- Naveen

(*) https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1347013.html

--
Naveen N. Rao (5):
  trace/kprobes: fix check for kretprobe offset within function entry
  powerpc: kretprobes: override default function entry offset
  perf: probe: factor out the ftrace README scanning
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: powerpc: choose local entry point with kretprobes

 arch/powerpc/kernel/kprobes.c   |  9 
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 40 +--
 kernel/trace/trace_kprobe.c |  2 +-
 tools/perf/arch/powerpc/util/sym-handling.c | 14 --
 tools/perf/util/probe-event.c   | 12 ++---
 tools/perf/util/probe-file.c| 77 -
 tools/perf/util/probe-file.h|  1 +
 8 files changed, 97 insertions(+), 59 deletions(-)

-- 
2.11.1



[PATCH v5 3/5] perf: probe: factor out the ftrace README scanning

2017-03-08 Thread Naveen N. Rao
Simplify and separate out the ftrace README scanning logic into a
separate helper. This is used subsequently to scan for all patterns of
interest and to cache the result.

Since we are only interested in availability of probe argument type x,
we will only scan for that.

Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/util/probe-file.c | 70 +++-
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1a62daceb028..8a219cd831b7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -877,35 +877,31 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
+enum ftrace_readme {
+   FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_END,
+};
+
 static struct {
const char *pattern;
-   boolavail;
-   boolchecked;
-} probe_type_table[] = {
-#define DEFINE_TYPE(idx, pat, def_avail)   \
-   [idx] = {.pattern = pat, .avail = (def_avail)}
-   DEFINE_TYPE(PROBE_TYPE_U, "* u8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_S, "* s8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_X, "* x8/16/32/64,*", false),
-   DEFINE_TYPE(PROBE_TYPE_STRING, "* string,*", true),
-   DEFINE_TYPE(PROBE_TYPE_BITFIELD,
-   "* b@/", true),
+   bool avail;
+} ftrace_readme_table[] = {
+#define DEFINE_TYPE(idx, pat)  \
+   [idx] = {.pattern = pat, .avail = false}
+   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
 };
 
-bool probe_type_is_available(enum probe_type type)
+static bool scan_ftrace_readme(enum ftrace_readme type)
 {
+   int fd;
FILE *fp;
char *buf = NULL;
size_t len = 0;
-   bool target_line = false;
-   bool ret = probe_type_table[type].avail;
-   int fd;
+   bool ret = false;
+   static bool scanned = false;
 
-   if (type >= PROBE_TYPE_END)
-   return false;
-   /* We don't have to check the type which supported by default */
-   if (ret || probe_type_table[type].checked)
-   return ret;
+   if (scanned)
+   goto result;
 
fd = open_trace_file("README", false);
if (fd < 0)
@@ -917,21 +913,29 @@ bool probe_type_is_available(enum probe_type type)
return ret;
}
 
-   while (getline(, , fp) > 0 && !ret) {
-   if (!target_line) {
-   target_line = !!strstr(buf, " type: ");
-   if (!target_line)
-   continue;
-   } else if (strstr(buf, "\t  ") != buf)
-   break;
-   ret = strglobmatch(buf, probe_type_table[type].pattern);
-   }
-   /* Cache the result */
-   probe_type_table[type].checked = true;
-   probe_type_table[type].avail = ret;
+   while (getline(, , fp) > 0)
+   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
+   if (!ftrace_readme_table[i].avail)
+   ftrace_readme_table[i].avail =
+   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
+   scanned = true;
 
fclose(fp);
free(buf);
 
-   return ret;
+result:
+   if (type >= FTRACE_README_END)
+   return false;
+
+   return ftrace_readme_table[type].avail;
+}
+
+bool probe_type_is_available(enum probe_type type)
+{
+   if (type >= PROBE_TYPE_END)
+   return false;
+   else if (type == PROBE_TYPE_X)
+   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+
+   return true;
 }
-- 
2.11.1



[PATCH v5 3/5] perf: probe: factor out the ftrace README scanning

2017-03-08 Thread Naveen N. Rao
Simplify and separate out the ftrace README scanning logic into a
separate helper. This is used subsequently to scan for all patterns of
interest and to cache the result.

Since we are only interested in availability of probe argument type x,
we will only scan for that.

Acked-by: Masami Hiramatsu 
Signed-off-by: Naveen N. Rao 
---
 tools/perf/util/probe-file.c | 70 +++-
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1a62daceb028..8a219cd831b7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -877,35 +877,31 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
+enum ftrace_readme {
+   FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_END,
+};
+
 static struct {
const char *pattern;
-   boolavail;
-   boolchecked;
-} probe_type_table[] = {
-#define DEFINE_TYPE(idx, pat, def_avail)   \
-   [idx] = {.pattern = pat, .avail = (def_avail)}
-   DEFINE_TYPE(PROBE_TYPE_U, "* u8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_S, "* s8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_X, "* x8/16/32/64,*", false),
-   DEFINE_TYPE(PROBE_TYPE_STRING, "* string,*", true),
-   DEFINE_TYPE(PROBE_TYPE_BITFIELD,
-   "* b@/", true),
+   bool avail;
+} ftrace_readme_table[] = {
+#define DEFINE_TYPE(idx, pat)  \
+   [idx] = {.pattern = pat, .avail = false}
+   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
 };
 
-bool probe_type_is_available(enum probe_type type)
+static bool scan_ftrace_readme(enum ftrace_readme type)
 {
+   int fd;
FILE *fp;
char *buf = NULL;
size_t len = 0;
-   bool target_line = false;
-   bool ret = probe_type_table[type].avail;
-   int fd;
+   bool ret = false;
+   static bool scanned = false;
 
-   if (type >= PROBE_TYPE_END)
-   return false;
-   /* We don't have to check the type which supported by default */
-   if (ret || probe_type_table[type].checked)
-   return ret;
+   if (scanned)
+   goto result;
 
fd = open_trace_file("README", false);
if (fd < 0)
@@ -917,21 +913,29 @@ bool probe_type_is_available(enum probe_type type)
return ret;
}
 
-   while (getline(, , fp) > 0 && !ret) {
-   if (!target_line) {
-   target_line = !!strstr(buf, " type: ");
-   if (!target_line)
-   continue;
-   } else if (strstr(buf, "\t  ") != buf)
-   break;
-   ret = strglobmatch(buf, probe_type_table[type].pattern);
-   }
-   /* Cache the result */
-   probe_type_table[type].checked = true;
-   probe_type_table[type].avail = ret;
+   while (getline(, , fp) > 0)
+   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
+   if (!ftrace_readme_table[i].avail)
+   ftrace_readme_table[i].avail =
+   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
+   scanned = true;
 
fclose(fp);
free(buf);
 
-   return ret;
+result:
+   if (type >= FTRACE_README_END)
+   return false;
+
+   return ftrace_readme_table[type].avail;
+}
+
+bool probe_type_is_available(enum probe_type type)
+{
+   if (type >= PROBE_TYPE_END)
+   return false;
+   else if (type == PROBE_TYPE_X)
+   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+
+   return true;
 }
-- 
2.11.1



[PATCH v5 1/5] trace/kprobes: fix check for kretprobe offset within function entry

2017-03-08 Thread Naveen N. Rao
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]

Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 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 177bdf6c6aeb..47e4da5b4fa2 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -268,6 +268,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 448759d4a263..32e6ac5131ed 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 = (kprobe_opcode_t *)(((char *)addr) + p->offset);
+   addr = (kprobe_opcode_t *)(((char *)addr) +

[PATCH v5 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-08 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.

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]

Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 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 28fb62c32678..c9bdc9ded0c3 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 && !pe

[PATCH v5 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-08 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.

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]

Acked-by: Masami Hiramatsu 
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 28fb62c32678..c9bdc9ded0c3 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

[PATCH v5 1/5] trace/kprobes: fix check for kretprobe offset within function entry

2017-03-08 Thread Naveen N. Rao
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]

Acked-by: Masami Hiramatsu 
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 177bdf6c6aeb..47e4da5b4fa2 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -268,6 +268,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 448759d4a263..32e6ac5131ed 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 = (kprobe_opcode_t *)(((char *)addr) + p->offset);
+   addr = (kprobe_opcode_t *)(((char *)addr) + offset);
if (addr)
return addr;
 

[PATCH v5 2/5] powerpc: kretprobes: override default function entry offset

2017-03-08 Thread Naveen N. Rao
With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Acked-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>
Acked-by: Michael Ellerman <m...@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kprobes.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index fce05a38851c..331751701fed 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -131,6 +131,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
kcb->kprobe_saved_msr = regs->msr;
 }
 
+bool arch_function_offset_within_entry(unsigned long offset)
+{
+#ifdef PPC64_ELF_ABI_v2
+   return offset <= 8;
+#else
+   return !offset;
+#endif
+}
+
 void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
  struct pt_regs *regs)
 {
-- 
2.11.1



[PATCH v5 2/5] powerpc: kretprobes: override default function entry offset

2017-03-08 Thread Naveen N. Rao
With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Acked-by: Ananth N Mavinakayanahalli 
Acked-by: Michael Ellerman 
Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/kprobes.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index fce05a38851c..331751701fed 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -131,6 +131,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
kcb->kprobe_saved_msr = regs->msr;
 }
 
+bool arch_function_offset_within_entry(unsigned long offset)
+{
+#ifdef PPC64_ELF_ABI_v2
+   return offset <= 8;
+#else
+   return !offset;
+#endif
+}
+
 void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
  struct pt_regs *regs)
 {
-- 
2.11.1



[PATCH v5 5/5] perf: powerpc: choose local entry point with kretprobes

2017-03-08 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..39dbe512b9fc 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "probe-file.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,13 +80,18 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
return;
 
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe) {
+#ifdef HAVE_LIBELF_SUPPORT
+   if (!kretprobe_offset_is_supported())
+#endif
+   return;
+   }
+
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
 
if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
-- 
2.11.1



[PATCH v5 5/5] perf: powerpc: choose local entry point with kretprobes

2017-03-08 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..39dbe512b9fc 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "probe-file.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,13 +80,18 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
return;
 
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe) {
+#ifdef HAVE_LIBELF_SUPPORT
+   if (!kretprobe_offset_is_supported())
+#endif
+   return;
+   }
+
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
 
if (map->dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS)
-- 
2.11.1



[PATCH v2 6/6] perf: powerpc: choose local entry point with kretprobes

2017-03-07 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
Changes:
- updated to address build issues due to dropping patch 5/6.

 tools/perf/arch/powerpc/util/sym-handling.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..b8cbefc49aca 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "probe-file.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,11 +80,16 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe
+#ifdef HAVE_LIBELF_SUPPORT
+   && !kretprobe_offset_is_supported()
+#endif
+   )
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.1



[PATCH v2 6/6] perf: powerpc: choose local entry point with kretprobes

2017-03-07 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao 
---
Changes:
- updated to address build issues due to dropping patch 5/6.

 tools/perf/arch/powerpc/util/sym-handling.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..b8cbefc49aca 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "probe-file.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,11 +80,16 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe
+#ifdef HAVE_LIBELF_SUPPORT
+   && !kretprobe_offset_is_supported()
+#endif
+   )
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.1



Re: [PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c

2017-03-07 Thread Naveen N. Rao
On 2017/03/07 04:51PM, Masami Hiramatsu wrote:
> On Tue,  7 Mar 2017 16:17:40 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> 
> > probe-file.c needs libelf, but scanning ftrace README does not require
> > that. As such, move the ftrace README scanning logic out of probe-file.c
> > and into trace-event-parse.c.
> 
> As far as I can see, there is no reason to push this out from probe-file.c
> because anyway this API using code requires libelf. Without this, I can
> still build perf with NO_LIBELF=1. So I wouldn't like to pick this.
> (I think we can drop this from this series)

Ok. We can drop this. I'll rework patch 6/6.

Thanks,
Naveen



Re: [PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c

2017-03-07 Thread Naveen N. Rao
On 2017/03/07 04:51PM, Masami Hiramatsu wrote:
> On Tue,  7 Mar 2017 16:17:40 +0530
> "Naveen N. Rao"  wrote:
> 
> > probe-file.c needs libelf, but scanning ftrace README does not require
> > that. As such, move the ftrace README scanning logic out of probe-file.c
> > and into trace-event-parse.c.
> 
> As far as I can see, there is no reason to push this out from probe-file.c
> because anyway this API using code requires libelf. Without this, I can
> still build perf with NO_LIBELF=1. So I wouldn't like to pick this.
> (I think we can drop this from this series)

Ok. We can drop this. I'll rework patch 6/6.

Thanks,
Naveen



Re: [PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c

2017-03-07 Thread Naveen N. Rao
On 2017/03/07 03:03PM, Masami Hiramatsu wrote:
> On Tue,  7 Mar 2017 16:17:40 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> 
> > probe-file.c needs libelf, but scanning ftrace README does not require
> > that. As such, move the ftrace README scanning logic out of probe-file.c
> > and into trace-event-parse.c.
> 
> Hmm, it seems probe-file.c doesn't require libelf at all...
> I would like to keep ftrace related things in probe-file.c.

Not sure I understand. probe-file.h explicitly calls out a need for 
libelf due to the probe cache and related routines - commit 
40218daea1db1 ("perf list: Show SDT and pre-cached events").

However, if you prefer to retain the ftrace README scanning here, we can 
drop this patch and I can update patch 6 to check for libelf.

Thanks,
Naveen



Re: [PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c

2017-03-07 Thread Naveen N. Rao
On 2017/03/07 03:03PM, Masami Hiramatsu wrote:
> On Tue,  7 Mar 2017 16:17:40 +0530
> "Naveen N. Rao"  wrote:
> 
> > probe-file.c needs libelf, but scanning ftrace README does not require
> > that. As such, move the ftrace README scanning logic out of probe-file.c
> > and into trace-event-parse.c.
> 
> Hmm, it seems probe-file.c doesn't require libelf at all...
> I would like to keep ftrace related things in probe-file.c.

Not sure I understand. probe-file.h explicitly calls out a need for 
libelf due to the probe cache and related routines - commit 
40218daea1db1 ("perf list: Show SDT and pre-cached events").

However, if you prefer to retain the ftrace README scanning here, we can 
drop this patch and I can update patch 6 to check for libelf.

Thanks,
Naveen



[RESEND PATCH 2/6] powerpc: kretprobes: override default function entry offset

2017-03-07 Thread Naveen N. Rao
With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Acked-by: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>
Acked-by: Michael Ellerman <m...@ellerman.id.au>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kprobes.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index fce05a38851c..331751701fed 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -131,6 +131,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
kcb->kprobe_saved_msr = regs->msr;
 }
 
+bool arch_function_offset_within_entry(unsigned long offset)
+{
+#ifdef PPC64_ELF_ABI_v2
+   return offset <= 8;
+#else
+   return !offset;
+#endif
+}
+
 void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
  struct pt_regs *regs)
 {
-- 
2.11.1



[RESEND PATCH 2/6] powerpc: kretprobes: override default function entry offset

2017-03-07 Thread Naveen N. Rao
With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Acked-by: Ananth N Mavinakayanahalli 
Acked-by: Michael Ellerman 
Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/kprobes.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index fce05a38851c..331751701fed 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -131,6 +131,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
kcb->kprobe_saved_msr = regs->msr;
 }
 
+bool arch_function_offset_within_entry(unsigned long offset)
+{
+#ifdef PPC64_ELF_ABI_v2
+   return offset <= 8;
+#else
+   return !offset;
+#endif
+}
+
 void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
  struct pt_regs *regs)
 {
-- 
2.11.1



[RESEND PATCH 6/6] perf: powerpc: choose local entry point with kretprobes

2017-03-07 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..e93b3db25012 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "trace-event.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,11 +80,12 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe && 
!kretprobe_offset_is_supported())
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.1



[RESEND PATCH 6/6] perf: powerpc: choose local entry point with kretprobes

2017-03-07 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Acked-by: Masami Hiramatsu 
Signed-off-by: Naveen N. Rao 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..e93b3db25012 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "trace-event.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,11 +80,12 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe && 
!kretprobe_offset_is_supported())
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.1



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

2017-03-07 Thread Naveen N. Rao
On 2017/03/06 10:06PM, Masami Hiramatsu wrote:
> On Mon,  6 Mar 2017 23:19:09 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> 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 <mhira...@kernel.org>

Thanks for the review, Masami!
I ended up adding one more patch to this series (patch 5/6) to move the
ftrace README scanning out of probe-file.c, as it doesn't need libelf.
Patch 6 fails to build without libelf otherwise. Please take a look.

Arnaldo,
I am re-sending the remaining patches in this series which apply on top
of the 4 patches you sent to Ingo, so as to keep this simple. All the
patches have been acked, except the new patch 5/6. Kindly take a look.

Thanks,
Naveen

--
Naveen N. Rao (6):
  trace/kprobes: fix check for kretprobe offset within function entry
  powerpc: kretprobes: override default function entry offset
  perf: probe: factor out the ftrace README scanning
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: probes: move ftrace README parsing logic into
trace-event-parse.c
  perf: powerpc: choose local entry point with kretprobes

 arch/powerpc/kernel/kprobes.c   |  9 +++
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 40 -
 kernel/trace/trace_kprobe.c |  2 +-
 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++--
 tools/perf/util/probe-event.c   | 12 ++--
 tools/perf/util/probe-file.c| 80 +++---
 tools/perf/util/probe-file.h|  1 -
 tools/perf/util/trace-event-parse.c | 89 +
 tools/perf/util/trace-event.h   |  4 ++
 10 files changed, 149 insertions(+), 99 deletions(-)

-- 
2.11.1



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

2017-03-07 Thread Naveen N. Rao
On 2017/03/06 10:06PM, Masami Hiramatsu wrote:
> 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 for the review, Masami!
I ended up adding one more patch to this series (patch 5/6) to move the
ftrace README scanning out of probe-file.c, as it doesn't need libelf.
Patch 6 fails to build without libelf otherwise. Please take a look.

Arnaldo,
I am re-sending the remaining patches in this series which apply on top
of the 4 patches you sent to Ingo, so as to keep this simple. All the
patches have been acked, except the new patch 5/6. Kindly take a look.

Thanks,
Naveen

--
Naveen N. Rao (6):
  trace/kprobes: fix check for kretprobe offset within function entry
  powerpc: kretprobes: override default function entry offset
  perf: probe: factor out the ftrace README scanning
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: probes: move ftrace README parsing logic into
trace-event-parse.c
  perf: powerpc: choose local entry point with kretprobes

 arch/powerpc/kernel/kprobes.c   |  9 +++
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 40 -
 kernel/trace/trace_kprobe.c |  2 +-
 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++--
 tools/perf/util/probe-event.c   | 12 ++--
 tools/perf/util/probe-file.c| 80 +++---
 tools/perf/util/probe-file.h|  1 -
 tools/perf/util/trace-event-parse.c | 89 +
 tools/perf/util/trace-event.h   |  4 ++
 10 files changed, 149 insertions(+), 99 deletions(-)

-- 
2.11.1



[PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c

2017-03-07 Thread Naveen N. Rao
probe-file.c needs libelf, but scanning ftrace README does not require
that. As such, move the ftrace README scanning logic out of probe-file.c
and into trace-event-parse.c.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/util/probe-file.c| 87 +++-
 tools/perf/util/probe-file.h|  2 -
 tools/perf/util/trace-event-parse.c | 89 +
 tools/perf/util/trace-event.h   |  4 ++
 4 files changed, 99 insertions(+), 83 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1542cd0d6799..ff872fa30cdb 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -26,6 +26,7 @@
 #include 
 #include "probe-event.h"
 #include "probe-file.h"
+#include "trace-event.h"
 #include "session.h"
 
 #define MAX_CMDLEN 256
@@ -70,33 +71,17 @@ static void print_both_open_warning(int kerr, int uerr)
}
 }
 
-int open_trace_file(const char *trace_file, bool readwrite)
-{
-   char buf[PATH_MAX];
-   int ret;
-
-   ret = e_snprintf(buf, PATH_MAX, "%s/%s",
-tracing_path, trace_file);
-   if (ret >= 0) {
-   pr_debug("Opening %s write=%d\n", buf, readwrite);
-   if (readwrite && !probe_event_dry_run)
-   ret = open(buf, O_RDWR | O_APPEND, 0);
-   else
-   ret = open(buf, O_RDONLY, 0);
-
-   if (ret < 0)
-   ret = -errno;
-   }
-   return ret;
-}
-
 static int open_kprobe_events(bool readwrite)
 {
+   if (probe_event_dry_run)
+   readwrite = false;
return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
+   if (probe_event_dry_run)
+   readwrite = false;
return open_trace_file("uprobe_events", readwrite);
 }
 
@@ -877,72 +862,12 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
-enum ftrace_readme {
-   FTRACE_README_PROBE_TYPE_X = 0,
-   FTRACE_README_KRETPROBE_OFFSET,
-   FTRACE_README_END,
-};
-
-static struct {
-   const char *pattern;
-   bool avail;
-} ftrace_readme_table[] = {
-#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)
-{
-   int fd;
-   FILE *fp;
-   char *buf = NULL;
-   size_t len = 0;
-   bool ret = false;
-   static bool scanned = false;
-
-   if (scanned)
-   goto result;
-
-   fd = open_trace_file("README", false);
-   if (fd < 0)
-   return ret;
-
-   fp = fdopen(fd, "r");
-   if (!fp) {
-   close(fd);
-   return ret;
-   }
-
-   while (getline(, , fp) > 0)
-   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
-   if (!ftrace_readme_table[i].avail)
-   ftrace_readme_table[i].avail =
-   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
-   scanned = true;
-
-   fclose(fp);
-   free(buf);
-
-result:
-   if (type >= FTRACE_README_END)
-   return false;
-
-   return ftrace_readme_table[type].avail;
-}
-
 bool probe_type_is_available(enum probe_type type)
 {
if (type >= PROBE_TYPE_END)
return false;
else if (type == PROBE_TYPE_X)
-   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+   return probe_type_x_is_supported();
 
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 dbf95a00864a..eba44c3e9dca 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -35,7 +35,6 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
-int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);
@@ -65,7 +64,6 @@ 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 prob

[PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c

2017-03-07 Thread Naveen N. Rao
probe-file.c needs libelf, but scanning ftrace README does not require
that. As such, move the ftrace README scanning logic out of probe-file.c
and into trace-event-parse.c.

Signed-off-by: Naveen N. Rao 
---
 tools/perf/util/probe-file.c| 87 +++-
 tools/perf/util/probe-file.h|  2 -
 tools/perf/util/trace-event-parse.c | 89 +
 tools/perf/util/trace-event.h   |  4 ++
 4 files changed, 99 insertions(+), 83 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1542cd0d6799..ff872fa30cdb 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -26,6 +26,7 @@
 #include 
 #include "probe-event.h"
 #include "probe-file.h"
+#include "trace-event.h"
 #include "session.h"
 
 #define MAX_CMDLEN 256
@@ -70,33 +71,17 @@ static void print_both_open_warning(int kerr, int uerr)
}
 }
 
-int open_trace_file(const char *trace_file, bool readwrite)
-{
-   char buf[PATH_MAX];
-   int ret;
-
-   ret = e_snprintf(buf, PATH_MAX, "%s/%s",
-tracing_path, trace_file);
-   if (ret >= 0) {
-   pr_debug("Opening %s write=%d\n", buf, readwrite);
-   if (readwrite && !probe_event_dry_run)
-   ret = open(buf, O_RDWR | O_APPEND, 0);
-   else
-   ret = open(buf, O_RDONLY, 0);
-
-   if (ret < 0)
-   ret = -errno;
-   }
-   return ret;
-}
-
 static int open_kprobe_events(bool readwrite)
 {
+   if (probe_event_dry_run)
+   readwrite = false;
return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
+   if (probe_event_dry_run)
+   readwrite = false;
return open_trace_file("uprobe_events", readwrite);
 }
 
@@ -877,72 +862,12 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
-enum ftrace_readme {
-   FTRACE_README_PROBE_TYPE_X = 0,
-   FTRACE_README_KRETPROBE_OFFSET,
-   FTRACE_README_END,
-};
-
-static struct {
-   const char *pattern;
-   bool avail;
-} ftrace_readme_table[] = {
-#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)
-{
-   int fd;
-   FILE *fp;
-   char *buf = NULL;
-   size_t len = 0;
-   bool ret = false;
-   static bool scanned = false;
-
-   if (scanned)
-   goto result;
-
-   fd = open_trace_file("README", false);
-   if (fd < 0)
-   return ret;
-
-   fp = fdopen(fd, "r");
-   if (!fp) {
-   close(fd);
-   return ret;
-   }
-
-   while (getline(, , fp) > 0)
-   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
-   if (!ftrace_readme_table[i].avail)
-   ftrace_readme_table[i].avail =
-   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
-   scanned = true;
-
-   fclose(fp);
-   free(buf);
-
-result:
-   if (type >= FTRACE_README_END)
-   return false;
-
-   return ftrace_readme_table[type].avail;
-}
-
 bool probe_type_is_available(enum probe_type type)
 {
if (type >= PROBE_TYPE_END)
return false;
else if (type == PROBE_TYPE_X)
-   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+   return probe_type_x_is_supported();
 
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 dbf95a00864a..eba44c3e9dca 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -35,7 +35,6 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
-int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);
@@ -65,7 +64,6 @@ 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 *tg

[RESEND PATCH 4/6] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-07 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.

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]

Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 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 28fb62c32678..c9bdc9ded0c3 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 && !pe

[RESEND PATCH 4/6] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-07 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.

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]

Acked-by: Masami Hiramatsu 
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 28fb62c32678..c9bdc9ded0c3 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

[RESEND PATCH 3/6] perf: probe: factor out the ftrace README scanning

2017-03-07 Thread Naveen N. Rao
Simplify and separate out the ftrace README scanning logic into a
separate helper. This is used subsequently to scan for all patterns of
interest and to cache the result.

Since we are only interested in availability of probe argument type x,
we will only scan for that.

Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/util/probe-file.c | 70 +++-
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1a62daceb028..8a219cd831b7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -877,35 +877,31 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
+enum ftrace_readme {
+   FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_END,
+};
+
 static struct {
const char *pattern;
-   boolavail;
-   boolchecked;
-} probe_type_table[] = {
-#define DEFINE_TYPE(idx, pat, def_avail)   \
-   [idx] = {.pattern = pat, .avail = (def_avail)}
-   DEFINE_TYPE(PROBE_TYPE_U, "* u8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_S, "* s8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_X, "* x8/16/32/64,*", false),
-   DEFINE_TYPE(PROBE_TYPE_STRING, "* string,*", true),
-   DEFINE_TYPE(PROBE_TYPE_BITFIELD,
-   "* b@/", true),
+   bool avail;
+} ftrace_readme_table[] = {
+#define DEFINE_TYPE(idx, pat)  \
+   [idx] = {.pattern = pat, .avail = false}
+   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
 };
 
-bool probe_type_is_available(enum probe_type type)
+static bool scan_ftrace_readme(enum ftrace_readme type)
 {
+   int fd;
FILE *fp;
char *buf = NULL;
size_t len = 0;
-   bool target_line = false;
-   bool ret = probe_type_table[type].avail;
-   int fd;
+   bool ret = false;
+   static bool scanned = false;
 
-   if (type >= PROBE_TYPE_END)
-   return false;
-   /* We don't have to check the type which supported by default */
-   if (ret || probe_type_table[type].checked)
-   return ret;
+   if (scanned)
+   goto result;
 
fd = open_trace_file("README", false);
if (fd < 0)
@@ -917,21 +913,29 @@ bool probe_type_is_available(enum probe_type type)
return ret;
}
 
-   while (getline(, , fp) > 0 && !ret) {
-   if (!target_line) {
-   target_line = !!strstr(buf, " type: ");
-   if (!target_line)
-   continue;
-   } else if (strstr(buf, "\t  ") != buf)
-   break;
-   ret = strglobmatch(buf, probe_type_table[type].pattern);
-   }
-   /* Cache the result */
-   probe_type_table[type].checked = true;
-   probe_type_table[type].avail = ret;
+   while (getline(, , fp) > 0)
+   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
+   if (!ftrace_readme_table[i].avail)
+   ftrace_readme_table[i].avail =
+   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
+   scanned = true;
 
fclose(fp);
free(buf);
 
-   return ret;
+result:
+   if (type >= FTRACE_README_END)
+   return false;
+
+   return ftrace_readme_table[type].avail;
+}
+
+bool probe_type_is_available(enum probe_type type)
+{
+   if (type >= PROBE_TYPE_END)
+   return false;
+   else if (type == PROBE_TYPE_X)
+   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+
+   return true;
 }
-- 
2.11.1



[RESEND PATCH 3/6] perf: probe: factor out the ftrace README scanning

2017-03-07 Thread Naveen N. Rao
Simplify and separate out the ftrace README scanning logic into a
separate helper. This is used subsequently to scan for all patterns of
interest and to cache the result.

Since we are only interested in availability of probe argument type x,
we will only scan for that.

Acked-by: Masami Hiramatsu 
Signed-off-by: Naveen N. Rao 
---
 tools/perf/util/probe-file.c | 70 +++-
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1a62daceb028..8a219cd831b7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -877,35 +877,31 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
+enum ftrace_readme {
+   FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_END,
+};
+
 static struct {
const char *pattern;
-   boolavail;
-   boolchecked;
-} probe_type_table[] = {
-#define DEFINE_TYPE(idx, pat, def_avail)   \
-   [idx] = {.pattern = pat, .avail = (def_avail)}
-   DEFINE_TYPE(PROBE_TYPE_U, "* u8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_S, "* s8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_X, "* x8/16/32/64,*", false),
-   DEFINE_TYPE(PROBE_TYPE_STRING, "* string,*", true),
-   DEFINE_TYPE(PROBE_TYPE_BITFIELD,
-   "* b@/", true),
+   bool avail;
+} ftrace_readme_table[] = {
+#define DEFINE_TYPE(idx, pat)  \
+   [idx] = {.pattern = pat, .avail = false}
+   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
 };
 
-bool probe_type_is_available(enum probe_type type)
+static bool scan_ftrace_readme(enum ftrace_readme type)
 {
+   int fd;
FILE *fp;
char *buf = NULL;
size_t len = 0;
-   bool target_line = false;
-   bool ret = probe_type_table[type].avail;
-   int fd;
+   bool ret = false;
+   static bool scanned = false;
 
-   if (type >= PROBE_TYPE_END)
-   return false;
-   /* We don't have to check the type which supported by default */
-   if (ret || probe_type_table[type].checked)
-   return ret;
+   if (scanned)
+   goto result;
 
fd = open_trace_file("README", false);
if (fd < 0)
@@ -917,21 +913,29 @@ bool probe_type_is_available(enum probe_type type)
return ret;
}
 
-   while (getline(, , fp) > 0 && !ret) {
-   if (!target_line) {
-   target_line = !!strstr(buf, " type: ");
-   if (!target_line)
-   continue;
-   } else if (strstr(buf, "\t  ") != buf)
-   break;
-   ret = strglobmatch(buf, probe_type_table[type].pattern);
-   }
-   /* Cache the result */
-   probe_type_table[type].checked = true;
-   probe_type_table[type].avail = ret;
+   while (getline(, , fp) > 0)
+   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
+   if (!ftrace_readme_table[i].avail)
+   ftrace_readme_table[i].avail =
+   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
+   scanned = true;
 
fclose(fp);
free(buf);
 
-   return ret;
+result:
+   if (type >= FTRACE_README_END)
+   return false;
+
+   return ftrace_readme_table[type].avail;
+}
+
+bool probe_type_is_available(enum probe_type type)
+{
+   if (type >= PROBE_TYPE_END)
+   return false;
+   else if (type == PROBE_TYPE_X)
+   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+
+   return true;
 }
-- 
2.11.1



[RESEND PATCH 1/6] trace/kprobes: fix check for kretprobe offset within function entry

2017-03-07 Thread Naveen N. Rao
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]

Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 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 177bdf6c6aeb..47e4da5b4fa2 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -268,6 +268,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 448759d4a263..32e6ac5131ed 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 = (kprobe_opcode_t *)(((char *)addr) + p->offset);
+   addr = (kprobe_opcode_t *)(((char *)addr) +

[RESEND PATCH 1/6] trace/kprobes: fix check for kretprobe offset within function entry

2017-03-07 Thread Naveen N. Rao
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]

Acked-by: Masami Hiramatsu 
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 177bdf6c6aeb..47e4da5b4fa2 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -268,6 +268,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 448759d4a263..32e6ac5131ed 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 = (kprobe_opcode_t *)(((char *)addr) + p->offset);
+   addr = (kprobe_opcode_t *)(((char *)addr) + offset);
if (addr)
return addr;
 

[tip:perf/core] perf probe: Generalize probe event file open routine

2017-03-07 Thread tip-bot for Naveen N. Rao
Commit-ID:  e491bc2f0dd9f1b4a23ba6f3da88f6b695c4a4c9
Gitweb: http://git.kernel.org/tip/e491bc2f0dd9f1b4a23ba6f3da88f6b695c4a4c9
Author: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
AuthorDate: Thu, 23 Feb 2017 17:07:23 +0530
Committer:  Arnaldo Carvalho de Melo <a...@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:18 -0300

perf probe: Generalize probe event file open routine

Generalize probe event file open routine into a generic function for opening
trace files.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Cc: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/b580465c7a4dcd5d3b40fdf8568e6be45d0a6333.1487849577.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com>
---
 tools/perf/util/probe-file.c | 20 +++-
 tools/perf/util/probe-file.h |  1 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..1a62dac 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -70,7 +70,7 @@ static void print_both_open_warning(int kerr, int uerr)
}
 }
 
-static int open_probe_events(const char *trace_file, bool readwrite)
+int open_trace_file(const char *trace_file, bool readwrite)
 {
char buf[PATH_MAX];
int ret;
@@ -92,12 +92,12 @@ static int open_probe_events(const char *trace_file, bool 
readwrite)
 
 static int open_kprobe_events(bool readwrite)
 {
-   return open_probe_events("kprobe_events", readwrite);
+   return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
-   return open_probe_events("uprobe_events", readwrite);
+   return open_trace_file("uprobe_events", readwrite);
 }
 
 int probe_file__open(int flag)
@@ -899,6 +899,7 @@ bool probe_type_is_available(enum probe_type type)
size_t len = 0;
bool target_line = false;
bool ret = probe_type_table[type].avail;
+   int fd;
 
if (type >= PROBE_TYPE_END)
return false;
@@ -906,14 +907,16 @@ bool probe_type_is_available(enum probe_type type)
if (ret || probe_type_table[type].checked)
return ret;
 
-   if (asprintf(, "%s/README", tracing_path) < 0)
+   fd = open_trace_file("README", false);
+   if (fd < 0)
return ret;
 
-   fp = fopen(buf, "r");
-   if (!fp)
-   goto end;
+   fp = fdopen(fd, "r");
+   if (!fp) {
+   close(fd);
+   return ret;
+   }
 
-   zfree();
while (getline(, , fp) > 0 && !ret) {
if (!target_line) {
target_line = !!strstr(buf, " type: ");
@@ -928,7 +931,6 @@ bool probe_type_is_available(enum probe_type type)
probe_type_table[type].avail = ret;
 
fclose(fp);
-end:
free(buf);
 
return ret;
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index eba44c3..a17a82e 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -35,6 +35,7 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
+int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);


[tip:perf/core] perf probe: Generalize probe event file open routine

2017-03-07 Thread tip-bot for Naveen N. Rao
Commit-ID:  e491bc2f0dd9f1b4a23ba6f3da88f6b695c4a4c9
Gitweb: http://git.kernel.org/tip/e491bc2f0dd9f1b4a23ba6f3da88f6b695c4a4c9
Author: Naveen N. Rao 
AuthorDate: Thu, 23 Feb 2017 17:07:23 +0530
Committer:  Arnaldo Carvalho de Melo 
CommitDate: Fri, 3 Mar 2017 19:07:18 -0300

perf probe: Generalize probe event file open routine

Generalize probe event file open routine into a generic function for opening
trace files.

Signed-off-by: Naveen N. Rao 
Acked-by: Masami Hiramatsu 
Cc: Ananth N Mavinakayanahalli 
Cc: Michael Ellerman 
Cc: Steven Rostedt 
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/b580465c7a4dcd5d3b40fdf8568e6be45d0a6333.1487849577.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/probe-file.c | 20 +++-
 tools/perf/util/probe-file.h |  1 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b647..1a62dac 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -70,7 +70,7 @@ static void print_both_open_warning(int kerr, int uerr)
}
 }
 
-static int open_probe_events(const char *trace_file, bool readwrite)
+int open_trace_file(const char *trace_file, bool readwrite)
 {
char buf[PATH_MAX];
int ret;
@@ -92,12 +92,12 @@ static int open_probe_events(const char *trace_file, bool 
readwrite)
 
 static int open_kprobe_events(bool readwrite)
 {
-   return open_probe_events("kprobe_events", readwrite);
+   return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
-   return open_probe_events("uprobe_events", readwrite);
+   return open_trace_file("uprobe_events", readwrite);
 }
 
 int probe_file__open(int flag)
@@ -899,6 +899,7 @@ bool probe_type_is_available(enum probe_type type)
size_t len = 0;
bool target_line = false;
bool ret = probe_type_table[type].avail;
+   int fd;
 
if (type >= PROBE_TYPE_END)
return false;
@@ -906,14 +907,16 @@ bool probe_type_is_available(enum probe_type type)
if (ret || probe_type_table[type].checked)
return ret;
 
-   if (asprintf(, "%s/README", tracing_path) < 0)
+   fd = open_trace_file("README", false);
+   if (fd < 0)
return ret;
 
-   fp = fopen(buf, "r");
-   if (!fp)
-   goto end;
+   fp = fdopen(fd, "r");
+   if (!fp) {
+   close(fd);
+   return ret;
+   }
 
-   zfree();
while (getline(, , fp) > 0 && !ret) {
if (!target_line) {
target_line = !!strstr(buf, " type: ");
@@ -928,7 +931,6 @@ bool probe_type_is_available(enum probe_type type)
probe_type_table[type].avail = ret;
 
fclose(fp);
-end:
free(buf);
 
return ret;
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index eba44c3..a17a82e 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -35,6 +35,7 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
+int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);


[tip:perf/core] trace/kprobes: Allow return probes with offsets and absolute addresses

2017-03-07 Thread tip-bot for Naveen N. Rao
Commit-ID:  35b6f55aa9ba65141f2def0997e23aab13715d3f
Gitweb: http://git.kernel.org/tip/35b6f55aa9ba65141f2def0997e23aab13715d3f
Author: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
AuthorDate: Wed, 22 Feb 2017 19:23:39 +0530
Committer:  Arnaldo Carvalho de Melo <a...@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:18 -0300

trace/kprobes: Allow return probes with offsets and absolute addresses

Since the kernel includes many non-global functions with same names, we
will need to use offsets from other symbols (typically _text/_stext) or
absolute addresses to place return probes on specific functions. Also,
the core register_kretprobe() API never forbid use of offsets or
absolute addresses with kretprobes.

Allow its use with the trace infrastructure. To distinguish kernels that
support this, update ftrace README to explicitly call this out.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Cc: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/183e7ce2921a08c9c755ee9a5da3134febc6695b.1487770934.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com>
---
 kernel/trace/trace.c| 1 +
 kernel/trace/trace_kprobe.c | 8 
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index f351095..0ed834d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4355,6 +4355,7 @@ static const char readme_msg[] =
"\t   -:[/]\n"
 #ifdef CONFIG_KPROBE_EVENTS
"\tplace: [:][+]|\n"
+  "place (kretprobe): [:][+]|\n"
 #endif
 #ifdef CONFIG_UPROBE_EVENTS
"\tplace: :\n"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index eadd96e..18775ef 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -680,10 +680,6 @@ static int create_trace_kprobe(int argc, char **argv)
return -EINVAL;
}
if (isdigit(argv[1][0])) {
-   if (is_return) {
-   pr_info("Return probe point must be a symbol.\n");
-   return -EINVAL;
-   }
/* an address specified */
ret = kstrtoul([1][0], 0, (unsigned long *));
if (ret) {
@@ -699,10 +695,6 @@ static int create_trace_kprobe(int argc, char **argv)
pr_info("Failed to parse symbol.\n");
return ret;
}
-   if (offset && is_return) {
-   pr_info("Return probe must be used without offset.\n");
-   return -EINVAL;
-   }
}
argc -= 2; argv += 2;
 


[tip:perf/core] trace/kprobes: Allow return probes with offsets and absolute addresses

2017-03-07 Thread tip-bot for Naveen N. Rao
Commit-ID:  35b6f55aa9ba65141f2def0997e23aab13715d3f
Gitweb: http://git.kernel.org/tip/35b6f55aa9ba65141f2def0997e23aab13715d3f
Author: Naveen N. Rao 
AuthorDate: Wed, 22 Feb 2017 19:23:39 +0530
Committer:  Arnaldo Carvalho de Melo 
CommitDate: Fri, 3 Mar 2017 19:07:18 -0300

trace/kprobes: Allow return probes with offsets and absolute addresses

Since the kernel includes many non-global functions with same names, we
will need to use offsets from other symbols (typically _text/_stext) or
absolute addresses to place return probes on specific functions. Also,
the core register_kretprobe() API never forbid use of offsets or
absolute addresses with kretprobes.

Allow its use with the trace infrastructure. To distinguish kernels that
support this, update ftrace README to explicitly call this out.

Signed-off-by: Naveen N. Rao 
Acked-by: Masami Hiramatsu 
Cc: Ananth N Mavinakayanahalli 
Cc: Michael Ellerman 
Cc: Steven Rostedt 
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/183e7ce2921a08c9c755ee9a5da3134febc6695b.1487770934.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 kernel/trace/trace.c| 1 +
 kernel/trace/trace_kprobe.c | 8 
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index f351095..0ed834d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4355,6 +4355,7 @@ static const char readme_msg[] =
"\t   -:[/]\n"
 #ifdef CONFIG_KPROBE_EVENTS
"\tplace: [:][+]|\n"
+  "place (kretprobe): [:][+]|\n"
 #endif
 #ifdef CONFIG_UPROBE_EVENTS
"\tplace: :\n"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index eadd96e..18775ef 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -680,10 +680,6 @@ static int create_trace_kprobe(int argc, char **argv)
return -EINVAL;
}
if (isdigit(argv[1][0])) {
-   if (is_return) {
-   pr_info("Return probe point must be a symbol.\n");
-   return -EINVAL;
-   }
/* an address specified */
ret = kstrtoul([1][0], 0, (unsigned long *));
if (ret) {
@@ -699,10 +695,6 @@ static int create_trace_kprobe(int argc, char **argv)
pr_info("Failed to parse symbol.\n");
return ret;
}
-   if (offset && is_return) {
-   pr_info("Return probe must be used without offset.\n");
-   return -EINVAL;
-   }
}
argc -= 2; argv += 2;
 


[tip:perf/core] kretprobes: Ensure probe location is at function entry

2017-03-07 Thread tip-bot for Naveen N. Rao
Commit-ID:  90ec5e89e393c76e19afc845d8f88a5dc8315919
Gitweb: http://git.kernel.org/tip/90ec5e89e393c76e19afc845d8f88a5dc8315919
Author: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
AuthorDate: Wed, 22 Feb 2017 19:23:37 +0530
Committer:  Arnaldo Carvalho de Melo <a...@redhat.com>
CommitDate: Fri, 3 Mar 2017 19:07:17 -0300

kretprobes: Ensure probe location is at function entry

kretprobes can be registered by specifying an absolute address or by
specifying offset to a symbol. However, we need to ensure this falls at
function entry so as to be able to determine the return address.

Validate the same during kretprobe registration. By default, there
should not be any offset from a function entry, as determined through a
kallsyms_lookup(). Introduce arch_function_offset_within_entry() as a
way for architectures to override this.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
Acked-by: Masami Hiramatsu <mhira...@kernel.org>
Cc: Ananth N Mavinakayanahalli <ana...@linux.vnet.ibm.com>
Cc: Michael Ellerman <m...@ellerman.id.au>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/f1583bc4839a3862cfc2acefcc56f9c8837fa2ba.1487770934.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com>
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 13 +
 2 files changed, 14 insertions(+)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index c328e4f..177bdf6 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -267,6 +267,7 @@ extern int arch_init_kprobes(void);
 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 within_kprobe_blacklist(unsigned long addr);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 699c5bc..448759d 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1875,12 +1875,25 @@ static int pre_handler_kretprobe(struct kprobe *p, 
struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
 
+bool __weak arch_function_offset_within_entry(unsigned long offset)
+{
+   return !offset;
+}
+
 int register_kretprobe(struct kretprobe *rp)
 {
int ret = 0;
struct kretprobe_instance *inst;
int i;
void *addr;
+   unsigned long offset;
+
+   addr = kprobe_addr(>kp);
+   if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, ))
+   return -EINVAL;
+
+   if (!arch_function_offset_within_entry(offset))
+   return -EINVAL;
 
if (kretprobe_blacklist_size) {
addr = kprobe_addr(>kp);


[tip:perf/core] kretprobes: Ensure probe location is at function entry

2017-03-07 Thread tip-bot for Naveen N. Rao
Commit-ID:  90ec5e89e393c76e19afc845d8f88a5dc8315919
Gitweb: http://git.kernel.org/tip/90ec5e89e393c76e19afc845d8f88a5dc8315919
Author: Naveen N. Rao 
AuthorDate: Wed, 22 Feb 2017 19:23:37 +0530
Committer:  Arnaldo Carvalho de Melo 
CommitDate: Fri, 3 Mar 2017 19:07:17 -0300

kretprobes: Ensure probe location is at function entry

kretprobes can be registered by specifying an absolute address or by
specifying offset to a symbol. However, we need to ensure this falls at
function entry so as to be able to determine the return address.

Validate the same during kretprobe registration. By default, there
should not be any offset from a function entry, as determined through a
kallsyms_lookup(). Introduce arch_function_offset_within_entry() as a
way for architectures to override this.

Signed-off-by: Naveen N. Rao 
Acked-by: Masami Hiramatsu 
Cc: Ananth N Mavinakayanahalli 
Cc: Michael Ellerman 
Cc: Steven Rostedt 
Cc: linuxppc-...@lists.ozlabs.org
Link: 
http://lkml.kernel.org/r/f1583bc4839a3862cfc2acefcc56f9c8837fa2ba.1487770934.git.naveen.n@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 13 +
 2 files changed, 14 insertions(+)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index c328e4f..177bdf6 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -267,6 +267,7 @@ extern int arch_init_kprobes(void);
 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 within_kprobe_blacklist(unsigned long addr);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 699c5bc..448759d 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1875,12 +1875,25 @@ static int pre_handler_kretprobe(struct kprobe *p, 
struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
 
+bool __weak arch_function_offset_within_entry(unsigned long offset)
+{
+   return !offset;
+}
+
 int register_kretprobe(struct kretprobe *rp)
 {
int ret = 0;
struct kretprobe_instance *inst;
int i;
void *addr;
+   unsigned long offset;
+
+   addr = kprobe_addr(>kp);
+   if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, ))
+   return -EINVAL;
+
+   if (!arch_function_offset_within_entry(offset))
+   return -EINVAL;
 
if (kretprobe_blacklist_size) {
addr = kprobe_addr(>kp);


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 <naveen.n@linux.vnet.ibm.com>
---
 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, a

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)

[PATCH 2/2] arm64: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
Commit fc62d0207ae0 ("kprobes: Introduce weak variant of
kprobe_exceptions_notify()") introduces a generic empty version of the
function for architectures that don't need special handling, like arm64.
As such, remove the arch/arm64/ specific handler.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/arm64/kernel/probes/kprobes.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c 
b/arch/arm64/kernel/probes/kprobes.c
index 2a07aae5b8a2..c5c45942fb6e 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -372,12 +372,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, 
unsigned int fsr)
return 0;
 }
 
-int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
-  unsigned long val, void *data)
-{
-   return NOTIFY_DONE;
-}
-
 static void __kprobes kprobe_handler(struct pt_regs *regs)
 {
struct kprobe *p, *cur_kprobe;
-- 
2.11.1



[PATCH 2/2] arm64: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
Commit fc62d0207ae0 ("kprobes: Introduce weak variant of
kprobe_exceptions_notify()") introduces a generic empty version of the
function for architectures that don't need special handling, like arm64.
As such, remove the arch/arm64/ specific handler.

Signed-off-by: Naveen N. Rao 
---
 arch/arm64/kernel/probes/kprobes.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/arch/arm64/kernel/probes/kprobes.c 
b/arch/arm64/kernel/probes/kprobes.c
index 2a07aae5b8a2..c5c45942fb6e 100644
--- a/arch/arm64/kernel/probes/kprobes.c
+++ b/arch/arm64/kernel/probes/kprobes.c
@@ -372,12 +372,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, 
unsigned int fsr)
return 0;
 }
 
-int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
-  unsigned long val, void *data)
-{
-   return NOTIFY_DONE;
-}
-
 static void __kprobes kprobe_handler(struct pt_regs *regs)
 {
struct kprobe *p, *cur_kprobe;
-- 
2.11.1



Re: [PATCH 2/3] arm: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
On 2017/03/06 06:38PM, Russell King - ARM Linux wrote:
> On Mon, Mar 06, 2017 at 11:37:20PM +0530, Naveen N. Rao wrote:
> > On 2017/02/08 01:24AM, Naveen N Rao wrote:
> > > ... as the weak variant will do.
> > > 
> > > Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> > > ---
> > >  arch/arm/probes/kprobes/core.c | 10 --
> > >  arch/arm64/kernel/probes/kprobes.c |  6 --
> > >  2 files changed, 16 deletions(-)
> > 
> > With the generic changes in this series now in -rc1, can you please pick 
> > this up?
> 
> It would've been nice to have been in the To: or Cc: on this patch,
> I suspect everyone on the ARM side ignored this series (I certainly
> didn't notice it, and I suspect the ARM64 folk didn't notice it for
> exactly the same reason.)
> 
> In any case, this patch needs to be split - ARM and ARM64 are
> maintained separately (as stated in MAINTAINERS), and patches go via
> different trees.  Please resubmit with the patch split between the
> architectures and proper recipients in the headers.

Got it. Please find the updated patches in this thread.

Thanks,
Naveen



Re: [PATCH 2/3] arm: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
On 2017/03/06 06:38PM, Russell King - ARM Linux wrote:
> On Mon, Mar 06, 2017 at 11:37:20PM +0530, Naveen N. Rao wrote:
> > On 2017/02/08 01:24AM, Naveen N Rao wrote:
> > > ... as the weak variant will do.
> > > 
> > > Signed-off-by: Naveen N. Rao 
> > > ---
> > >  arch/arm/probes/kprobes/core.c | 10 --
> > >  arch/arm64/kernel/probes/kprobes.c |  6 --
> > >  2 files changed, 16 deletions(-)
> > 
> > With the generic changes in this series now in -rc1, can you please pick 
> > this up?
> 
> It would've been nice to have been in the To: or Cc: on this patch,
> I suspect everyone on the ARM side ignored this series (I certainly
> didn't notice it, and I suspect the ARM64 folk didn't notice it for
> exactly the same reason.)
> 
> In any case, this patch needs to be split - ARM and ARM64 are
> maintained separately (as stated in MAINTAINERS), and patches go via
> different trees.  Please resubmit with the patch split between the
> architectures and proper recipients in the headers.

Got it. Please find the updated patches in this thread.

Thanks,
Naveen



[PATCH 1/2] arm: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
Commit fc62d0207ae0 ("kprobes: Introduce weak variant of
kprobe_exceptions_notify()") introduces a generic empty version of the
function for architectures that don't need special handling, like arm.
As such, remove the arch/arm/ specific handler.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/arm/probes/kprobes/core.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index b6dc9d838a9a..4a548b99965b 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -392,16 +392,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, 
unsigned int fsr)
return 0;
 }
 
-int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
-  unsigned long val, void *data)
-{
-   /*
-* notify_die() is currently never called on ARM,
-* so this callback is currently empty.
-*/
-   return NOTIFY_DONE;
-}
-
 /*
  * When a retprobed function returns, trampoline_handler() is called,
  * calling the kretprobe's handler. We construct a struct pt_regs to
-- 
2.11.1



[PATCH 1/2] arm: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
Commit fc62d0207ae0 ("kprobes: Introduce weak variant of
kprobe_exceptions_notify()") introduces a generic empty version of the
function for architectures that don't need special handling, like arm.
As such, remove the arch/arm/ specific handler.

Signed-off-by: Naveen N. Rao 
---
 arch/arm/probes/kprobes/core.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index b6dc9d838a9a..4a548b99965b 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -392,16 +392,6 @@ int __kprobes kprobe_fault_handler(struct pt_regs *regs, 
unsigned int fsr)
return 0;
 }
 
-int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
-  unsigned long val, void *data)
-{
-   /*
-* notify_die() is currently never called on ARM,
-* so this callback is currently empty.
-*/
-   return NOTIFY_DONE;
-}
-
 /*
  * When a retprobed function returns, trampoline_handler() is called,
  * calling the kretprobe's handler. We construct a struct pt_regs to
-- 
2.11.1



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 <mhira...@kernel.org> wrote:
> 
> > On Sat, 4 Mar 2017 09:49:11 +0900
> > Masami Hiramatsu <mhira...@kernel.org> wrote:
> > 
> > > On Thu,  2 Mar 2017 23:25:06 +0530
> > > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> 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 2/3] arm: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
On 2017/02/08 01:24AM, Naveen N Rao wrote:
> ... as the weak variant will do.
> 
> Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> ---
>  arch/arm/probes/kprobes/core.c | 10 --
>  arch/arm64/kernel/probes/kprobes.c |  6 --
>  2 files changed, 16 deletions(-)

With the generic changes in this series now in -rc1, can you please pick 
this up?

Thanks,
Naveen



Re: [PATCH 2/3] arm: kprobes: remove kprobe_exceptions_notify

2017-03-06 Thread Naveen N. Rao
On 2017/02/08 01:24AM, Naveen N Rao wrote:
> ... as the weak variant will do.
> 
> Signed-off-by: Naveen N. Rao 
> ---
>  arch/arm/probes/kprobes/core.c | 10 --
>  arch/arm64/kernel/probes/kprobes.c |  6 --
>  2 files changed, 16 deletions(-)

With the generic changes in this series now in -rc1, can you please pick 
this up?

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" <naveen.n@linux.vnet.ibm.com> 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



[PATCH v4 1/3] perf: probe: factor out the ftrace README scanning

2017-03-02 Thread Naveen N. Rao
Simplify and separate out the ftrace README scanning logic into a
separate helper. This is used subsequently to scan for all patterns of
interest and to cache the result.

Since we are only interested in availability of probe argument type x,
we will only scan for that.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/util/probe-file.c | 70 +++-
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1a62daceb028..8a219cd831b7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -877,35 +877,31 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
+enum ftrace_readme {
+   FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_END,
+};
+
 static struct {
const char *pattern;
-   boolavail;
-   boolchecked;
-} probe_type_table[] = {
-#define DEFINE_TYPE(idx, pat, def_avail)   \
-   [idx] = {.pattern = pat, .avail = (def_avail)}
-   DEFINE_TYPE(PROBE_TYPE_U, "* u8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_S, "* s8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_X, "* x8/16/32/64,*", false),
-   DEFINE_TYPE(PROBE_TYPE_STRING, "* string,*", true),
-   DEFINE_TYPE(PROBE_TYPE_BITFIELD,
-   "* b@/", true),
+   bool avail;
+} ftrace_readme_table[] = {
+#define DEFINE_TYPE(idx, pat)  \
+   [idx] = {.pattern = pat, .avail = false}
+   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
 };
 
-bool probe_type_is_available(enum probe_type type)
+static bool scan_ftrace_readme(enum ftrace_readme type)
 {
+   int fd;
FILE *fp;
char *buf = NULL;
size_t len = 0;
-   bool target_line = false;
-   bool ret = probe_type_table[type].avail;
-   int fd;
+   bool ret = false;
+   static bool scanned = false;
 
-   if (type >= PROBE_TYPE_END)
-   return false;
-   /* We don't have to check the type which supported by default */
-   if (ret || probe_type_table[type].checked)
-   return ret;
+   if (scanned)
+   goto result;
 
fd = open_trace_file("README", false);
if (fd < 0)
@@ -917,21 +913,29 @@ bool probe_type_is_available(enum probe_type type)
return ret;
}
 
-   while (getline(, , fp) > 0 && !ret) {
-   if (!target_line) {
-   target_line = !!strstr(buf, " type: ");
-   if (!target_line)
-   continue;
-   } else if (strstr(buf, "\t  ") != buf)
-   break;
-   ret = strglobmatch(buf, probe_type_table[type].pattern);
-   }
-   /* Cache the result */
-   probe_type_table[type].checked = true;
-   probe_type_table[type].avail = ret;
+   while (getline(, , fp) > 0)
+   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
+   if (!ftrace_readme_table[i].avail)
+   ftrace_readme_table[i].avail =
+   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
+   scanned = true;
 
fclose(fp);
free(buf);
 
-   return ret;
+result:
+   if (type >= FTRACE_README_END)
+   return false;
+
+   return ftrace_readme_table[type].avail;
+}
+
+bool probe_type_is_available(enum probe_type type)
+{
+   if (type >= PROBE_TYPE_END)
+   return false;
+   else if (type == PROBE_TYPE_X)
+   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+
+   return true;
 }
-- 
2.11.1



[PATCH v4 1/3] perf: probe: factor out the ftrace README scanning

2017-03-02 Thread Naveen N. Rao
Simplify and separate out the ftrace README scanning logic into a
separate helper. This is used subsequently to scan for all patterns of
interest and to cache the result.

Since we are only interested in availability of probe argument type x,
we will only scan for that.

Signed-off-by: Naveen N. Rao 
---
 tools/perf/util/probe-file.c | 70 +++-
 1 file changed, 37 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 1a62daceb028..8a219cd831b7 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -877,35 +877,31 @@ int probe_cache__show_all_caches(struct strfilter *filter)
return 0;
 }
 
+enum ftrace_readme {
+   FTRACE_README_PROBE_TYPE_X = 0,
+   FTRACE_README_END,
+};
+
 static struct {
const char *pattern;
-   boolavail;
-   boolchecked;
-} probe_type_table[] = {
-#define DEFINE_TYPE(idx, pat, def_avail)   \
-   [idx] = {.pattern = pat, .avail = (def_avail)}
-   DEFINE_TYPE(PROBE_TYPE_U, "* u8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_S, "* s8/16/32/64,*", true),
-   DEFINE_TYPE(PROBE_TYPE_X, "* x8/16/32/64,*", false),
-   DEFINE_TYPE(PROBE_TYPE_STRING, "* string,*", true),
-   DEFINE_TYPE(PROBE_TYPE_BITFIELD,
-   "* b@/", true),
+   bool avail;
+} ftrace_readme_table[] = {
+#define DEFINE_TYPE(idx, pat)  \
+   [idx] = {.pattern = pat, .avail = false}
+   DEFINE_TYPE(FTRACE_README_PROBE_TYPE_X, "*type: * x8/16/32/64,*"),
 };
 
-bool probe_type_is_available(enum probe_type type)
+static bool scan_ftrace_readme(enum ftrace_readme type)
 {
+   int fd;
FILE *fp;
char *buf = NULL;
size_t len = 0;
-   bool target_line = false;
-   bool ret = probe_type_table[type].avail;
-   int fd;
+   bool ret = false;
+   static bool scanned = false;
 
-   if (type >= PROBE_TYPE_END)
-   return false;
-   /* We don't have to check the type which supported by default */
-   if (ret || probe_type_table[type].checked)
-   return ret;
+   if (scanned)
+   goto result;
 
fd = open_trace_file("README", false);
if (fd < 0)
@@ -917,21 +913,29 @@ bool probe_type_is_available(enum probe_type type)
return ret;
}
 
-   while (getline(, , fp) > 0 && !ret) {
-   if (!target_line) {
-   target_line = !!strstr(buf, " type: ");
-   if (!target_line)
-   continue;
-   } else if (strstr(buf, "\t  ") != buf)
-   break;
-   ret = strglobmatch(buf, probe_type_table[type].pattern);
-   }
-   /* Cache the result */
-   probe_type_table[type].checked = true;
-   probe_type_table[type].avail = ret;
+   while (getline(, , fp) > 0)
+   for (enum ftrace_readme i = 0; i < FTRACE_README_END; i++)
+   if (!ftrace_readme_table[i].avail)
+   ftrace_readme_table[i].avail =
+   strglobmatch(buf, 
ftrace_readme_table[i].pattern);
+   scanned = true;
 
fclose(fp);
free(buf);
 
-   return ret;
+result:
+   if (type >= FTRACE_README_END)
+   return false;
+
+   return ftrace_readme_table[type].avail;
+}
+
+bool probe_type_is_available(enum probe_type type)
+{
+   if (type >= PROBE_TYPE_END)
+   return false;
+   else if (type == PROBE_TYPE_X)
+   return scan_ftrace_readme(FTRACE_README_PROBE_TYPE_X);
+
+   return true;
 }
-- 
2.11.1



Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-02 Thread Naveen N. Rao
On 2017/02/24 05:11PM, Arnaldo Carvalho de Melo wrote:
> Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:
> > On Fri, 24 Feb 2017 00:46:08 +0530
> > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> > > Thanks. I hope that's an Ack for this patchset?
> > 
> > OK, for 1/5, 2/5, 3/5, and 5/5;
> > 
> > Acked-by: Masami Hiramatsu <mhira...@kernel.org>
> > 
> > And could you make v4 series including all patches? (Not only updates)
> 
> So, to make progress I processed these:
> 
> [acme@jouet linux]$ git log --oneline -3
> eb55608340b7 perf probe: Generalize probe event file open routine
> 859d718fac06 trace/kprobes: Allow return probes with offsets and absolute 
> addresses
> a10489121c81 kretprobes: Ensure probe location is at function entry
> [acme@jouet linux]$
> 
> Waiting for Naveen to react to these last minute considerations from
> Masami and for the Ack from the PPC guys about "[PATCH v2 2/5] powerpc:
> kretprobes: override default function entry offset".

Arnaldo,
I am posting the remaining three patches in this series. These three
patches are on top of the above 3 patches you have processed and the
other powerpc kretprobes patch (v2 2/5).

Masami,
Kindly review and let me know if this is fine.


Thanks,
Naveen

---
Naveen N. Rao (3):
  perf: probe: factor out the ftrace README scanning
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: powerpc: choose local entry point with kretprobes

 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++--
 tools/perf/util/probe-event.c   | 12 ++---
 tools/perf/util/probe-file.c| 77 -
 tools/perf/util/probe-file.h|  1 +
 4 files changed, 56 insertions(+), 44 deletions(-)

-- 
2.11.1



Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-02 Thread Naveen N. Rao
On 2017/02/24 05:11PM, Arnaldo Carvalho de Melo wrote:
> Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:
> > On Fri, 24 Feb 2017 00:46:08 +0530
> > "Naveen N. Rao"  wrote:
> > > Thanks. I hope that's an Ack for this patchset?
> > 
> > OK, for 1/5, 2/5, 3/5, and 5/5;
> > 
> > Acked-by: Masami Hiramatsu 
> > 
> > And could you make v4 series including all patches? (Not only updates)
> 
> So, to make progress I processed these:
> 
> [acme@jouet linux]$ git log --oneline -3
> eb55608340b7 perf probe: Generalize probe event file open routine
> 859d718fac06 trace/kprobes: Allow return probes with offsets and absolute 
> addresses
> a10489121c81 kretprobes: Ensure probe location is at function entry
> [acme@jouet linux]$
> 
> Waiting for Naveen to react to these last minute considerations from
> Masami and for the Ack from the PPC guys about "[PATCH v2 2/5] powerpc:
> kretprobes: override default function entry offset".

Arnaldo,
I am posting the remaining three patches in this series. These three
patches are on top of the above 3 patches you have processed and the
other powerpc kretprobes patch (v2 2/5).

Masami,
Kindly review and let me know if this is fine.


Thanks,
Naveen

---
Naveen N. Rao (3):
  perf: probe: factor out the ftrace README scanning
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: powerpc: choose local entry point with kretprobes

 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++--
 tools/perf/util/probe-event.c   | 12 ++---
 tools/perf/util/probe-file.c| 77 -
 tools/perf/util/probe-file.h|  1 +
 4 files changed, 56 insertions(+), 44 deletions(-)

-- 
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 <naveen.n@linux.vnet.ibm.com>
---
 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



[PATCH v4 3/3] perf: powerpc: choose local entry point with kretprobes

2017-03-02 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..cc7c2697c036 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "probe-file.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,11 +80,12 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe && 
!kretprobe_offset_is_supported())
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.1



[PATCH v4 3/3] perf: powerpc: choose local entry point with kretprobes

2017-03-02 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..cc7c2697c036 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -10,6 +10,7 @@
 #include "symbol.h"
 #include "map.h"
 #include "probe-event.h"
+#include "probe-file.h"
 
 #ifdef HAVE_LIBELF_SUPPORT
 bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
@@ -79,11 +80,12 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe && 
!kretprobe_offset_is_supported())
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.1



Re: [PATCH v3 1/2] perf: probe: generalize probe event file open routine

2017-03-01 Thread Naveen N. Rao
On 2017/02/25 01:46AM, Masami Hiramatsu wrote:
> On Thu, 23 Feb 2017 17:07:23 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> 
> > ...into a generic function for opening trace files.
> 
> Even if it repeats subject, please write complete description...

Agh, ok sure. I will try to curb my urge to not type too much :)

> 
> Patch itself is OK to me.

Thanks for the review on this series!

- Naveen



Re: [PATCH v3 1/2] perf: probe: generalize probe event file open routine

2017-03-01 Thread Naveen N. Rao
On 2017/02/25 01:46AM, Masami Hiramatsu wrote:
> On Thu, 23 Feb 2017 17:07:23 +0530
> "Naveen N. Rao"  wrote:
> 
> > ...into a generic function for opening trace files.
> 
> Even if it repeats subject, please write complete description...

Agh, ok sure. I will try to curb my urge to not type too much :)

> 
> Patch itself is OK to me.

Thanks for the review on this series!

- Naveen



Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-01 Thread Naveen N. Rao
On 2017/02/25 08:55AM, Masami Hiramatsu wrote:
> On Fri, 24 Feb 2017 17:11:03 -0300
> Arnaldo Carvalho de Melo <a...@kernel.org> wrote:
> 
> > Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:
> > > On Fri, 24 Feb 2017 00:46:08 +0530
> > > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> > > > Thanks. I hope that's an Ack for this patchset?
> > > 
> > > OK, for 1/5, 2/5, 3/5, and 5/5;
> > > 
> > > Acked-by: Masami Hiramatsu <mhira...@kernel.org>
> > > 
> > > And could you make v4 series including all patches? (Not only updates)
> > 
> > So, to make progress I processed these:
> > 
> > [acme@jouet linux]$ git log --oneline -3
> > eb55608340b7 perf probe: Generalize probe event file open routine
> > 859d718fac06 trace/kprobes: Allow return probes with offsets and absolute 
> > addresses
> > a10489121c81 kretprobes: Ensure probe location is at function entry
> > [acme@jouet linux]$
> > 
> > Waiting for Naveen to react to these last minute considerations from
> > Masami and for the Ack from the PPC guys about "[PATCH v2 2/5] powerpc:
> > kretprobes: override default function entry offset".

Thanks Arnaldo!
Sorry, couldn't get to this sooner as I was off for a day...
I see that you've picked up 3 of the patches and Ananth/Michael have 
acked the powerpc patch.

I will post the remaining ones tonight/tomorrow.

> 
> Thanks Arnaldo!!
> 
> Naveen, please update your ppc and perf patches and send it to Arnaldo.
> I'm happy to review it.

Sure thanks, I'll work on those tonight/tomorrow.


- Naveen



Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-03-01 Thread Naveen N. Rao
On 2017/02/25 08:55AM, Masami Hiramatsu wrote:
> On Fri, 24 Feb 2017 17:11:03 -0300
> Arnaldo Carvalho de Melo  wrote:
> 
> > Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:
> > > On Fri, 24 Feb 2017 00:46:08 +0530
> > > "Naveen N. Rao"  wrote:
> > > > Thanks. I hope that's an Ack for this patchset?
> > > 
> > > OK, for 1/5, 2/5, 3/5, and 5/5;
> > > 
> > > Acked-by: Masami Hiramatsu 
> > > 
> > > And could you make v4 series including all patches? (Not only updates)
> > 
> > So, to make progress I processed these:
> > 
> > [acme@jouet linux]$ git log --oneline -3
> > eb55608340b7 perf probe: Generalize probe event file open routine
> > 859d718fac06 trace/kprobes: Allow return probes with offsets and absolute 
> > addresses
> > a10489121c81 kretprobes: Ensure probe location is at function entry
> > [acme@jouet linux]$
> > 
> > Waiting for Naveen to react to these last minute considerations from
> > Masami and for the Ack from the PPC guys about "[PATCH v2 2/5] powerpc:
> > kretprobes: override default function entry offset".

Thanks Arnaldo!
Sorry, couldn't get to this sooner as I was off for a day...
I see that you've picked up 3 of the patches and Ananth/Michael have 
acked the powerpc patch.

I will post the remaining ones tonight/tomorrow.

> 
> Thanks Arnaldo!!
> 
> Naveen, please update your ppc and perf patches and send it to Arnaldo.
> I'm happy to review it.

Sure thanks, I'll work on those tonight/tomorrow.


- Naveen



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

2017-03-01 Thread Naveen N. Rao
On 2017/02/25 02:12AM, Masami Hiramatsu wrote:
> On Thu, 23 Feb 2017 17:07:24 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> 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.
> > 
> > Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> > ---
> >  tools/perf/util/probe-event.c | 49 
> > ---
> >  tools/perf/util/probe-event.h |  2 ++
> >  2 files changed, 44 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 35f5b7b7715c..dd6b9ce0eef3 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -737,6 +737,43 @@ post_process_module_probe_trace_events(struct 
> > probe_trace_event *tevs,
> > return ret;
> >  }
> >  
> > +bool is_kretprobe_offset_supported(void)
> > +{
> > +   FILE *fp;
> > +   char *buf = NULL;
> > +   size_t len = 0;
> > +   bool target_line = false;
> > +   static int supported = -1;
> > +   int fd;
> > +
> > +   if (supported >= 0)
> > +   return !!supported;
> > +
> > +   fd = open_trace_file("README", false);
> > +   if (fd < 0)
> > +   return false;
> > +
> > +   fp = fdopen(fd, "r");
> > +   if (!fp) {
> > +   close(fd);
> > +   return false;
> > +   }
> > +
> > +   while (getline(, , fp) > 0) {
> > +   target_line = !!strstr(buf, "place (kretprobe): ");
> > +   if (!target_line)
> > +   continue;
> > +   supported = 1;
> > +   }
> > +   if (supported == -1)
> > +   supported = 0;
> > +
> > +   fclose(fp);
> > +   free(buf);
> > +
> > +   return !!supported;
> > +}
> 
> Hmm, I think you can do more than that. 
> Can you reuse probe_type_is_available() to scan README?
> I think we can have something like scan_ftrace_readme() in probe-file.c
> to scan all the options and cache the results. 
> 
> probe_type_is_available() and kreprobe_offset_is_available()
> just returns cached result or scan it in first call.(I would like to
> ask you to do it in probe-file.c too)

Ok sure, that makes sense. I see that we only ever care about support 
for hex type, so I will add a separate routine to only look for that and 
the newly added kretprobe offset support.

- Naveen



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

2017-03-01 Thread Naveen N. Rao
On 2017/02/25 02:12AM, Masami Hiramatsu wrote:
> On Thu, 23 Feb 2017 17:07:24 +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.
> > 
> > Signed-off-by: Naveen N. Rao 
> > ---
> >  tools/perf/util/probe-event.c | 49 
> > ---
> >  tools/perf/util/probe-event.h |  2 ++
> >  2 files changed, 44 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 35f5b7b7715c..dd6b9ce0eef3 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -737,6 +737,43 @@ post_process_module_probe_trace_events(struct 
> > probe_trace_event *tevs,
> > return ret;
> >  }
> >  
> > +bool is_kretprobe_offset_supported(void)
> > +{
> > +   FILE *fp;
> > +   char *buf = NULL;
> > +   size_t len = 0;
> > +   bool target_line = false;
> > +   static int supported = -1;
> > +   int fd;
> > +
> > +   if (supported >= 0)
> > +   return !!supported;
> > +
> > +   fd = open_trace_file("README", false);
> > +   if (fd < 0)
> > +   return false;
> > +
> > +   fp = fdopen(fd, "r");
> > +   if (!fp) {
> > +   close(fd);
> > +   return false;
> > +   }
> > +
> > +   while (getline(, , fp) > 0) {
> > +   target_line = !!strstr(buf, "place (kretprobe): ");
> > +   if (!target_line)
> > +   continue;
> > +   supported = 1;
> > +   }
> > +   if (supported == -1)
> > +   supported = 0;
> > +
> > +   fclose(fp);
> > +   free(buf);
> > +
> > +   return !!supported;
> > +}
> 
> Hmm, I think you can do more than that. 
> Can you reuse probe_type_is_available() to scan README?
> I think we can have something like scan_ftrace_readme() in probe-file.c
> to scan all the options and cache the results. 
> 
> probe_type_is_available() and kreprobe_offset_is_available()
> just returns cached result or scan it in first call.(I would like to
> ask you to do it in probe-file.c too)

Ok sure, that makes sense. I see that we only ever care about support 
for hex type, so I will add a separate routine to only look for that and 
the newly added kretprobe offset support.

- Naveen



Re: [PATCH v2 3.5/5] trace/kprobes: Add back warning about offset in return probes

2017-03-01 Thread Naveen N. Rao
On 2017/02/27 11:52AM, Steven Rostedt (VMware) wrote:
> Let's not remove the warning about offsets and return probes when the
> offset is invalid.

Good point!

Thanks, Steve!

> 
> Signed-off-by: Steven Rostedt (VMware) <rost...@goodmis.org>

Acked-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>

> ---
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 3f4f788..f626235 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -695,6 +695,11 @@ static int create_trace_kprobe(int argc, char **argv)
>   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;
> + }
>   }
>   argc -= 2; argv += 2;
> 
> 



Re: [PATCH v2 3.5/5] trace/kprobes: Add back warning about offset in return probes

2017-03-01 Thread Naveen N. Rao
On 2017/02/27 11:52AM, Steven Rostedt (VMware) wrote:
> Let's not remove the warning about offsets and return probes when the
> offset is invalid.

Good point!

Thanks, Steve!

> 
> Signed-off-by: Steven Rostedt (VMware) 

Acked-by: Naveen N. Rao 

> ---
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 3f4f788..f626235 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -695,6 +695,11 @@ static int create_trace_kprobe(int argc, char **argv)
>   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;
> + }
>   }
>   argc -= 2; argv += 2;
> 
> 



Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-02-23 Thread Naveen N. Rao
On 2017/02/23 06:10PM, Masami Hiramatsu wrote:
> On Wed, 22 Feb 2017 19:23:40 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> 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.
> > 
> > Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
> > ---
> >  tools/perf/util/probe-event.c | 47 
> > ---
> >  tools/perf/util/probe-event.h |  2 ++
> >  2 files changed, 42 insertions(+), 7 deletions(-)
> > 

[snip]

> 
> Could you reuse (refactoring) probe_type_is_available() in probe-file.c to 
> share
> opening README file?

Done. I've sent patches to do that, please review.

> 
> Others looks good to me :)

Thanks. I hope that's an Ack for this patchset?

If so, and if Ingo/Michael agree, would it be ok to take the kernel bits 
through the powerpc tree like we did for kprobe_exceptions_notify() 
cleanup?


Regards,
Naveen



Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-02-23 Thread Naveen N. Rao
On 2017/02/23 06:10PM, Masami Hiramatsu wrote:
> On Wed, 22 Feb 2017 19:23:40 +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.
> > 
> > Signed-off-by: Naveen N. Rao 
> > ---
> >  tools/perf/util/probe-event.c | 47 
> > ---
> >  tools/perf/util/probe-event.h |  2 ++
> >  2 files changed, 42 insertions(+), 7 deletions(-)
> > 

[snip]

> 
> Could you reuse (refactoring) probe_type_is_available() in probe-file.c to 
> share
> opening README file?

Done. I've sent patches to do that, please review.

> 
> Others looks good to me :)

Thanks. I hope that's an Ack for this patchset?

If so, and if Ingo/Michael agree, would it be ok to take the kernel bits 
through the powerpc tree like we did for kprobe_exceptions_notify() 
cleanup?


Regards,
Naveen



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

2017-02-23 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 <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/util/probe-event.c | 49 ---
 tools/perf/util/probe-event.h |  2 ++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 35f5b7b7715c..dd6b9ce0eef3 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -737,6 +737,43 @@ post_process_module_probe_trace_events(struct 
probe_trace_event *tevs,
return ret;
 }
 
+bool is_kretprobe_offset_supported(void)
+{
+   FILE *fp;
+   char *buf = NULL;
+   size_t len = 0;
+   bool target_line = false;
+   static int supported = -1;
+   int fd;
+
+   if (supported >= 0)
+   return !!supported;
+
+   fd = open_trace_file("README", false);
+   if (fd < 0)
+   return false;
+
+   fp = fdopen(fd, "r");
+   if (!fp) {
+   close(fd);
+   return false;
+   }
+
+   while (getline(, , fp) > 0) {
+   target_line = !!strstr(buf, "place (kretprobe): ");
+   if (!target_line)
+   continue;
+   supported = 1;
+   }
+   if (supported == -1)
+   supported = 0;
+
+   fclose(fp);
+   free(buf);
+
+   return !!supported;
+}
+
 static int
 post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
   int ntevs)
@@ -757,7 +794,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 && !is_kretprobe_offset_supported())
continue;
/* If we found a wrong one, mark it by NULL symbol */
if (kprobe_warn_out_range(tevs[i].point.symbol,
@@ -1528,11 +1567,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 +2875,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 || is_kretprobe_offset_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-event.h b/tools/perf/util/probe-event.h
index 5d4e94061402..449d4f311355 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -135,6 +135,8 @@ bool perf_probe_with_var(struct perf_probe_event *pev);
 /* Check the perf_probe_event needs debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
 
+bool is_kretprobe_offset_supported(void);
+
 /* Release event contents */
 void clear_perf_probe_event(struct perf_probe_event *pev);
 void clear_probe_trace_event(struct probe_trace_event *tev);
-- 
2.11.1



[PATCH v3 1/2] perf: probe: generalize probe event file open routine

2017-02-23 Thread Naveen N. Rao
...into a generic function for opening trace files.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/util/probe-file.c | 20 +++-
 tools/perf/util/probe-file.h |  1 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b64731f65..1a62daceb028 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -70,7 +70,7 @@ static void print_both_open_warning(int kerr, int uerr)
}
 }
 
-static int open_probe_events(const char *trace_file, bool readwrite)
+int open_trace_file(const char *trace_file, bool readwrite)
 {
char buf[PATH_MAX];
int ret;
@@ -92,12 +92,12 @@ static int open_probe_events(const char *trace_file, bool 
readwrite)
 
 static int open_kprobe_events(bool readwrite)
 {
-   return open_probe_events("kprobe_events", readwrite);
+   return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
-   return open_probe_events("uprobe_events", readwrite);
+   return open_trace_file("uprobe_events", readwrite);
 }
 
 int probe_file__open(int flag)
@@ -899,6 +899,7 @@ bool probe_type_is_available(enum probe_type type)
size_t len = 0;
bool target_line = false;
bool ret = probe_type_table[type].avail;
+   int fd;
 
if (type >= PROBE_TYPE_END)
return false;
@@ -906,14 +907,16 @@ bool probe_type_is_available(enum probe_type type)
if (ret || probe_type_table[type].checked)
return ret;
 
-   if (asprintf(, "%s/README", tracing_path) < 0)
+   fd = open_trace_file("README", false);
+   if (fd < 0)
return ret;
 
-   fp = fopen(buf, "r");
-   if (!fp)
-   goto end;
+   fp = fdopen(fd, "r");
+   if (!fp) {
+   close(fd);
+   return ret;
+   }
 
-   zfree();
while (getline(, , fp) > 0 && !ret) {
if (!target_line) {
target_line = !!strstr(buf, " type: ");
@@ -928,7 +931,6 @@ bool probe_type_is_available(enum probe_type type)
probe_type_table[type].avail = ret;
 
fclose(fp);
-end:
free(buf);
 
return ret;
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index eba44c3e9dca..a17a82eff8a0 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -35,6 +35,7 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
+int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);
-- 
2.11.1



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

2017-02-23 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 | 49 ---
 tools/perf/util/probe-event.h |  2 ++
 2 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 35f5b7b7715c..dd6b9ce0eef3 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -737,6 +737,43 @@ post_process_module_probe_trace_events(struct 
probe_trace_event *tevs,
return ret;
 }
 
+bool is_kretprobe_offset_supported(void)
+{
+   FILE *fp;
+   char *buf = NULL;
+   size_t len = 0;
+   bool target_line = false;
+   static int supported = -1;
+   int fd;
+
+   if (supported >= 0)
+   return !!supported;
+
+   fd = open_trace_file("README", false);
+   if (fd < 0)
+   return false;
+
+   fp = fdopen(fd, "r");
+   if (!fp) {
+   close(fd);
+   return false;
+   }
+
+   while (getline(, , fp) > 0) {
+   target_line = !!strstr(buf, "place (kretprobe): ");
+   if (!target_line)
+   continue;
+   supported = 1;
+   }
+   if (supported == -1)
+   supported = 0;
+
+   fclose(fp);
+   free(buf);
+
+   return !!supported;
+}
+
 static int
 post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
   int ntevs)
@@ -757,7 +794,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 && !is_kretprobe_offset_supported())
continue;
/* If we found a wrong one, mark it by NULL symbol */
if (kprobe_warn_out_range(tevs[i].point.symbol,
@@ -1528,11 +1567,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 +2875,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 || is_kretprobe_offset_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-event.h b/tools/perf/util/probe-event.h
index 5d4e94061402..449d4f311355 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -135,6 +135,8 @@ bool perf_probe_with_var(struct perf_probe_event *pev);
 /* Check the perf_probe_event needs debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
 
+bool is_kretprobe_offset_supported(void);
+
 /* Release event contents */
 void clear_perf_probe_event(struct perf_probe_event *pev);
 void clear_probe_trace_event(struct probe_trace_event *tev);
-- 
2.11.1



[PATCH v3 1/2] perf: probe: generalize probe event file open routine

2017-02-23 Thread Naveen N. Rao
...into a generic function for opening trace files.

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

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b64731f65..1a62daceb028 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c
@@ -70,7 +70,7 @@ static void print_both_open_warning(int kerr, int uerr)
}
 }
 
-static int open_probe_events(const char *trace_file, bool readwrite)
+int open_trace_file(const char *trace_file, bool readwrite)
 {
char buf[PATH_MAX];
int ret;
@@ -92,12 +92,12 @@ static int open_probe_events(const char *trace_file, bool 
readwrite)
 
 static int open_kprobe_events(bool readwrite)
 {
-   return open_probe_events("kprobe_events", readwrite);
+   return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
-   return open_probe_events("uprobe_events", readwrite);
+   return open_trace_file("uprobe_events", readwrite);
 }
 
 int probe_file__open(int flag)
@@ -899,6 +899,7 @@ bool probe_type_is_available(enum probe_type type)
size_t len = 0;
bool target_line = false;
bool ret = probe_type_table[type].avail;
+   int fd;
 
if (type >= PROBE_TYPE_END)
return false;
@@ -906,14 +907,16 @@ bool probe_type_is_available(enum probe_type type)
if (ret || probe_type_table[type].checked)
return ret;
 
-   if (asprintf(, "%s/README", tracing_path) < 0)
+   fd = open_trace_file("README", false);
+   if (fd < 0)
return ret;
 
-   fp = fopen(buf, "r");
-   if (!fp)
-   goto end;
+   fp = fdopen(fd, "r");
+   if (!fp) {
+   close(fd);
+   return ret;
+   }
 
-   zfree();
while (getline(, , fp) > 0 && !ret) {
if (!target_line) {
target_line = !!strstr(buf, " type: ");
@@ -928,7 +931,6 @@ bool probe_type_is_available(enum probe_type type)
probe_type_table[type].avail = ret;
 
fclose(fp);
-end:
free(buf);
 
return ret;
diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index eba44c3e9dca..a17a82eff8a0 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h
@@ -35,6 +35,7 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
+int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);
-- 
2.11.1



[PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-02-22 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 <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/util/probe-event.c | 47 ---
 tools/perf/util/probe-event.h |  2 ++
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 35f5b7b7715c..f6bc61c47271 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -737,6 +737,41 @@ post_process_module_probe_trace_events(struct 
probe_trace_event *tevs,
return ret;
 }
 
+bool is_kretprobe_offset_supported(void)
+{
+   FILE *fp;
+   char *buf = NULL;
+   size_t len = 0;
+   bool target_line = false;
+   static int supported = -1;
+
+   if (supported >= 0)
+   return !!supported;
+
+   if (asprintf(, "%s/README", tracing_path) < 0)
+   return false;
+
+   fp = fopen(buf, "r");
+   if (!fp)
+   goto end;
+
+   zfree();
+   while (getline(, , fp) > 0) {
+   target_line = !!strstr(buf, "place (kretprobe): ");
+   if (!target_line)
+   continue;
+   supported = 1;
+   }
+   if (supported == -1)
+   supported = 0;
+
+   fclose(fp);
+end:
+   free(buf);
+
+   return !!supported;
+}
+
 static int
 post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
   int ntevs)
@@ -757,7 +792,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 && !is_kretprobe_offset_supported())
continue;
/* If we found a wrong one, mark it by NULL symbol */
if (kprobe_warn_out_range(tevs[i].point.symbol,
@@ -1528,11 +1565,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 +2873,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 || is_kretprobe_offset_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-event.h b/tools/perf/util/probe-event.h
index 5d4e94061402..449d4f311355 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -135,6 +135,8 @@ bool perf_probe_with_var(struct perf_probe_event *pev);
 /* Check the perf_probe_event needs debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
 
+bool is_kretprobe_offset_supported(void);
+
 /* Release event contents */
 void clear_perf_probe_event(struct perf_probe_event *pev);
 void clear_probe_trace_event(struct probe_trace_event *tev);
-- 
2.11.0



[PATCH v2 3/5] trace/kprobes: allow return probes with offsets and absolute addresses

2017-02-22 Thread Naveen N. Rao
Since the kernel includes many non-global functions with same names, we
will need to use offsets from other symbols (typically _text/_stext) or
absolute addresses to place return probes on specific functions. Also,
the core register_kretprobe() API never forbid use of offsets or
absolute addresses with kretprobes.

Allow its use with the trace infrastructure. To distinguish kernels that
support this, update ftrace README to explicitly call this out.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 kernel/trace/trace.c| 1 +
 kernel/trace/trace_kprobe.c | 8 
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d7449783987a..ababe56b3e8f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4362,6 +4362,7 @@ static const char readme_msg[] =
"\t   -:[/]\n"
 #ifdef CONFIG_KPROBE_EVENT
"\tplace: [:][+]|\n"
+  "place (kretprobe): [:][+]|\n"
 #endif
 #ifdef CONFIG_UPROBE_EVENT
"\tplace: :\n"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7ad9e53ad174..2768cb60ebd7 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -679,10 +679,6 @@ static int create_trace_kprobe(int argc, char **argv)
return -EINVAL;
}
if (isdigit(argv[1][0])) {
-   if (is_return) {
-   pr_info("Return probe point must be a symbol.\n");
-   return -EINVAL;
-   }
/* an address specified */
ret = kstrtoul([1][0], 0, (unsigned long *));
if (ret) {
@@ -698,10 +694,6 @@ static int create_trace_kprobe(int argc, char **argv)
pr_info("Failed to parse symbol.\n");
return ret;
}
-   if (offset && is_return) {
-   pr_info("Return probe must be used without offset.\n");
-   return -EINVAL;
-   }
}
argc -= 2; argv += 2;
 
-- 
2.11.0



[PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

2017-02-22 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 | 47 ---
 tools/perf/util/probe-event.h |  2 ++
 2 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 35f5b7b7715c..f6bc61c47271 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -737,6 +737,41 @@ post_process_module_probe_trace_events(struct 
probe_trace_event *tevs,
return ret;
 }
 
+bool is_kretprobe_offset_supported(void)
+{
+   FILE *fp;
+   char *buf = NULL;
+   size_t len = 0;
+   bool target_line = false;
+   static int supported = -1;
+
+   if (supported >= 0)
+   return !!supported;
+
+   if (asprintf(, "%s/README", tracing_path) < 0)
+   return false;
+
+   fp = fopen(buf, "r");
+   if (!fp)
+   goto end;
+
+   zfree();
+   while (getline(, , fp) > 0) {
+   target_line = !!strstr(buf, "place (kretprobe): ");
+   if (!target_line)
+   continue;
+   supported = 1;
+   }
+   if (supported == -1)
+   supported = 0;
+
+   fclose(fp);
+end:
+   free(buf);
+
+   return !!supported;
+}
+
 static int
 post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
   int ntevs)
@@ -757,7 +792,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 && !is_kretprobe_offset_supported())
continue;
/* If we found a wrong one, mark it by NULL symbol */
if (kprobe_warn_out_range(tevs[i].point.symbol,
@@ -1528,11 +1565,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 +2873,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 || is_kretprobe_offset_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-event.h b/tools/perf/util/probe-event.h
index 5d4e94061402..449d4f311355 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -135,6 +135,8 @@ bool perf_probe_with_var(struct perf_probe_event *pev);
 /* Check the perf_probe_event needs debuginfo */
 bool perf_probe_event_need_dwarf(struct perf_probe_event *pev);
 
+bool is_kretprobe_offset_supported(void);
+
 /* Release event contents */
 void clear_perf_probe_event(struct perf_probe_event *pev);
 void clear_probe_trace_event(struct probe_trace_event *tev);
-- 
2.11.0



[PATCH v2 3/5] trace/kprobes: allow return probes with offsets and absolute addresses

2017-02-22 Thread Naveen N. Rao
Since the kernel includes many non-global functions with same names, we
will need to use offsets from other symbols (typically _text/_stext) or
absolute addresses to place return probes on specific functions. Also,
the core register_kretprobe() API never forbid use of offsets or
absolute addresses with kretprobes.

Allow its use with the trace infrastructure. To distinguish kernels that
support this, update ftrace README to explicitly call this out.

Signed-off-by: Naveen N. Rao 
---
 kernel/trace/trace.c| 1 +
 kernel/trace/trace_kprobe.c | 8 
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d7449783987a..ababe56b3e8f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4362,6 +4362,7 @@ static const char readme_msg[] =
"\t   -:[/]\n"
 #ifdef CONFIG_KPROBE_EVENT
"\tplace: [:][+]|\n"
+  "place (kretprobe): [:][+]|\n"
 #endif
 #ifdef CONFIG_UPROBE_EVENT
"\tplace: :\n"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7ad9e53ad174..2768cb60ebd7 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -679,10 +679,6 @@ static int create_trace_kprobe(int argc, char **argv)
return -EINVAL;
}
if (isdigit(argv[1][0])) {
-   if (is_return) {
-   pr_info("Return probe point must be a symbol.\n");
-   return -EINVAL;
-   }
/* an address specified */
ret = kstrtoul([1][0], 0, (unsigned long *));
if (ret) {
@@ -698,10 +694,6 @@ static int create_trace_kprobe(int argc, char **argv)
pr_info("Failed to parse symbol.\n");
return ret;
}
-   if (offset && is_return) {
-   pr_info("Return probe must be used without offset.\n");
-   return -EINVAL;
-   }
}
argc -= 2; argv += 2;
 
-- 
2.11.0



[PATCH v2 5/5] perf: powerpc: choose local entry point with kretprobes

2017-02-22 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..73dbdc83286c 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -79,11 +79,12 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe && 
!is_kretprobe_offset_supported())
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.0



[PATCH v2 5/5] perf: powerpc: choose local entry point with kretprobes

2017-02-22 Thread Naveen N. Rao
perf now uses an offset from _text/_stext for kretprobes if the kernel
supports it, rather than the actual function name. As such, let's choose
the LEP for powerpc ABIv2 so as to ensure the probe gets hit. Do it only
if the kernel supports specifying offsets with kretprobes.

Signed-off-by: Naveen N. Rao 
---
 tools/perf/arch/powerpc/util/sym-handling.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/sym-handling.c 
b/tools/perf/arch/powerpc/util/sym-handling.c
index 1030a6e504bb..73dbdc83286c 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c
@@ -79,11 +79,12 @@ void arch__fix_tev_from_maps(struct perf_probe_event *pev,
 * However, if the user specifies an offset, we fall back to using the
 * GEP since all userspace applications (objdump/readelf) show function
 * disassembly with offsets from the GEP.
-*
-* In addition, we shouldn't specify an offset for kretprobes.
 */
-   if (pev->point.offset || (!pev->uprobes && pev->point.retprobe) ||
-   !map || !sym)
+   if (pev->point.offset || !map || !sym)
+   return;
+
+   /* For kretprobes, add an offset only if the kernel supports it */
+   if (!pev->uprobes && pev->point.retprobe && 
!is_kretprobe_offset_supported())
return;
 
lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);
-- 
2.11.0



[PATCH v2 2/5] powerpc: kretprobes: override default function entry offset

2017-02-22 Thread Naveen N. Rao
With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kprobes.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 735ff3d3f77d..e37b76b8b6b2 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -131,6 +131,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
kcb->kprobe_saved_msr = regs->msr;
 }
 
+bool arch_function_offset_within_entry(unsigned long offset)
+{
+#ifdef PPC64_ELF_ABI_v2
+   return offset <= 8;
+#else
+   return !offset;
+#endif
+}
+
 void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
  struct pt_regs *regs)
 {
-- 
2.11.0



[PATCH v2 2/5] powerpc: kretprobes: override default function entry offset

2017-02-22 Thread Naveen N. Rao
With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/kprobes.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 735ff3d3f77d..e37b76b8b6b2 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -131,6 +131,15 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
kcb->kprobe_saved_msr = regs->msr;
 }
 
+bool arch_function_offset_within_entry(unsigned long offset)
+{
+#ifdef PPC64_ELF_ABI_v2
+   return offset <= 8;
+#else
+   return !offset;
+#endif
+}
+
 void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,
  struct pt_regs *regs)
 {
-- 
2.11.0



[PATCH v2 1/5] kretprobes: ensure probe location is at function entry

2017-02-22 Thread Naveen N. Rao
kretprobes can be registered by specifying an absolute address or by
specifying offset to a symbol. However, we need to ensure this falls at
function entry so as to be able to determine the return address.

Validate the same during kretprobe registration. By default, there
should not be any offset from a function entry, as determined through a
kallsyms_lookup(). Introduce arch_function_offset_within_entry() as a
way for architectures to override this.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 13 +
 2 files changed, 14 insertions(+)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 16ddfb8b304a..862f87adcbb4 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -266,6 +266,7 @@ extern int arch_init_kprobes(void);
 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 within_kprobe_blacklist(unsigned long addr);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ebb4dadca66b..524766563896 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1869,12 +1869,25 @@ static int pre_handler_kretprobe(struct kprobe *p, 
struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
 
+bool __weak arch_function_offset_within_entry(unsigned long offset)
+{
+   return !offset;
+}
+
 int register_kretprobe(struct kretprobe *rp)
 {
int ret = 0;
struct kretprobe_instance *inst;
int i;
void *addr;
+   unsigned long offset;
+
+   addr = kprobe_addr(>kp);
+   if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, ))
+   return -EINVAL;
+
+   if (!arch_function_offset_within_entry(offset))
+   return -EINVAL;
 
if (kretprobe_blacklist_size) {
addr = kprobe_addr(>kp);
-- 
2.11.0



[PATCH v2 0/5] kretprobe fixes

2017-02-22 Thread Naveen N. Rao
I'm including all patches (generic and powerpc changes) in
this series as suggested by Masami.

v1 patches:
https://marc.info/?l=linux-kernel=148718276424380
https://marc.info/?l=linux-kernel=148723314105453=2

Patches 1 and 2 are the same as v1.
Patch 3 is updated to include a line in ftrace README.
Patch 4 is new.
Patch 5 is updated to consider ftrace README.


Thanks,
Naveen

Naveen N. Rao (5):
  kretprobes: ensure probe location is at function entry
  powerpc: kretprobes: override default function entry offset
  trace/kprobes: allow return probes with offsets and absolute addresses
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: powerpc: choose local entry point with kretprobes

 arch/powerpc/kernel/kprobes.c   |  9 ++
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 13 
 kernel/trace/trace.c|  1 +
 kernel/trace/trace_kprobe.c |  8 -
 tools/perf/arch/powerpc/util/sym-handling.c |  9 +++---
 tools/perf/util/probe-event.c   | 47 -
 tools/perf/util/probe-event.h   |  2 ++
 8 files changed, 71 insertions(+), 19 deletions(-)

-- 
2.11.0



[PATCH v2 0/5] kretprobe fixes

2017-02-22 Thread Naveen N. Rao
I'm including all patches (generic and powerpc changes) in
this series as suggested by Masami.

v1 patches:
https://marc.info/?l=linux-kernel=148718276424380
https://marc.info/?l=linux-kernel=148723314105453=2

Patches 1 and 2 are the same as v1.
Patch 3 is updated to include a line in ftrace README.
Patch 4 is new.
Patch 5 is updated to consider ftrace README.


Thanks,
Naveen

Naveen N. Rao (5):
  kretprobes: ensure probe location is at function entry
  powerpc: kretprobes: override default function entry offset
  trace/kprobes: allow return probes with offsets and absolute addresses
  perf: kretprobes: offset from reloc_sym if kernel supports it
  perf: powerpc: choose local entry point with kretprobes

 arch/powerpc/kernel/kprobes.c   |  9 ++
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 13 
 kernel/trace/trace.c|  1 +
 kernel/trace/trace_kprobe.c |  8 -
 tools/perf/arch/powerpc/util/sym-handling.c |  9 +++---
 tools/perf/util/probe-event.c   | 47 -
 tools/perf/util/probe-event.h   |  2 ++
 8 files changed, 71 insertions(+), 19 deletions(-)

-- 
2.11.0



[PATCH v2 1/5] kretprobes: ensure probe location is at function entry

2017-02-22 Thread Naveen N. Rao
kretprobes can be registered by specifying an absolute address or by
specifying offset to a symbol. However, we need to ensure this falls at
function entry so as to be able to determine the return address.

Validate the same during kretprobe registration. By default, there
should not be any offset from a function entry, as determined through a
kallsyms_lookup(). Introduce arch_function_offset_within_entry() as a
way for architectures to override this.

Signed-off-by: Naveen N. Rao 
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c| 13 +
 2 files changed, 14 insertions(+)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 16ddfb8b304a..862f87adcbb4 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -266,6 +266,7 @@ extern int arch_init_kprobes(void);
 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 within_kprobe_blacklist(unsigned long addr);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ebb4dadca66b..524766563896 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1869,12 +1869,25 @@ static int pre_handler_kretprobe(struct kprobe *p, 
struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
 
+bool __weak arch_function_offset_within_entry(unsigned long offset)
+{
+   return !offset;
+}
+
 int register_kretprobe(struct kretprobe *rp)
 {
int ret = 0;
struct kretprobe_instance *inst;
int i;
void *addr;
+   unsigned long offset;
+
+   addr = kprobe_addr(>kp);
+   if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, ))
+   return -EINVAL;
+
+   if (!arch_function_offset_within_entry(offset))
+   return -EINVAL;
 
if (kretprobe_blacklist_size) {
addr = kprobe_addr(>kp);
-- 
2.11.0



Re: [PATCH 0/2] powerpc: kretprobe updates

2017-02-22 Thread Naveen N. Rao
On 2017/02/21 10:07PM, Masami Hiramatsu wrote:
> On Mon, 20 Feb 2017 15:20:24 +0530
> "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> 
> > On 2017/02/19 01:42PM, Masami Hiramatsu wrote:
> > > On Fri, 17 Feb 2017 17:42:54 -0300
> > > Arnaldo Carvalho de Melo <a...@kernel.org> wrote:
> > > 
> > > > Em Fri, Feb 17, 2017 at 07:44:33PM +0900, Masami Hiramatsu escreveu:
> > > > > On Thu, 16 Feb 2017 13:47:37 +0530
> > > > > "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com> wrote:
> > > > > 
> > > > > > I am posting the powerpc bits in the same thread so as to keep these
> > > > > > changes together. I am not sure how this should be taken upstream as
> > > > > > there are atleast three different trees involved: one for the core
> > > > > > kprobes infrastructure, one for powerpc and one for perf.
> > > > 
> > > > > Hmm, could you make these (and other related) patches and
> > > > > other series in one series? Or wait for the other series
> > > > > are merged correctly.
> > > > 
> > > > Well, patches like these should be done in a way that the tooling parts
> > > > can deal with kernels with or without the kernel changes, so that older
> > > > tools work with new kernels and new tools work with older kernels.
> > > > 
> > > > "work" as in the previous behaviour is kept when a new tool deals with
> > > > an older kernel and an older tool would warn the user that what it needs
> > > > is not present in that kernel.
> > > > 
> > > > Is this the case? I just looked briefly at the patch commit logs.
> > > 
> > > Thanks Arnaldo,
> > > 
> > > Naveen, I think this one and your previous series are incompatible
> > > with older kernel. So those should be merged in one series and
> > > at least (1) update ftrace's README special file to show explicitly
> > > which can accept text+offset style for kretprobes, and 
> > 
> > Sure - do you mean Documentation/trace/kprobetrace.txt? And, do you want 
> > me to include kernel version where this changed?
> 
> No, I meant /sys/kernel/debug/tracing/README. For some reasons, perf
> probe already parse it in util/probe-file.c. 
> Please see commit 180b20616ce57e93eb692170c793be94c456b1e2 and 
> 864256255597aad86abcecbe6c53da8852ded15b

Got it.

Thanks,
Naveen



Re: [PATCH 0/2] powerpc: kretprobe updates

2017-02-22 Thread Naveen N. Rao
On 2017/02/21 10:07PM, Masami Hiramatsu wrote:
> On Mon, 20 Feb 2017 15:20:24 +0530
> "Naveen N. Rao"  wrote:
> 
> > On 2017/02/19 01:42PM, Masami Hiramatsu wrote:
> > > On Fri, 17 Feb 2017 17:42:54 -0300
> > > Arnaldo Carvalho de Melo  wrote:
> > > 
> > > > Em Fri, Feb 17, 2017 at 07:44:33PM +0900, Masami Hiramatsu escreveu:
> > > > > On Thu, 16 Feb 2017 13:47:37 +0530
> > > > > "Naveen N. Rao"  wrote:
> > > > > 
> > > > > > I am posting the powerpc bits in the same thread so as to keep these
> > > > > > changes together. I am not sure how this should be taken upstream as
> > > > > > there are atleast three different trees involved: one for the core
> > > > > > kprobes infrastructure, one for powerpc and one for perf.
> > > > 
> > > > > Hmm, could you make these (and other related) patches and
> > > > > other series in one series? Or wait for the other series
> > > > > are merged correctly.
> > > > 
> > > > Well, patches like these should be done in a way that the tooling parts
> > > > can deal with kernels with or without the kernel changes, so that older
> > > > tools work with new kernels and new tools work with older kernels.
> > > > 
> > > > "work" as in the previous behaviour is kept when a new tool deals with
> > > > an older kernel and an older tool would warn the user that what it needs
> > > > is not present in that kernel.
> > > > 
> > > > Is this the case? I just looked briefly at the patch commit logs.
> > > 
> > > Thanks Arnaldo,
> > > 
> > > Naveen, I think this one and your previous series are incompatible
> > > with older kernel. So those should be merged in one series and
> > > at least (1) update ftrace's README special file to show explicitly
> > > which can accept text+offset style for kretprobes, and 
> > 
> > Sure - do you mean Documentation/trace/kprobetrace.txt? And, do you want 
> > me to include kernel version where this changed?
> 
> No, I meant /sys/kernel/debug/tracing/README. For some reasons, perf
> probe already parse it in util/probe-file.c. 
> Please see commit 180b20616ce57e93eb692170c793be94c456b1e2 and 
> 864256255597aad86abcecbe6c53da8852ded15b

Got it.

Thanks,
Naveen



[PATCH v2 3/5] kprobes: Skip preparing optprobe if the probe is ftrace-based

2017-02-21 Thread Naveen N. Rao
From: Masami Hiramatsu <mhira...@kernel.org>

Skip preparing optprobe if the probe is ftrace-based, since anyway, it
must not be optimized (or already optimized by ftrace).

Tested-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
---
 kernel/kprobes.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index fbc7a70ff33e..7e93500dede7 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -708,13 +708,20 @@ static void kill_optimized_kprobe(struct kprobe *p)
arch_remove_optimized_kprobe(op);
 }
 
+static inline
+void __prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
+{
+   if (!kprobe_ftrace(p))
+   arch_prepare_optimized_kprobe(op, p);
+}
+
 /* Try to prepare optimized instructions */
 static void prepare_optimized_kprobe(struct kprobe *p)
 {
struct optimized_kprobe *op;
 
op = container_of(p, struct optimized_kprobe, kp);
-   arch_prepare_optimized_kprobe(op, p);
+   __prepare_optimized_kprobe(op, p);
 }
 
 /* Allocate new optimized_kprobe and try to prepare optimized instructions */
@@ -728,7 +735,7 @@ static struct kprobe *alloc_aggr_kprobe(struct kprobe *p)
 
INIT_LIST_HEAD(>list);
op->kp.addr = p->addr;
-   arch_prepare_optimized_kprobe(op, p);
+   __prepare_optimized_kprobe(op, p);
 
return >kp;
 }
-- 
2.11.0



[PATCH v2 3/5] kprobes: Skip preparing optprobe if the probe is ftrace-based

2017-02-21 Thread Naveen N. Rao
From: Masami Hiramatsu 

Skip preparing optprobe if the probe is ftrace-based, since anyway, it
must not be optimized (or already optimized by ftrace).

Tested-by: Naveen N. Rao 
Signed-off-by: Masami Hiramatsu 
---
 kernel/kprobes.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index fbc7a70ff33e..7e93500dede7 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -708,13 +708,20 @@ static void kill_optimized_kprobe(struct kprobe *p)
arch_remove_optimized_kprobe(op);
 }
 
+static inline
+void __prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
+{
+   if (!kprobe_ftrace(p))
+   arch_prepare_optimized_kprobe(op, p);
+}
+
 /* Try to prepare optimized instructions */
 static void prepare_optimized_kprobe(struct kprobe *p)
 {
struct optimized_kprobe *op;
 
op = container_of(p, struct optimized_kprobe, kp);
-   arch_prepare_optimized_kprobe(op, p);
+   __prepare_optimized_kprobe(op, p);
 }
 
 /* Allocate new optimized_kprobe and try to prepare optimized instructions */
@@ -728,7 +735,7 @@ static struct kprobe *alloc_aggr_kprobe(struct kprobe *p)
 
INIT_LIST_HEAD(>list);
op->kp.addr = p->addr;
-   arch_prepare_optimized_kprobe(op, p);
+   __prepare_optimized_kprobe(op, p);
 
return >kp;
 }
-- 
2.11.0



[PATCH v2 5/5] powerpc: kprobes: prefer ftrace when probing function entry

2017-02-21 Thread Naveen N. Rao
KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
eliminates the need for a trap, as well as the need to emulate or
single-step instructions.

Though OPTPROBES provides us with similar performance, we have limited
optprobes trampoline slots. As such, when asked to probe at a function
entry, default to using the ftrace infrastructure.

With:
# cd /sys/kernel/debug/tracing
# echo 'p _do_fork' > kprobe_events

before patch:
# cat ../kprobes/list
c00daf08  k  _do_fork+0x8[DISABLED]
c0044fc0  k  kretprobe_trampoline+0x0[OPTIMIZED]

and after patch:
# cat ../kprobes/list
c00d074c  k  _do_fork+0xc[DISABLED][FTRACE]
c00412b0  k  kretprobe_trampoline+0x0[OPTIMIZED]

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/kprobes.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index b78b274e1d6e..23d19678a56f 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -49,8 +49,21 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, 
unsigned int offset)
 #ifdef PPC64_ELF_ABI_v2
/* PPC64 ABIv2 needs local entry point */
addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
-   if (addr && !offset)
-   addr = (kprobe_opcode_t *)ppc_function_entry(addr);
+   if (addr && !offset) {
+#ifdef CONFIG_KPROBES_ON_FTRACE
+   unsigned long faddr;
+   /*
+* Per livepatch.h, ftrace location is always within the first
+* 16 bytes of a function on powerpc with -mprofile-kernel.
+*/
+   faddr = ftrace_location_range((unsigned long)addr,
+ (unsigned long)addr + 16);
+   if (faddr)
+   addr = (kprobe_opcode_t *)faddr;
+   else
+#endif
+   addr = (kprobe_opcode_t *)ppc_function_entry(addr);
+   }
 #elif defined(PPC64_ELF_ABI_v1)
/*
 * 64bit powerpc ABIv1 uses function descriptors:
-- 
2.11.0



[PATCH v2 5/5] powerpc: kprobes: prefer ftrace when probing function entry

2017-02-21 Thread Naveen N. Rao
KPROBES_ON_FTRACE avoids much of the overhead with regular kprobes as it
eliminates the need for a trap, as well as the need to emulate or
single-step instructions.

Though OPTPROBES provides us with similar performance, we have limited
optprobes trampoline slots. As such, when asked to probe at a function
entry, default to using the ftrace infrastructure.

With:
# cd /sys/kernel/debug/tracing
# echo 'p _do_fork' > kprobe_events

before patch:
# cat ../kprobes/list
c00daf08  k  _do_fork+0x8[DISABLED]
c0044fc0  k  kretprobe_trampoline+0x0[OPTIMIZED]

and after patch:
# cat ../kprobes/list
c00d074c  k  _do_fork+0xc[DISABLED][FTRACE]
c00412b0  k  kretprobe_trampoline+0x0[OPTIMIZED]

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/kprobes.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index b78b274e1d6e..23d19678a56f 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -49,8 +49,21 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, 
unsigned int offset)
 #ifdef PPC64_ELF_ABI_v2
/* PPC64 ABIv2 needs local entry point */
addr = (kprobe_opcode_t *)kallsyms_lookup_name(name);
-   if (addr && !offset)
-   addr = (kprobe_opcode_t *)ppc_function_entry(addr);
+   if (addr && !offset) {
+#ifdef CONFIG_KPROBES_ON_FTRACE
+   unsigned long faddr;
+   /*
+* Per livepatch.h, ftrace location is always within the first
+* 16 bytes of a function on powerpc with -mprofile-kernel.
+*/
+   faddr = ftrace_location_range((unsigned long)addr,
+ (unsigned long)addr + 16);
+   if (faddr)
+   addr = (kprobe_opcode_t *)faddr;
+   else
+#endif
+   addr = (kprobe_opcode_t *)ppc_function_entry(addr);
+   }
 #elif defined(PPC64_ELF_ABI_v1)
/*
 * 64bit powerpc ABIv1 uses function descriptors:
-- 
2.11.0



[PATCH v2 4/5] powerpc: kprobes: add support for KPROBES_ON_FTRACE

2017-02-21 Thread Naveen N. Rao
Allow kprobes to be placed on ftrace _mcount() call sites. This
optimization avoids the use of a trap, by riding on ftrace
infrastructure.

This depends on HAVE_DYNAMIC_FTRACE_WITH_REGS which depends on
MPROFILE_KERNEL, which is only currently enabled on powerpc64le with
newer toolchains.

Based on the x86 code by Masami.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 .../debug/kprobes-on-ftrace/arch-support.txt   |   2 +-
 arch/powerpc/Kconfig   |   1 +
 arch/powerpc/include/asm/kprobes.h |  10 ++
 arch/powerpc/kernel/Makefile   |   3 +
 arch/powerpc/kernel/kprobes-ftrace.c   | 104 +
 arch/powerpc/kernel/kprobes.c  |   8 +-
 6 files changed, 126 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/kprobes-ftrace.c

diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt 
b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
index 40f44d041fb4..930430c6aef6 100644
--- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
+++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
@@ -27,7 +27,7 @@
 |   nios2: | TODO |
 |openrisc: | TODO |
 |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  ok  |
 |s390: | TODO |
 |   score: | TODO |
 |  sh: | TODO |
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 5e7aaa9976e2..c2fdd895816c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -101,6 +101,7 @@ config PPC
select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && 
POWER7_CPU)
select HAVE_KPROBES
select HAVE_OPTPROBES if PPC64
+   select HAVE_KPROBES_ON_FTRACE
select HAVE_ARCH_KGDB
select HAVE_KRETPROBES
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/powerpc/include/asm/kprobes.h 
b/arch/powerpc/include/asm/kprobes.h
index ab5bd200bb48..a8a3c04ba248 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -100,6 +100,16 @@ extern int kprobe_exceptions_notify(struct notifier_block 
*self,
 extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
 extern int kprobe_handler(struct pt_regs *regs);
 extern int kprobe_post_handler(struct pt_regs *regs);
+#ifdef CONFIG_KPROBES_ON_FTRACE
+extern int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
+  struct kprobe_ctlblk *kcb);
+#else
+static inline int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
+ struct kprobe_ctlblk *kcb)
+{
+   return 0;
+}
+#endif
 #else
 static inline int kprobe_handler(struct pt_regs *regs) { return 0; }
 static inline int kprobe_post_handler(struct pt_regs *regs) { return 0; }
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a048b37b9b27..88b21427ccc7 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_BOOTX_TEXT)+= btext.o
 obj-$(CONFIG_SMP)  += smp.o
 obj-$(CONFIG_KPROBES)  += kprobes.o
 obj-$(CONFIG_OPTPROBES)+= optprobes.o optprobes_head.o
+obj-$(CONFIG_KPROBES_ON_FTRACE)+= kprobes-ftrace.o
 obj-$(CONFIG_UPROBES)  += uprobes.o
 obj-$(CONFIG_PPC_UDBG_16550)   += legacy_serial.o udbg_16550.o
 obj-$(CONFIG_STACKTRACE)   += stacktrace.o
@@ -154,6 +155,8 @@ GCOV_PROFILE_machine_kexec_32.o := n
 UBSAN_SANITIZE_machine_kexec_32.o := n
 GCOV_PROFILE_kprobes.o := n
 UBSAN_SANITIZE_kprobes.o := n
+GCOV_PROFILE_kprobes-ftrace.o := n
+UBSAN_SANITIZE_kprobes-ftrace.o := n
 UBSAN_SANITIZE_vdso.o := n
 
 extra-$(CONFIG_PPC_FPU)+= fpu.o
diff --git a/arch/powerpc/kernel/kprobes-ftrace.c 
b/arch/powerpc/kernel/kprobes-ftrace.c
new file mode 100644
index ..6c089d9757c9
--- /dev/null
+++ b/arch/powerpc/kernel/kprobes-ftrace.c
@@ -0,0 +1,104 @@
+/*
+ * Dynamic Ftrace based Kprobes Optimization
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) Hitachi Ltd., 2012
+ * Copyright 2016 Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
+ *   IBM Corporation
+ */
+#include 
+#include 

[PATCH v2 2/5] powerpc: ftrace: restore LR from pt_regs

2017-02-21 Thread Naveen N. Rao
Pass the real LR to the ftrace handler. This is needed for
KPROBES_ON_FTRACE for the pre handlers.

Also, with KPROBES_ON_FTRACE, the link register may be updated by the
pre handlers or by a registed kretprobe. Honor updated LR by restoring
it from pt_regs, rather than from the stack save area.

Live patch and function graph continue to work fine with this change.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/entry_64.S | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 8fd8718722a1..744b2f91444a 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1248,9 +1248,10 @@ _GLOBAL(ftrace_caller)
 
/* Get the _mcount() call site out of LR */
mflrr7
-   /* Save it as pt_regs->nip & pt_regs->link */
+   /* Save it as pt_regs->nip */
std r7, _NIP(r1)
-   std r7, _LINK(r1)
+   /* Save the read LR in pt_regs->link */
+   std r0, _LINK(r1)
 
/* Save callee's TOC in the ABI compliant location */
std r2, 24(r1)
@@ -1297,16 +1298,16 @@ ftrace_call:
REST_8GPRS(16,r1)
REST_8GPRS(24,r1)
 
+   /* Restore possibly modified LR */
+   ld  r0, _LINK(r1)
+   mtlrr0
+
/* Restore callee's TOC */
ld  r2, 24(r1)
 
/* Pop our stack frame */
addi r1, r1, SWITCH_FRAME_SIZE
 
-   /* Restore original LR for return to B */
-   ld  r0, LRSAVE(r1)
-   mtlrr0
-
 #ifdef CONFIG_LIVEPATCH
 /* Based on the cmpd above, if the NIP was altered handle livepatch */
bne-livepatch_handler
-- 
2.11.0



[PATCH v2 4/5] powerpc: kprobes: add support for KPROBES_ON_FTRACE

2017-02-21 Thread Naveen N. Rao
Allow kprobes to be placed on ftrace _mcount() call sites. This
optimization avoids the use of a trap, by riding on ftrace
infrastructure.

This depends on HAVE_DYNAMIC_FTRACE_WITH_REGS which depends on
MPROFILE_KERNEL, which is only currently enabled on powerpc64le with
newer toolchains.

Based on the x86 code by Masami.

Signed-off-by: Naveen N. Rao 
---
 .../debug/kprobes-on-ftrace/arch-support.txt   |   2 +-
 arch/powerpc/Kconfig   |   1 +
 arch/powerpc/include/asm/kprobes.h |  10 ++
 arch/powerpc/kernel/Makefile   |   3 +
 arch/powerpc/kernel/kprobes-ftrace.c   | 104 +
 arch/powerpc/kernel/kprobes.c  |   8 +-
 6 files changed, 126 insertions(+), 2 deletions(-)
 create mode 100644 arch/powerpc/kernel/kprobes-ftrace.c

diff --git a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt 
b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
index 40f44d041fb4..930430c6aef6 100644
--- a/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
+++ b/Documentation/features/debug/kprobes-on-ftrace/arch-support.txt
@@ -27,7 +27,7 @@
 |   nios2: | TODO |
 |openrisc: | TODO |
 |  parisc: | TODO |
-| powerpc: | TODO |
+| powerpc: |  ok  |
 |s390: | TODO |
 |   score: | TODO |
 |  sh: | TODO |
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 5e7aaa9976e2..c2fdd895816c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -101,6 +101,7 @@ config PPC
select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && 
POWER7_CPU)
select HAVE_KPROBES
select HAVE_OPTPROBES if PPC64
+   select HAVE_KPROBES_ON_FTRACE
select HAVE_ARCH_KGDB
select HAVE_KRETPROBES
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/powerpc/include/asm/kprobes.h 
b/arch/powerpc/include/asm/kprobes.h
index ab5bd200bb48..a8a3c04ba248 100644
--- a/arch/powerpc/include/asm/kprobes.h
+++ b/arch/powerpc/include/asm/kprobes.h
@@ -100,6 +100,16 @@ extern int kprobe_exceptions_notify(struct notifier_block 
*self,
 extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
 extern int kprobe_handler(struct pt_regs *regs);
 extern int kprobe_post_handler(struct pt_regs *regs);
+#ifdef CONFIG_KPROBES_ON_FTRACE
+extern int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
+  struct kprobe_ctlblk *kcb);
+#else
+static inline int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
+ struct kprobe_ctlblk *kcb)
+{
+   return 0;
+}
+#endif
 #else
 static inline int kprobe_handler(struct pt_regs *regs) { return 0; }
 static inline int kprobe_post_handler(struct pt_regs *regs) { return 0; }
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a048b37b9b27..88b21427ccc7 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_BOOTX_TEXT)+= btext.o
 obj-$(CONFIG_SMP)  += smp.o
 obj-$(CONFIG_KPROBES)  += kprobes.o
 obj-$(CONFIG_OPTPROBES)+= optprobes.o optprobes_head.o
+obj-$(CONFIG_KPROBES_ON_FTRACE)+= kprobes-ftrace.o
 obj-$(CONFIG_UPROBES)  += uprobes.o
 obj-$(CONFIG_PPC_UDBG_16550)   += legacy_serial.o udbg_16550.o
 obj-$(CONFIG_STACKTRACE)   += stacktrace.o
@@ -154,6 +155,8 @@ GCOV_PROFILE_machine_kexec_32.o := n
 UBSAN_SANITIZE_machine_kexec_32.o := n
 GCOV_PROFILE_kprobes.o := n
 UBSAN_SANITIZE_kprobes.o := n
+GCOV_PROFILE_kprobes-ftrace.o := n
+UBSAN_SANITIZE_kprobes-ftrace.o := n
 UBSAN_SANITIZE_vdso.o := n
 
 extra-$(CONFIG_PPC_FPU)+= fpu.o
diff --git a/arch/powerpc/kernel/kprobes-ftrace.c 
b/arch/powerpc/kernel/kprobes-ftrace.c
new file mode 100644
index ..6c089d9757c9
--- /dev/null
+++ b/arch/powerpc/kernel/kprobes-ftrace.c
@@ -0,0 +1,104 @@
+/*
+ * Dynamic Ftrace based Kprobes Optimization
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright (C) Hitachi Ltd., 2012
+ * Copyright 2016 Naveen N. Rao 
+ *   IBM Corporation
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static nokprobe_inline
+int __skip_singleste

[PATCH v2 2/5] powerpc: ftrace: restore LR from pt_regs

2017-02-21 Thread Naveen N. Rao
Pass the real LR to the ftrace handler. This is needed for
KPROBES_ON_FTRACE for the pre handlers.

Also, with KPROBES_ON_FTRACE, the link register may be updated by the
pre handlers or by a registed kretprobe. Honor updated LR by restoring
it from pt_regs, rather than from the stack save area.

Live patch and function graph continue to work fine with this change.

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/entry_64.S | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 8fd8718722a1..744b2f91444a 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1248,9 +1248,10 @@ _GLOBAL(ftrace_caller)
 
/* Get the _mcount() call site out of LR */
mflrr7
-   /* Save it as pt_regs->nip & pt_regs->link */
+   /* Save it as pt_regs->nip */
std r7, _NIP(r1)
-   std r7, _LINK(r1)
+   /* Save the read LR in pt_regs->link */
+   std r0, _LINK(r1)
 
/* Save callee's TOC in the ABI compliant location */
std r2, 24(r1)
@@ -1297,16 +1298,16 @@ ftrace_call:
REST_8GPRS(16,r1)
REST_8GPRS(24,r1)
 
+   /* Restore possibly modified LR */
+   ld  r0, _LINK(r1)
+   mtlrr0
+
/* Restore callee's TOC */
ld  r2, 24(r1)
 
/* Pop our stack frame */
addi r1, r1, SWITCH_FRAME_SIZE
 
-   /* Restore original LR for return to B */
-   ld  r0, LRSAVE(r1)
-   mtlrr0
-
 #ifdef CONFIG_LIVEPATCH
 /* Based on the cmpd above, if the NIP was altered handle livepatch */
bne-livepatch_handler
-- 
2.11.0



[PATCH v2 1/5] powerpc: ftrace: minor cleanup

2017-02-21 Thread Naveen N. Rao
Move the stack setup and teardown code to the ftrace_graph_caller().
This way, we don't incur the cost of setting it up unless function graph
is enabled for this function.

Also, remove the extraneous LR restore code after the function graph
stub. LR has previously been restored and neither livepatch_handler()
nor ftrace_graph_caller() return back here.

Signed-off-by: Naveen N. Rao <naveen.n@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/entry_64.S | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 6432d4bf08c8..8fd8718722a1 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1313,16 +1313,12 @@ ftrace_call:
 #endif
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
-   stdur1, -112(r1)
 .globl ftrace_graph_call
 ftrace_graph_call:
b   ftrace_graph_stub
 _GLOBAL(ftrace_graph_stub)
-   addir1, r1, 112
 #endif
 
-   ld  r0,LRSAVE(r1)   /* restore callee's lr at _mcount site */
-   mtlrr0
bctr/* jump after _mcount site */
 #endif /* CC_USING_MPROFILE_KERNEL */
 
@@ -1446,6 +1442,7 @@ _GLOBAL(ftrace_stub)
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 #ifndef CC_USING_MPROFILE_KERNEL
 _GLOBAL(ftrace_graph_caller)
+   stdur1, -112(r1)
/* load r4 with local address */
ld  r4, 128(r1)
subir4, r4, MCOUNT_INSN_SIZE
@@ -1471,6 +1468,7 @@ _GLOBAL(ftrace_graph_caller)
 
 #else /* CC_USING_MPROFILE_KERNEL */
 _GLOBAL(ftrace_graph_caller)
+   stdur1, -112(r1)
/* with -mprofile-kernel, parameter regs are still alive at _mcount */
std r10, 104(r1)
std r9, 96(r1)
-- 
2.11.0



[PATCH v2 1/5] powerpc: ftrace: minor cleanup

2017-02-21 Thread Naveen N. Rao
Move the stack setup and teardown code to the ftrace_graph_caller().
This way, we don't incur the cost of setting it up unless function graph
is enabled for this function.

Also, remove the extraneous LR restore code after the function graph
stub. LR has previously been restored and neither livepatch_handler()
nor ftrace_graph_caller() return back here.

Signed-off-by: Naveen N. Rao 
---
 arch/powerpc/kernel/entry_64.S | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 6432d4bf08c8..8fd8718722a1 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1313,16 +1313,12 @@ ftrace_call:
 #endif
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
-   stdur1, -112(r1)
 .globl ftrace_graph_call
 ftrace_graph_call:
b   ftrace_graph_stub
 _GLOBAL(ftrace_graph_stub)
-   addir1, r1, 112
 #endif
 
-   ld  r0,LRSAVE(r1)   /* restore callee's lr at _mcount site */
-   mtlrr0
bctr/* jump after _mcount site */
 #endif /* CC_USING_MPROFILE_KERNEL */
 
@@ -1446,6 +1442,7 @@ _GLOBAL(ftrace_stub)
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 #ifndef CC_USING_MPROFILE_KERNEL
 _GLOBAL(ftrace_graph_caller)
+   stdur1, -112(r1)
/* load r4 with local address */
ld  r4, 128(r1)
subir4, r4, MCOUNT_INSN_SIZE
@@ -1471,6 +1468,7 @@ _GLOBAL(ftrace_graph_caller)
 
 #else /* CC_USING_MPROFILE_KERNEL */
 _GLOBAL(ftrace_graph_caller)
+   stdur1, -112(r1)
/* with -mprofile-kernel, parameter regs are still alive at _mcount */
std r10, 104(r1)
std r9, 96(r1)
-- 
2.11.0



<    1   2   3   4   5   6   7   8   9   10   >