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/[email protected]/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
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: 0xc0000000004ba9c4
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
<snip>
[ 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: 0xc0000000004ba9e4
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
c000000000041370 k kretprobe_trampoline+0x0 [OPTIMIZED]
c0000000004ba0b8 r do_open+0x8 [DISABLED]
c000000000443430 r do_open+0x0 [DISABLED]
Acked-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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;
@@ -1413,6 +1411,11 @@ static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
return ERR_PTR(-EINVAL);
}
+static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
+{
+ return _kprobe_addr(p->addr, p->symbol_name, p->offset);
+}
+
/* Check passed kprobe is valid and return kprobe in kprobe_table. */
static struct kprobe *__get_valid_kprobe(struct kprobe *p)
{
@@ -1880,19 +1883,28 @@ bool __weak arch_function_offset_within_entry(unsigned long offset)
return !offset;
}
+bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
+{
+ kprobe_opcode_t *kp_addr = _kprobe_addr(addr, sym, offset);
+
+ if (IS_ERR(kp_addr))
+ return false;
+
+ if (!kallsyms_lookup_size_offset((unsigned long)kp_addr, NULL, &offset) ||
+ !arch_function_offset_within_entry(offset))
+ return false;
+
+ return true;
+}
+
int register_kretprobe(struct kretprobe *rp)
{
int ret = 0;
struct kretprobe_instance *inst;
int i;
void *addr;
- unsigned long offset;
-
- addr = kprobe_addr(&rp->kp);
- if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, &offset))
- return -EINVAL;
- if (!arch_function_offset_within_entry(offset))
+ if (!function_offset_within_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
return -EINVAL;
if (kretprobe_blacklist_size) {
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 12fb540da0e5..013f4e7146d4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -697,7 +697,7 @@ static int create_trace_kprobe(int argc, char **argv)
return ret;
}
if (offset && is_return &&
- !arch_function_offset_within_entry(offset)) {
+ !function_offset_within_entry(NULL, symbol, offset)) {
pr_info("Given offset is not valid for return probe.\n");
return -EINVAL;
}
--
2.11.1
With ABIv2, we offset 8 bytes into a function to get at the local entry
point.
Acked-by: Ananth N Mavinakayanahalli <[email protected]>
Acked-by: Michael Ellerman <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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
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): [<module>:]<symbol>[+<offset>]|<memaddr>
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: 0xc0000000004ba984
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
c000000000041370 k kretprobe_trampoline+0x0 [OPTIMIZED]
c0000000004433d0 r do_open+0x0 [DISABLED]
c0000000004433d0 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: 0xc0000000004ba984
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
c000000000041370 k kretprobe_trampoline+0x0 [OPTIMIZED]
c0000000004433d0 r do_open+0x0 [DISABLED]
c0000000004ba058 r do_open+0x8 [DISABLED]
Acked-by: Masami Hiramatsu <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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 &&
+ (!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
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 <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Naveen N. Rao <[email protected]>
---
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;
- bool avail;
- bool checked;
-} 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<bit-width>@<bit-offset>/<container-size>", 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(&buf, &len, 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(&buf, &len, 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
On Wed, 8 Mar 2017 13:56:10 +0530
"Naveen N. Rao" <[email protected]> wrote:
> 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 <[email protected]>
This patch is OK. And I found that most of functions in sym-handling.c
are used only when libelf is supported. (e.g. probe-event.c itself
is not built when we have no libelf)
So, for the next cleanup, this file should not be compiled without
libelf.
Thanks!
>
> Signed-off-by: Naveen N. Rao <[email protected]>
> ---
> 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
>
--
Masami Hiramatsu <[email protected]>
"Naveen N. Rao" <[email protected]> writes:
> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
>
> Acked-by: Ananth N Mavinakayanahalli <[email protected]>
> Acked-by: Michael Ellerman <[email protected]>
> Signed-off-by: Naveen N. Rao <[email protected]>
> ---
> arch/powerpc/kernel/kprobes.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
I'm OK with this change, and I'm happy for it to go with the rest of the
series via acme's tree:
Acked-by: Michael Ellerman <[email protected]>
But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
touches this function, see the 2nd to last hunk at:
https://patchwork.ozlabs.org/patch/730675/
If this goes via acme's tree it will be awkward for me to merge the
series above via the powerpc tree.
So we could do topic branches and so on, or we could just drop this
patch from this series, and I'll merge it as part of the other series.
It won't do anything useful until it's merged with a tree that also has
the rest of this series. Or something else I haven't thought of.
cheers
> 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
Em Wed, Mar 08, 2017 at 07:54:12PM +0530, Naveen N. Rao escreveu:
> Hi Michael,
>
> On 2017/03/08 09:43PM, Michael Ellerman wrote:
> > "Naveen N. Rao" <[email protected]> writes:
> >
> > > With ABIv2, we offset 8 bytes into a function to get at the local entry
> > > point.
> > >
> > > Acked-by: Ananth N Mavinakayanahalli <[email protected]>
> > > Acked-by: Michael Ellerman <[email protected]>
> > > Signed-off-by: Naveen N. Rao <[email protected]>
> > > ---
> > > arch/powerpc/kernel/kprobes.c | 9 +++++++++
> > > 1 file changed, 9 insertions(+)
> >
> > I'm OK with this change, and I'm happy for it to go with the rest of the
> > series via acme's tree:
> >
> > Acked-by: Michael Ellerman <[email protected]>
> >
> >
> > But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
> > touches this function, see the 2nd to last hunk at:
> >
> > https://patchwork.ozlabs.org/patch/730675/
> >
> >
> > If this goes via acme's tree it will be awkward for me to merge the
> > series above via the powerpc tree.
>
> Ah yes, indeed.
>
> >
> > So we could do topic branches and so on, or we could just drop this
> > patch from this series, and I'll merge it as part of the other series.
> > It won't do anything useful until it's merged with a tree that also has
> > the rest of this series. Or something else I haven't thought of.
>
> The arch-independent change that this depends on has been picked up by
> Arnaldo and pushed to Ingo:
> https://www.mail-archive.com/[email protected]/msg115211.html
>
> I'm guessing this will go into v4.11? In which case, this powerpc patch
> should also go in. Otherwise kretprobes will be broken on powerpc64le.
I don't think so, I've put it in a perf/core branch, meaning its not
strictly fixes, could be processed in the next merge window if Ingo
thinks we've passed the current merge window threshold for such kind of
changes, and he merged it into perf/core, meaning, at this time, that it
is aimed for 4.12.
> I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for
> v4.11. If so, it would be good to take this patch through the powerpc
> tree. Otherwise, this can go via Ingo's tree.
If you guys convince Ingo that this should go _now_, then just cherry
pick what was merged into tip/perf/core that is needed for the arch
specific stuff and go from there.
- Arnaldo
On 2017/03/08 11:31AM, Masami Hiramatsu wrote:
> On Wed, 8 Mar 2017 13:56:10 +0530
> "Naveen N. Rao" <[email protected]> wrote:
>
> > 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 <[email protected]>
>
> This patch is OK. And I found that most of functions in sym-handling.c
> are used only when libelf is supported. (e.g. probe-event.c itself
> is not built when we have no libelf)
> So, for the next cleanup, this file should not be compiled without
> libelf.
There are still a few functions there which work without libelf. But, I
agree that the file has far too many #ifdefs between ABIv2 and libelf. I
will see if I can simplify this file.
Thanks,
Naveen
Hi Michael,
On 2017/03/08 09:43PM, Michael Ellerman wrote:
> "Naveen N. Rao" <[email protected]> writes:
>
> > With ABIv2, we offset 8 bytes into a function to get at the local entry
> > point.
> >
> > Acked-by: Ananth N Mavinakayanahalli <[email protected]>
> > Acked-by: Michael Ellerman <[email protected]>
> > Signed-off-by: Naveen N. Rao <[email protected]>
> > ---
> > arch/powerpc/kernel/kprobes.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
>
> I'm OK with this change, and I'm happy for it to go with the rest of the
> series via acme's tree:
>
> Acked-by: Michael Ellerman <[email protected]>
>
>
> But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
> touches this function, see the 2nd to last hunk at:
>
> https://patchwork.ozlabs.org/patch/730675/
>
>
> If this goes via acme's tree it will be awkward for me to merge the
> series above via the powerpc tree.
Ah yes, indeed.
>
> So we could do topic branches and so on, or we could just drop this
> patch from this series, and I'll merge it as part of the other series.
> It won't do anything useful until it's merged with a tree that also has
> the rest of this series. Or something else I haven't thought of.
The arch-independent change that this depends on has been picked up by
Arnaldo and pushed to Ingo:
https://www.mail-archive.com/[email protected]/msg115211.html
I'm guessing this will go into v4.11? In which case, this powerpc patch
should also go in. Otherwise kretprobes will be broken on powerpc64le.
I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for
v4.11. If so, it would be good to take this patch through the powerpc
tree. Otherwise, this can go via Ingo's tree.
Thanks,
Naveen
On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 08, 2017 at 07:54:12PM +0530, Naveen N. Rao escreveu:
> > Hi Michael,
> >
> > On 2017/03/08 09:43PM, Michael Ellerman wrote:
> > > "Naveen N. Rao" <[email protected]> writes:
> > >
> > > > With ABIv2, we offset 8 bytes into a function to get at the local entry
> > > > point.
> > > >
> > > > Acked-by: Ananth N Mavinakayanahalli <[email protected]>
> > > > Acked-by: Michael Ellerman <[email protected]>
> > > > Signed-off-by: Naveen N. Rao <[email protected]>
> > > > ---
> > > > arch/powerpc/kernel/kprobes.c | 9 +++++++++
> > > > 1 file changed, 9 insertions(+)
> > >
> > > I'm OK with this change, and I'm happy for it to go with the rest of the
> > > series via acme's tree:
> > >
> > > Acked-by: Michael Ellerman <[email protected]>
> > >
> > >
> > > But, you've also sent a series to do KPROBES_ON_FTRACE, and that also
> > > touches this function, see the 2nd to last hunk at:
> > >
> > > https://patchwork.ozlabs.org/patch/730675/
> > >
> > >
> > > If this goes via acme's tree it will be awkward for me to merge the
> > > series above via the powerpc tree.
> >
> > Ah yes, indeed.
> >
> > >
> > > So we could do topic branches and so on, or we could just drop this
> > > patch from this series, and I'll merge it as part of the other series.
> > > It won't do anything useful until it's merged with a tree that also has
> > > the rest of this series. Or something else I haven't thought of.
> >
> > The arch-independent change that this depends on has been picked up by
> > Arnaldo and pushed to Ingo:
> > https://www.mail-archive.com/[email protected]/msg115211.html
> >
> > I'm guessing this will go into v4.11? In which case, this powerpc patch
> > should also go in. Otherwise kretprobes will be broken on powerpc64le.
>
> I don't think so, I've put it in a perf/core branch, meaning its not
> strictly fixes, could be processed in the next merge window if Ingo
> thinks we've passed the current merge window threshold for such kind of
> changes, and he merged it into perf/core, meaning, at this time, that it
> is aimed for 4.12.
Ah, thanks for clarifying.
>
> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for
> > v4.11. If so, it would be good to take this patch through the powerpc
> > tree. Otherwise, this can go via Ingo's tree.
>
> If you guys convince Ingo that this should go _now_, then just cherry
> pick what was merged into tip/perf/core that is needed for the arch
> specific stuff and go from there.
Ok, in hindsight, I think Michael's concern was actually for v4.12
itself, in which case this particular patch can go via powerpc tree,
while the rest of the patches in this series can go via your tree.
Michael?
Thanks,
Naveen
"Naveen N. Rao" <[email protected]> writes:
> On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
>> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for
>> > v4.11. If so, it would be good to take this patch through the powerpc
>> > tree. Otherwise, this can go via Ingo's tree.
>>
>> If you guys convince Ingo that this should go _now_, then just cherry
>> pick what was merged into tip/perf/core that is needed for the arch
>> specific stuff and go from there.
>
> Ok, in hindsight, I think Michael's concern was actually for v4.12
Yes I was talking about 4.12, sorry I thought that was implied :)
> itself, in which case this particular patch can go via powerpc tree,
> while the rest of the patches in this series can go via your tree.
>
> Michael?
Yeah I think that's the easiest option. The function will be temporarily
unused until the two trees are merged, but I think that's fine.
cheers
On 2017/03/09 05:37PM, Michael Ellerman wrote:
> "Naveen N. Rao" <[email protected]> writes:
> > On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> >> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for
> >> > v4.11. If so, it would be good to take this patch through the powerpc
> >> > tree. Otherwise, this can go via Ingo's tree.
> >>
> >> If you guys convince Ingo that this should go _now_, then just cherry
> >> pick what was merged into tip/perf/core that is needed for the arch
> >> specific stuff and go from there.
> >
> > Ok, in hindsight, I think Michael's concern was actually for v4.12
>
> Yes I was talking about 4.12, sorry I thought that was implied :)
I suppose it was evident for everyone except the overzealous me :D
Sorry for all the confusion.
>
> > itself, in which case this particular patch can go via powerpc tree,
> > while the rest of the patches in this series can go via your tree.
> >
> > Michael?
>
> Yeah I think that's the easiest option. The function will be temporarily
> unused until the two trees are merged, but I think that's fine.
Sure, thanks!
- Naveen
Em Thu, Mar 09, 2017 at 05:37:38PM +1100, Michael Ellerman escreveu:
> "Naveen N. Rao" <[email protected]> writes:
> > On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> >> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for
> >> > v4.11. If so, it would be good to take this patch through the powerpc
> >> > tree. Otherwise, this can go via Ingo's tree.
> >>
> >> If you guys convince Ingo that this should go _now_, then just cherry
> >> pick what was merged into tip/perf/core that is needed for the arch
> >> specific stuff and go from there.
> >
> > Ok, in hindsight, I think Michael's concern was actually for v4.12
>
> Yes I was talking about 4.12, sorry I thought that was implied :)
>
> > itself, in which case this particular patch can go via powerpc tree,
> > while the rest of the patches in this series can go via your tree.
> >
> > Michael?
>
> Yeah I think that's the easiest option. The function will be temporarily
> unused until the two trees are merged, but I think that's fine.
Ok, done that, now compile testing building it in my
multi-distro/x-build containers.
- Arnaldo
On 2017/03/14 10:18AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 09, 2017 at 05:37:38PM +1100, Michael Ellerman escreveu:
> > "Naveen N. Rao" <[email protected]> writes:
> > > On 2017/03/08 11:29AM, Arnaldo Carvalho de Melo wrote:
> > >> > I wasn't sure if you were planning on picking up KPROBES_ON_FTRACE for
> > >> > v4.11. If so, it would be good to take this patch through the powerpc
> > >> > tree. Otherwise, this can go via Ingo's tree.
> > >>
> > >> If you guys convince Ingo that this should go _now_, then just cherry
> > >> pick what was merged into tip/perf/core that is needed for the arch
> > >> specific stuff and go from there.
> > >
> > > Ok, in hindsight, I think Michael's concern was actually for v4.12
> >
> > Yes I was talking about 4.12, sorry I thought that was implied :)
> >
> > > itself, in which case this particular patch can go via powerpc tree,
> > > while the rest of the patches in this series can go via your tree.
> > >
> > > Michael?
> >
> > Yeah I think that's the easiest option. The function will be temporarily
> > unused until the two trees are merged, but I think that's fine.
>
> Ok, done that, now compile testing building it in my
> multi-distro/x-build containers.
Thanks, Arnaldo!
I did however notice that you don't seem to have applied Patch 1/5 from
this series:
https://www.mail-archive.com/[email protected]/msg1347858.html
That patch is needed to ensure perf continues to work when ftrace README
advertises support for ref_reloc_sym+offset for kretprobes. Can you
please apply that as well?
- Naveen
Commit-ID: 44ca9341f65295c56e904cce4c84f5778f5c8537
Gitweb: http://git.kernel.org/tip/44ca9341f65295c56e904cce4c84f5778f5c8537
Author: Naveen N. Rao <[email protected]>
AuthorDate: Wed, 8 Mar 2017 13:56:10 +0530
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 14 Mar 2017 15:17:39 -0300
perf powerpc: Choose local entry point with kretprobes
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 <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/7445b5334673ef5404ac1d12609bad4d73d2b567.1488961018.git.naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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 1030a6e..39dbe51 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)
Commit-ID: 3da3ea7a8e205edc24b9491a459b46527c70b5b1
Gitweb: http://git.kernel.org/tip/3da3ea7a8e205edc24b9491a459b46527c70b5b1
Author: Naveen N. Rao <[email protected]>
AuthorDate: Wed, 8 Mar 2017 13:56:08 +0530
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 14 Mar 2017 15:17:38 -0300
perf probe: Factor out the ftrace README scanning
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 <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/6dc30edc747ba82a236593be6cf3a046fa9453b5.1488961018.git.naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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 1a62dac..8a219cd 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;
- bool avail;
- bool checked;
-} 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<bit-width>@<bit-offset>/<container-size>", 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(&buf, &len, 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(&buf, &len, 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;
}
Commit-ID: 7ab31d94bff96f4f80b38dc5147622b9f3889ac6
Gitweb: http://git.kernel.org/tip/7ab31d94bff96f4f80b38dc5147622b9f3889ac6
Author: Naveen N. Rao <[email protected]>
AuthorDate: Wed, 8 Mar 2017 13:56:09 +0530
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 14 Mar 2017 15:17:39 -0300
perf kretprobes: Offset from reloc_sym if kernel supports it
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): [<module>:]<symbol>[+<offset>]|<memaddr>
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: 0xc0000000004ba984
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
c000000000041370 k kretprobe_trampoline+0x0 [OPTIMIZED]
c0000000004433d0 r do_open+0x0 [DISABLED]
c0000000004433d0 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: 0xc0000000004ba984
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
c000000000041370 k kretprobe_trampoline+0x0 [OPTIMIZED]
c0000000004433d0 r do_open+0x0 [DISABLED]
c0000000004ba058 r do_open+0x8 [DISABLED]
Signed-off-by: Naveen N. Rao <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/496ef9f33c1ab16286ece9dd62aa672807aef91c.1488961018.git.naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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 28fb62c..c9bdc9d 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 8a219cd..1542cd0 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 a17a82e..dbf95a0 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)
{
Commit-ID: 1d585e70905e03e8c19c9aaf523ec246ae6b18a1
Gitweb: http://git.kernel.org/tip/1d585e70905e03e8c19c9aaf523ec246ae6b18a1
Author: Naveen N. Rao <[email protected]>
AuthorDate: Wed, 8 Mar 2017 13:56:06 +0530
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 15 Mar 2017 17:48:37 -0300
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: 0xc0000000004ba9c4
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
<snip>
[ 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: 0xc0000000004ba9e4
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
c000000000041370 k kretprobe_trampoline+0x0 [OPTIMIZED]
c0000000004ba0b8 r do_open+0x8 [DISABLED]
c000000000443430 r do_open+0x0 [DISABLED]
Signed-off-by: Naveen N. Rao <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/d8cd1ef420ec22e3643ac332fdabcffc77319a42.1488961018.git.naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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 177bdf6..47e4da5 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 4780ec23..d733479 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;
@@ -1413,6 +1411,11 @@ invalid:
return ERR_PTR(-EINVAL);
}
+static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
+{
+ return _kprobe_addr(p->addr, p->symbol_name, p->offset);
+}
+
/* Check passed kprobe is valid and return kprobe in kprobe_table. */
static struct kprobe *__get_valid_kprobe(struct kprobe *p)
{
@@ -1881,19 +1884,28 @@ bool __weak arch_function_offset_within_entry(unsigned long offset)
return !offset;
}
+bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
+{
+ kprobe_opcode_t *kp_addr = _kprobe_addr(addr, sym, offset);
+
+ if (IS_ERR(kp_addr))
+ return false;
+
+ if (!kallsyms_lookup_size_offset((unsigned long)kp_addr, NULL, &offset) ||
+ !arch_function_offset_within_entry(offset))
+ return false;
+
+ return true;
+}
+
int register_kretprobe(struct kretprobe *rp)
{
int ret = 0;
struct kretprobe_instance *inst;
int i;
void *addr;
- unsigned long offset;
-
- addr = kprobe_addr(&rp->kp);
- if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, &offset))
- return -EINVAL;
- if (!arch_function_offset_within_entry(offset))
+ if (!function_offset_within_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
return -EINVAL;
if (kretprobe_blacklist_size) {
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 12fb540..013f4e7 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -697,7 +697,7 @@ static int create_trace_kprobe(int argc, char **argv)
return ret;
}
if (offset && is_return &&
- !arch_function_offset_within_entry(offset)) {
+ !function_offset_within_entry(NULL, symbol, offset)) {
pr_info("Given offset is not valid for return probe.\n");
return -EINVAL;
}
On Wed, 2017-03-08 at 08:26:07 UTC, "Naveen N. Rao" wrote:
> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
>
> Acked-by: Ananth N Mavinakayanahalli <[email protected]>
> Acked-by: Michael Ellerman <[email protected]>
> Signed-off-by: Naveen N. Rao <[email protected]>
> Acked-by: Michael Ellerman <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/a64e3f35a45f4a84148d0ba30a3c75
cheers