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

[Naveen N. Rao]

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

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

- Naveen