[PATCH v2 0/5] kretprobe fixes

I'm including all patches (generic and powerpc changes) in
this series as suggested by Masami.

v1 patches:
https://marc.info/?l=linux-kernel&m=148718276424380
https://marc.info/?l=linux-kernel&m=148723314105453&w=2

Patches 1 and 2 are the same as v1.
Patch 3 is updated to include a line in ftrace README.
Patch 4 is new.
Patch 5 is updated to consider ftrace README.


Thanks,
Naveen

Naveen N. Rao (5):
  kretprobes: ensure probe location is at function entry
  powerpc: kretprobes: override default function entry offset
  trace/kprobes: allow return probes with offsets and absolute addresses
  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                            | 13 ++++++++
 kernel/trace/trace.c                        |  1 +
 kernel/trace/trace_kprobe.c                 |  8 -----
 tools/perf/arch/powerpc/util/sym-handling.c |  9 +++---
 tools/perf/util/probe-event.c               | 47 ++++++++++++++++++++++++-----
 tools/perf/util/probe-event.h               |  2 ++
 8 files changed, 71 insertions(+), 19 deletions(-)

-- 
2.11.0

kretprobes can be registered by specifying an absolute address or by
specifying offset to a symbol. However, we need to ensure this falls at
function entry so as to be able to determine the return address.

Validate the same during kretprobe registration. By default, there
should not be any offset from a function entry, as determined through a
kallsyms_lookup(). Introduce arch_function_offset_within_entry() as a
way for architectures to override this.

Signed-off-by: Naveen N. Rao <redacted>
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c        | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 16ddfb8b304a..862f87adcbb4 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h

@@ -266,6 +266,7 @@ extern int arch_init_kprobes(void);
 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 within_kprobe_blacklist(unsigned long addr);
 

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index ebb4dadca66b..524766563896 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c

@@ -1869,12 +1869,25 @@ static int pre_handler_kretprobe(struct kprobe *p, struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
 
+bool __weak arch_function_offset_within_entry(unsigned long offset)
+{
+	return !offset;
+}
+
 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))
+		return -EINVAL;
 
 	if (kretprobe_blacklist_size) {
 		addr = kprobe_addr(&rp->kp);

-- 
2.11.0

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 <redacted>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 9 +++++----
 1 file changed, 5 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..73dbdc83286c 100644
--- a/tools/perf/arch/powerpc/util/sym-handling.c
+++ b/tools/perf/arch/powerpc/util/sym-handling.c

@@ -79,11 +79,12 @@ 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 && !is_kretprobe_offset_supported())
 		return;
 
 	lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);

-- 
2.11.0

With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Signed-off-by: Naveen N. Rao <redacted>
---
 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 735ff3d3f77d..e37b76b8b6b2 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.0

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 <redacted>
---
 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;
+}
+
 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

Since the kernel includes many non-global functions with same names, we
will need to use offsets from other symbols (typically _text/_stext) or
absolute addresses to place return probes on specific functions. Also,
the core register_kretprobe() API never forbid use of offsets or
absolute addresses with kretprobes.

Allow its use with the trace infrastructure. To distinguish kernels that
support this, update ftrace README to explicitly call this out.

Signed-off-by: Naveen N. Rao <redacted>
---
 kernel/trace/trace.c        | 1 +
 kernel/trace/trace_kprobe.c | 8 --------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d7449783987a..ababe56b3e8f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c

@@ -4362,6 +4362,7 @@ static const char readme_msg[] =
 	"\t           -:[<group>/]<event>\n"
 #ifdef CONFIG_KPROBE_EVENT
 	"\t    place: [<module>:]<symbol>[+<offset>]|<memaddr>\n"
+  "place (kretprobe): [<module>:]<symbol>[+<offset>]|<memaddr>\n"
 #endif
 #ifdef CONFIG_UPROBE_EVENT
 	"\t    place: <path>:<offset>\n"

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7ad9e53ad174..2768cb60ebd7 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c

@@ -679,10 +679,6 @@ static int create_trace_kprobe(int argc, char **argv)
 		return -EINVAL;
 	}
 	if (isdigit(argv[1][0])) {
-		if (is_return) {
-			pr_info("Return probe point must be a symbol.\n");
-			return -EINVAL;
-		}
 		/* an address specified */
 		ret = kstrtoul(&argv[1][0], 0, (unsigned long *)&addr);
 		if (ret) {

@@ -698,10 +694,6 @@ static int create_trace_kprobe(int argc, char **argv)
 			pr_info("Failed to parse symbol.\n");
 			return ret;
 		}
-		if (offset && is_return) {
-			pr_info("Return probe must be used without offset.\n");
-			return -EINVAL;
-		}
 	}
 	argc -= 2; argv += 2;
 

-- 
2.11.0

On Wed, 22 Feb 2017 19:23:40 +0530
"Naveen N. Rao" [off-list ref] wrote:

Could you reuse (refactoring) probe_type_is_available() in probe-file.c to share
opening README file?

Others looks good to me :)

Thank you,

-- 
Masami Hiramatsu [off-list ref]

...into a generic function for opening trace files.

Signed-off-by: Naveen N. Rao <redacted>
---
 tools/perf/util/probe-file.c | 20 +++++++++++---------
 tools/perf/util/probe-file.h |  1 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b64731f65..1a62daceb028 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c

@@ -70,7 +70,7 @@ static void print_both_open_warning(int kerr, int uerr)
 	}
 }
 
-static int open_probe_events(const char *trace_file, bool readwrite)
+int open_trace_file(const char *trace_file, bool readwrite)
 {
 	char buf[PATH_MAX];
 	int ret;

@@ -92,12 +92,12 @@ static int open_probe_events(const char *trace_file, bool readwrite)
 
 static int open_kprobe_events(bool readwrite)
 {
-	return open_probe_events("kprobe_events", readwrite);
+	return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
-	return open_probe_events("uprobe_events", readwrite);
+	return open_trace_file("uprobe_events", readwrite);
 }
 
 int probe_file__open(int flag)

@@ -899,6 +899,7 @@ bool probe_type_is_available(enum probe_type type)
 	size_t len = 0;
 	bool target_line = false;
 	bool ret = probe_type_table[type].avail;
+	int fd;
 
 	if (type >= PROBE_TYPE_END)
 		return false;

@@ -906,14 +907,16 @@ bool probe_type_is_available(enum probe_type type)
 	if (ret || probe_type_table[type].checked)
 		return ret;
 
-	if (asprintf(&buf, "%s/README", tracing_path) < 0)
+	fd = open_trace_file("README", false);
+	if (fd < 0)
 		return ret;
 
-	fp = fopen(buf, "r");
-	if (!fp)
-		goto end;
+	fp = fdopen(fd, "r");
+	if (!fp) {
+		close(fd);
+		return ret;
+	}
 
-	zfree(&buf);
 	while (getline(&buf, &len, fp) > 0 && !ret) {
 		if (!target_line) {
 			target_line = !!strstr(buf, " type: ");

@@ -928,7 +931,6 @@ bool probe_type_is_available(enum probe_type type)
 	probe_type_table[type].avail = ret;
 
 	fclose(fp);
-end:
 	free(buf);
 
 	return ret;

diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index eba44c3e9dca..a17a82eff8a0 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h

@@ -35,6 +35,7 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
+int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);

-- 
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.

Signed-off-by: Naveen N. Rao <redacted>
---
 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;
+}
+
 static int
 post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 				       int ntevs)

@@ -757,7 +794,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 +1567,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 +2875,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.1

On 2017/02/23 06:10PM, Masami Hiramatsu wrote:

On Wed, 22 Feb 2017 19:23:40 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

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 <redacted>
---
 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 Thu, 23 Feb 2017 17:07:23 +0530
"Naveen N. Rao" [off-list ref] wrote:

...into a generic function for opening trace files.

Even if it repeats subject, please write complete description...

Patch itself is OK to me.

Thanks,

-- 
Masami Hiramatsu [off-list ref]

On Thu, 23 Feb 2017 17:07:24 +0530
"Naveen N. Rao" [off-list ref] wrote:

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)

Thank you,

-- 
Masami Hiramatsu [off-list ref]

On Fri, 24 Feb 2017 00:46:08 +0530
"Naveen N. Rao" [off-list ref] wrote:

On 2017/02/23 06:10PM, Masami Hiramatsu wrote:

quoted

On Wed, 22 Feb 2017 19:23:40 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

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 <redacted>
---
 tools/perf/util/probe-event.c | 47 ++++++++++++++++++++++++++++++++++++-------
 tools/perf/util/probe-event.h |  2 ++
 2 files changed, 42 insertions(+), 7 deletions(-)

[snip]

quoted

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.

quoted

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,


-- 
Masami Hiramatsu [off-list ref]

Em Wed, Feb 22, 2017 at 07:23:38PM +0530, Naveen N. Rao escreveu:

With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

So, I think I can carry the first one via Ingo, etc, what about this
one?

Is it ok for me to process it?

Seems simple enough, has been thru a lot of discussion, but would be
better if it was Reviewed-by the PPC maintainers or else just processed
by them.

Please advise,

- Arnaldo
 

Em Sat, Feb 25, 2017 at 01:46:01AM +0900, Masami Hiramatsu escreveu:

On Thu, 23 Feb 2017 17:07:23 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

...into a generic function for opening trace files.

Even if it repeats subject, please write complete description...

Patch itself is OK to me.

Did it and added your Acked-by as per your OK above.

- arnaldo
 

Thanks,

quoted

Signed-off-by: Naveen N. Rao <redacted>
---
 tools/perf/util/probe-file.c | 20 +++++++++++---------
 tools/perf/util/probe-file.h |  1 +
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
index 436b64731f65..1a62daceb028 100644
--- a/tools/perf/util/probe-file.c
+++ b/tools/perf/util/probe-file.c

@@ -70,7 +70,7 @@ static void print_both_open_warning(int kerr, int uerr)
 	}
 }
 
-static int open_probe_events(const char *trace_file, bool readwrite)
+int open_trace_file(const char *trace_file, bool readwrite)
 {
 	char buf[PATH_MAX];
 	int ret;

@@ -92,12 +92,12 @@ static int open_probe_events(const char *trace_file, bool readwrite)
 
 static int open_kprobe_events(bool readwrite)
 {
-	return open_probe_events("kprobe_events", readwrite);
+	return open_trace_file("kprobe_events", readwrite);
 }
 
 static int open_uprobe_events(bool readwrite)
 {
-	return open_probe_events("uprobe_events", readwrite);
+	return open_trace_file("uprobe_events", readwrite);
 }
 
 int probe_file__open(int flag)

@@ -899,6 +899,7 @@ bool probe_type_is_available(enum probe_type type)
 	size_t len = 0;
 	bool target_line = false;
 	bool ret = probe_type_table[type].avail;
+	int fd;
 
 	if (type >= PROBE_TYPE_END)
 		return false;

@@ -906,14 +907,16 @@ bool probe_type_is_available(enum probe_type type)
 	if (ret || probe_type_table[type].checked)
 		return ret;
 
-	if (asprintf(&buf, "%s/README", tracing_path) < 0)
+	fd = open_trace_file("README", false);
+	if (fd < 0)
 		return ret;
 
-	fp = fopen(buf, "r");
-	if (!fp)
-		goto end;
+	fp = fdopen(fd, "r");
+	if (!fp) {
+		close(fd);
+		return ret;
+	}
 
-	zfree(&buf);
 	while (getline(&buf, &len, fp) > 0 && !ret) {
 		if (!target_line) {
 			target_line = !!strstr(buf, " type: ");

@@ -928,7 +931,6 @@ bool probe_type_is_available(enum probe_type type)
 	probe_type_table[type].avail = ret;
 
 	fclose(fp);
-end:
 	free(buf);
 
 	return ret;

diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
index eba44c3e9dca..a17a82eff8a0 100644
--- a/tools/perf/util/probe-file.h
+++ b/tools/perf/util/probe-file.h

@@ -35,6 +35,7 @@ enum probe_type {
 
 /* probe-file.c depends on libelf */
 #ifdef HAVE_LIBELF_SUPPORT
+int open_trace_file(const char *trace_file, bool readwrite);
 int probe_file__open(int flag);
 int probe_file__open_both(int *kfd, int *ufd, int flag);
 struct strlist *probe_file__get_namelist(int fd);

-- 
2.11.1

-- 
Masami Hiramatsu [off-list ref]

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" [off-list ref] wrote:

quoted

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 [off-list ref] wrote:

Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:

quoted

On Fri, 24 Feb 2017 00:46:08 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

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.

-- 
Masami Hiramatsu [off-list ref]

On Wed, Feb 22, 2017 at 07:23:38PM +0530, Naveen N. Rao wrote:

With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

Looks good.

Signed-off-by: Naveen N. Rao <redacted>

Acked-by: Ananth N Mavinakayanahalli <redacted>

On Wed, 22 Feb 2017 19:23:39 +0530
"Naveen N. Rao" [off-list ref] wrote:

I understand that your retprobes will now have an offset, but I'm
worried we are removing informative errors. For those archs that don't
allow an offset, will we still get the error telling users that offsets
are not allowed?

I don't want to lose informative error handling.

-- Steve

 	}
 	argc -= 2; argv += 2;
 

Let's not remove the warning about offsets and return probes when the
offset is invalid.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3f4f788..f626235 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c

@@ -695,6 +695,11 @@ static int create_trace_kprobe(int argc, char **argv)
 			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;
+		}
 	}
 	argc -= 2; argv += 2;
 

Arnaldo Carvalho de Melo [off-list ref] writes:

Em Wed, Feb 22, 2017 at 07:23:38PM +0530, Naveen N. Rao escreveu:

quoted

With ABIv2, we offset 8 bytes into a function to get at the local entry
point.

So, I think I can carry the first one via Ingo, etc, what about this
one?

Is it ok for me to process it?

Yes please.

Seems simple enough, has been thru a lot of discussion, but would be
better if it was Reviewed-by the PPC maintainers or else just processed
by them.

Please advise,

I think it's best if you take it with patch 1. I realise they could go
separately, but they make more sense together I think.

Acked-by: Michael Ellerman <mpe@ellerman.id.au>

cheers

On Mon, 27 Feb 2017 11:52:04 -0500
"Steven Rostedt (VMware)" [off-list ref] wrote:

Let's not remove the warning about offsets and return probes when the
offset is invalid.

Agreed, This looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

-- 
Masami Hiramatsu [off-list ref]

On 2017/02/27 11:52AM, Steven Rostedt (VMware) wrote:

Let's not remove the warning about offsets and return probes when the
offset is invalid.

Good point!

Thanks, Steve!

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

Acked-by: Naveen N. Rao <redacted>

On 2017/02/25 02:12AM, Masami Hiramatsu wrote:

On Thu, 23 Feb 2017 17:07:24 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

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 <redacted>
---
 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

On 2017/02/25 08:55AM, Masami Hiramatsu wrote:

On Fri, 24 Feb 2017 17:11:03 -0300
Arnaldo Carvalho de Melo [off-list ref] wrote:

quoted

Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:

quoted

On Fri, 24 Feb 2017 00:46:08 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

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/25 01:46AM, Masami Hiramatsu wrote:

On Thu, 23 Feb 2017 17:07:23 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

...into a generic function for opening trace files.

Even if it repeats subject, please write complete description...

Agh, ok sure. I will try to curb my urge to not type too much :)

Patch itself is OK to me.

Thanks for the review on this series!

- Naveen

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:

quoted

Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:

quoted

On Fri, 24 Feb 2017 00:46:08 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

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

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 <redacted>
---
 tools/perf/arch/powerpc/util/sym-handling.c | 10 ++++++----
 1 file changed, 6 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..cc7c2697c036 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,11 +80,12 @@ 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 && !kretprobe_offset_is_supported())
 		return;
 
 	lep_offset = PPC64_LOCAL_ENTRY_OFFSET(sym->arch_sym);

-- 
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.

Signed-off-by: Naveen N. Rao <redacted>
---
 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 2017/02/24 05:11PM, Arnaldo Carvalho de Melo wrote:

Em Sat, Feb 25, 2017 at 02:29:17AM +0900, Masami Hiramatsu escreveu:

quoted

On Fri, 24 Feb 2017 00:46:08 +0530
"Naveen N. Rao" [off-list ref] wrote:

quoted

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(-)

-- 
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.

Signed-off-by: Naveen N. Rao <redacted>
---
 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 Thu,  2 Mar 2017 23:25:05 +0530
"Naveen N. Rao" [off-list ref] wrote:

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.

Ah, OK, this can simplify and shorten the actual scanning time.
If there are any needs for checking those in the future, we can
add it again at that moment.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thank you!