| Message ID | fd619418f5967ca7a77e5c1b49e632862040703b.1488475783.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, 2 Mar 2017 23:25:06 +0530 "Naveen N. Rao" <naveen.n.rao@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. Thanks, > > Signed-off-by: Naveen N. Rao <naveen.n.rao@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 >
On Sat, 4 Mar 2017 09:49:11 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > On Thu, 2 Mar 2017 23:25:06 +0530 > "Naveen N. Rao" <naveen.n.rao@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 0000000000000000 T user_read 0000000000000000 t user_read $ sudo ./perf probe -D user_read%return r:probe/user_read _text+3539616 r:probe/user_read_1 _text+3653408 OK, looks good. However, when I set the retprobes, I got an error. $ sudo ./perf probe -a user_read%return Failed to write event: Invalid argument Error: Failed to add events. And kernel rejected that. $ dmesg -k | tail -n 1 [ 850.315068] Given offset is not valid for return probe. Hmm, curious.. I tried normal probes $ sudo ./perf probe -D user_read p:probe/user_read _text+3539616 p:probe/user_read_1 _text+3653408 $ sudo ./perf probe -a user_read Added new events: probe:user_read (on user_read) probe:user_read_1 (on user_read) You can now use it in all perf tools, such as: perf record -e probe:user_read_1 -aR sleep 1 It works! $ sudo ./perf probe -l probe:user_read (on user_read@security/keys/user_defined.c) probe:user_read_1 (on user_read@selinux/ss/policydb.c) $ sudo cat /sys/kernel/debug/kprobes/list ffffffff9237bf20 k user_read+0x0 [DISABLED][FTRACE] ffffffff923602a0 k user_read+0x0 [DISABLED][FTRACE] So, the both "_text+3539616" and "_text+3653408" are correctly located on the entry address of user_read functions. It seems kernel-side symbol+offset check is wrong. Thank you, > > Thanks, > > > > > Signed-off-by: Naveen N. Rao <naveen.n.rao@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 > > > > > -- > Masami Hiramatsu <mhiramat@kernel.org>
On Sat, 4 Mar 2017 11:35:51 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > On Sat, 4 Mar 2017 09:49:11 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > On Thu, 2 Mar 2017 23:25:06 +0530 > > "Naveen N. Rao" <naveen.n.rao@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 > 0000000000000000 T user_read > 0000000000000000 t user_read > $ sudo ./perf probe -D user_read%return > r:probe/user_read _text+3539616 > r:probe/user_read_1 _text+3653408 > > OK, looks good. However, when I set the retprobes, I got an error. > > $ sudo ./perf probe -a user_read%return > Failed to write event: Invalid argument > Error: Failed to add events. > > And kernel rejected that. > > $ dmesg -k | tail -n 1 > [ 850.315068] Given offset is not valid for return probe. > > Hmm, curious.. > > I tried normal probes > > $ sudo ./perf probe -D user_read > p:probe/user_read _text+3539616 > p:probe/user_read_1 _text+3653408 > $ sudo ./perf probe -a user_read > Added new events: > probe:user_read (on user_read) > probe:user_read_1 (on user_read) > > You can now use it in all perf tools, such as: > > perf record -e probe:user_read_1 -aR sleep 1 > > It works! > > $ sudo ./perf probe -l > probe:user_read (on user_read@security/keys/user_defined.c) > probe:user_read_1 (on user_read@selinux/ss/policydb.c) > $ sudo cat /sys/kernel/debug/kprobes/list > ffffffff9237bf20 k user_read+0x0 [DISABLED][FTRACE] > ffffffff923602a0 k user_read+0x0 [DISABLED][FTRACE] > > So, the both "_text+3539616" and "_text+3653408" are correctly located > on the entry address of user_read functions. It seems kernel-side > symbol+offset check is wrong. FYI, without this patch, perf probe returns same place for same-name functions. So this patch itself looks good. $ sudo ./perf probe -D user_read%return r:probe/user_read user_read+0 r:probe/user_read_1 user_read+0 Thanks,
On Sat, 4 Mar 2017 11:35:51 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > On Sat, 4 Mar 2017 09:49:11 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > On Thu, 2 Mar 2017 23:25:06 +0530 > > "Naveen N. Rao" <naveen.n.rao@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 > 0000000000000000 T user_read > 0000000000000000 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, &offset); if (ret) { pr_info("Failed to parse symbol.\n"); return ret; } if (offset && is_return && !arch_function_offset_within_entry(offset)) { pr_info("Given offset is not valid for return probe.\n"); return -EINVAL; } } So, actually, traceprobe_split_symbol_offset() just split out symbol and offset from symbol string (e.g. "_text+3539616"). So, you should use kallsyms_lookup_size_offset() here again to check offset. Please try attached patch (I've already tested on x86-64). $ sudo ./perf probe -a user_read%return Added new events: probe:user_read (on user_read%return) probe:user_read_1 (on user_read%return) You can now use it in all perf tools, such as: perf record -e probe:user_read_1 -aR sleep 1 $ sudo ./perf probe -l probe:user_read (on user_read%return@security/keys/user_defined.c) probe:user_read_1 (on user_read%return@selinux/ss/policydb.c) $ sudo cat /sys/kernel/debug/kprobes/list ffffffff9637bf70 r user_read+0x0 [DISABLED][FTRACE] ffffffff963602f0 r user_read+0x0 [DISABLED][FTRACE] Thank you,
On 2017/03/04 09:49AM, Masami Hiramatsu wrote: > On Thu, 2 Mar 2017 23:25:06 +0530 > "Naveen N. Rao" <naveen.n.rao@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): [<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] Thanks, - Naveen
On 2017/03/04 01:34PM, Masami Hiramatsu wrote: > On Sat, 4 Mar 2017 11:35:51 +0900 > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > On Sat, 4 Mar 2017 09:49:11 +0900 > > Masami Hiramatsu <mhiramat@kernel.org> wrote: > > > > > On Thu, 2 Mar 2017 23:25:06 +0530 > > > "Naveen N. Rao" <naveen.n.rao@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 > > 0000000000000000 T user_read > > 0000000000000000 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, &offset); > 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 > ffffffff9637bf70 r user_read+0x0 [DISABLED][FTRACE] > ffffffff963602f0 r user_read+0x0 [DISABLED][FTRACE] Thanks, I will test this. - Naveen
On Mon, 6 Mar 2017 20:34:10 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote: > On 2017/03/04 09:49AM, Masami Hiramatsu wrote: > > On Thu, 2 Mar 2017 23:25:06 +0530 > > "Naveen N. Rao" <naveen.n.rao@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): [<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] Ok, with this usage example. Acked-by: Masami Hiramatsu <mhiramat@kernel.org> Thanks, > > > Thanks, > - Naveen >
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) {
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.rao@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(-)