Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1586166 > unrolled thread

[PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-02-22 15:00 +0100
    Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if  kernel supports it Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-23 10:20 +0100
      [PATCH v3 1/2] perf: probe: generalize probe event file open routine "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-02-23 12:40 +0100
        [PATCH v3 2/2] perf: kretprobes: offset from reloc_sym if kernel supports it "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-02-23 12:40 +0100
          Re: [PATCH v3 2/2] perf: kretprobes: offset from reloc_sym if  kernel supports it Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-24 18:30 +0100
        Re: [PATCH v3 1/2] perf: probe: generalize probe event file open  routine Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-24 17:50 +0100
          Re: [PATCH v3 1/2] perf: probe: generalize probe event file open  routine Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-24 21:10 +0100
      Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel  supports it "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-02-23 20:20 +0100
        Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if  kernel supports it Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-24 18:40 +0100
          Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel  supports it Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-24 21:20 +0100
            Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if  kernel supports it Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-25 01:00 +0100

#1586166 — [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

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

[toc] | [next] | [standalone]

#1586764 — Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

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
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1586832 — [PATCH v3 1/2] perf: probe: generalize probe event file open routine

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

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

On Thu, 23 Feb 2017 17:07:24 +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 | 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)

Thank you,


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


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1587786 — Re: [PATCH v3 1/2] perf: probe: generalize probe event file open routine

On Thu, 23 Feb 2017 17:07:23 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> 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,

> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  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 <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1587912 — Re: [PATCH v3 1/2] perf: probe: generalize probe event file open routine

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" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> 
> > ...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,
> 
> > 
> > Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> > ---
> >  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 <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1587094 — Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

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

[toc] | [prev] | [next] | [standalone]

#1587827 — Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

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,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [next] | [standalone]

#1587914 — Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

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

[toc] | [prev] | [next] | [standalone]

#1588010 — Re: [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it

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.

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web