| Message ID | 75bafc91e9d381636f68ab9923ce3d52f9157a09.1487770934.git.naveen.n.rao@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 22 Feb 2017 19:23:40 +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. > > Signed-off-by: Naveen N. Rao <naveen.n.rao@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(&buf, "%s/README", tracing_path) < 0) > + return false; > + > + fp = fopen(buf, "r"); > + if (!fp) > + goto end; > + > + zfree(&buf); > + while (getline(&buf, &len, 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; > +} Could you reuse (refactoring) probe_type_is_available() in probe-file.c to share opening README file? Others looks good to me :) Thank you, > + > 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 >
On 2017/02/23 06:10PM, Masami Hiramatsu wrote: > On Wed, 22 Feb 2017 19:23:40 +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. > > > > Signed-off-by: Naveen N. Rao <naveen.n.rao@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
On Fri, 24 Feb 2017 00:46:08 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote: > On 2017/02/23 06:10PM, Masami Hiramatsu wrote: > > On Wed, 22 Feb 2017 19:23:40 +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. > > > > > > Signed-off-by: Naveen N. Rao <naveen.n.rao@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. OK. > > > > > Others looks good to me :) > > 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 <mhiramat@kernel.org> And could you make v4 series including all patches? (Not only updates) > > 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? If it is not urgent (yes, it seems) and since it changes arch independent parts, I think this series should finally go through Ingo's tree. Thank you,
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.rao@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 <mhiramat@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
On Fri, 24 Feb 2017 17:11:03 -0300 Arnaldo Carvalho de Melo <acme@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.rao@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 <mhiramat@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!! Naveen, please update your ppc and perf patches and send it to Arnaldo. I'm happy to review it.
On 2017/02/25 08:55AM, Masami Hiramatsu wrote: > On Fri, 24 Feb 2017 17:11:03 -0300 > Arnaldo Carvalho de Melo <acme@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.rao@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 <mhiramat@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
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.rao@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 <mhiramat@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(-)
Em Thu, Mar 02, 2017 at 11:25:04PM +0530, Naveen N. Rao escreveu: > 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.rao@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 <mhiramat@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. Will process after Masami-san has time to review it, Thanks, - Arnaldo
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(&buf, "%s/README", tracing_path) < 0) + return false; + + fp = fopen(buf, "r"); + if (!fp) + goto end; + + zfree(&buf); + while (getline(&buf, &len, 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);
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 | 47 ++++++++++++++++++++++++++++++++++++------- tools/perf/util/probe-event.h | 2 ++ 2 files changed, 42 insertions(+), 7 deletions(-)