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

[Masami Hiramatsu]

On Thu,  2 Mar 2017 23:25:06 +0530
"Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>