[PATCH perf/core 0/4] perf-probe bugfixes

[PATCH perf/core 0/4] perf-probe bugfixes

[Masami Hiramatsu]

Hi Arnaldo,

Here is a series of bugfix patches for perf-probe. I've fixed some
bugs on non-debuginfo environment.
This series fixes below bugs;

 - --list shows the list of probes in stderr.
   (It is refined from https://lkml.org/lkml/2015/5/30/34)
 - failed to probe on gcc optimized symbols which has a postfix.
 - non-probe-able symbols (out of .text) are not checked when
   using symbol maps.
 - usage message is not shown if the last event is skipped.

Thank you,

---

Masami Hiramatsu (4):
      [BUGFIX] perf probe: List probes in stdout
      [BUGFIX] perf probe: Cut off the postfixes from event name
      [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map
      [BUGFIX] perf probe: Show usage even if the last event is skipped


 tools/perf/util/probe-event.c |  192 ++++++++++++++++++++++++++---------------
 1 file changed, 123 insertions(+), 69 deletions(-)


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering 
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH perf/core 1/4] [BUGFIX] perf probe: List probes in stdout

[Masami Hiramatsu]

Since commit 5e17b28f1e24 ("perf probe: Add --quiet option to
suppress output result message") have replaced printf with pr_info,
perf probe -l outputs its result in stderr. However, that is not
what the commit expected.
e.g.
  -----
  # perf probe -l > /dev/null
    probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
  -----
With this fix,
  -----
  # perf probe -l > list
  # cat list
    probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
  -----
Of course, --quiet(-q) still works on --add/--del.
  -----
  # perf probe -q vfs_write
  # perf probe -l
    probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
    probe:vfs_write      (on vfs_write@ksrc/linux-3/fs/read_write.c)
  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reported-by: Naohiro Aota <naota@elisp.net>
---
 tools/perf/util/probe-event.c |   49 +++++++++++++++++++++++++++++------------
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index d4cf50b..710830c 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2126,9 +2126,9 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
 	return NULL;
 }
 
-/* Show an event */
-static int show_perf_probe_event(struct perf_probe_event *pev,
-				 const char *module)
+static int perf_probe_event__sprintf(struct perf_probe_event *pev,
+				     const char *module,
+				     struct strbuf *result)
 {
 	int i, ret;
 	char buf[128];
@@ -2141,27 +2141,47 @@ static int show_perf_probe_event(struct perf_probe_event *pev,
 
 	ret = e_snprintf(buf, 128, "%s:%s", pev->group, pev->event);
 	if (ret < 0)
-		return ret;
+		goto out;
 
-	pr_info("  %-20s (on %s", buf, place);
+	strbuf_addf(result, "  %-20s (on %s", buf, place);
 	if (module)
-		pr_info(" in %s", module);
+		strbuf_addf(result, " in %s", module);
 
 	if (pev->nargs > 0) {
-		pr_info(" with");
+		strbuf_addstr(result, " with");
 		for (i = 0; i < pev->nargs; i++) {
 			ret = synthesize_perf_probe_arg(&pev->args[i],
 							buf, 128);
 			if (ret < 0)
-				break;
-			pr_info(" %s", buf);
+				goto out;
+			strbuf_addf(result, " %s", buf);
 		}
 	}
-	pr_info(")\n");
+	strbuf_addch(result, ')');
+out:
 	free(place);
 	return ret;
 }
 
+/* Show an event */
+static int show_perf_probe_event(struct perf_probe_event *pev,
+				 const char *module, bool use_stdout)
+{
+	struct strbuf buf = STRBUF_INIT;
+	int ret;
+
+	ret = perf_probe_event__sprintf(pev, module, &buf);
+	if (ret >= 0) {
+		if (use_stdout)
+			printf("%s\n", buf.buf);
+		else
+			pr_info("%s\n", buf.buf);
+	}
+	strbuf_release(&buf);
+
+	return ret;
+}
+
 static bool filter_probe_trace_event(struct probe_trace_event *tev,
 				     struct strfilter *filter)
 {
@@ -2200,9 +2220,10 @@ static int __show_perf_probe_events(int fd, bool is_kprobe,
 				goto next;
 			ret = convert_to_perf_probe_event(&tev, &pev,
 								is_kprobe);
-			if (ret >= 0)
-				ret = show_perf_probe_event(&pev,
-							    tev.point.module);
+			if (ret < 0)
+				goto next;
+			ret = show_perf_probe_event(&pev, tev.point.module,
+						    false);
 		}
 next:
 		clear_perf_probe_event(&pev);
@@ -2463,7 +2484,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		group = pev->group;
 		pev->event = tev->event;
 		pev->group = tev->group;
-		show_perf_probe_event(pev, tev->point.module);
+		show_perf_probe_event(pev, tev->point.module, true);
 		/* Trick here - restore current event/group */
 		pev->event = (char *)event;
 		pev->group = (char *)group;

Re: [PATCH perf/core 1/4] [BUGFIX] perf probe: List probes in stdout

[Arnaldo Carvalho de Melo]

Em Fri, Jun 12, 2015 at 02:08:13PM +0900, Masami Hiramatsu escreveu:
> Since commit 5e17b28f1e24 ("perf probe: Add --quiet option to
> suppress output result message") have replaced printf with pr_info,
> perf probe -l outputs its result in stderr. However, that is not
> what the commit expected.
> e.g.
>   -----
>   # perf probe -l > /dev/null
>     probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
>   -----
> With this fix,
>   -----
>   # perf probe -l > list
>   # cat list
>     probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)

Nope, with this fix just applied:

[acme@zoo linux]$ rm -f ~/bin/perf;hash -r;type perf;rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ; make -C tools/perf O=/tmp/build/perf install-bin > /dev/null; type perf
bash: type: perf: not found
  PERF_VERSION = 4.1.rc5.g3f046e
perf is /home/acme/bin/perf
[acme@zoo linux]$ sudo ~acme/bin/perf probe -l > /tmp/bla
  probe:SYSC_perf_event_open (on SYSC_perf_event_open@linux/kernel/events/core.c)
[acme@zoo linux]$ cat /tmp/bla
[acme@zoo linux]$ git log --oneline | head -1
3f046ed885e1 perf probe: List probes in stdout
acme@zoo linux]$

I see there is one place where this 'use_stdout' thing is always 'false', is
that the problem? Do we really need to have this complexity? Why not plain use
fprintf(stdout)? This is not a informative message, this is the command output,
right?

- Arnaldo

>   -----
> Of course, --quiet(-q) still works on --add/--del.
>   -----
>   # perf probe -q vfs_write
>   # perf probe -l
>     probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
>     probe:vfs_write      (on vfs_write@ksrc/linux-3/fs/read_write.c)
>   -----
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Reported-by: Naohiro Aota <naota@elisp.net>
> ---
>  tools/perf/util/probe-event.c |   49 +++++++++++++++++++++++++++++------------
>  1 file changed, 35 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index d4cf50b..710830c 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -2126,9 +2126,9 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
>  	return NULL;
>  }
>  
> -/* Show an event */
> -static int show_perf_probe_event(struct perf_probe_event *pev,
> -				 const char *module)
> +static int perf_probe_event__sprintf(struct perf_probe_event *pev,
> +				     const char *module,
> +				     struct strbuf *result)
>  {
>  	int i, ret;
>  	char buf[128];
> @@ -2141,27 +2141,47 @@ static int show_perf_probe_event(struct perf_probe_event *pev,
>  
>  	ret = e_snprintf(buf, 128, "%s:%s", pev->group, pev->event);
>  	if (ret < 0)
> -		return ret;
> +		goto out;
>  
> -	pr_info("  %-20s (on %s", buf, place);
> +	strbuf_addf(result, "  %-20s (on %s", buf, place);
>  	if (module)
> -		pr_info(" in %s", module);
> +		strbuf_addf(result, " in %s", module);
>  
>  	if (pev->nargs > 0) {
> -		pr_info(" with");
> +		strbuf_addstr(result, " with");
>  		for (i = 0; i < pev->nargs; i++) {
>  			ret = synthesize_perf_probe_arg(&pev->args[i],
>  							buf, 128);
>  			if (ret < 0)
> -				break;
> -			pr_info(" %s", buf);
> +				goto out;
> +			strbuf_addf(result, " %s", buf);
>  		}
>  	}
> -	pr_info(")\n");
> +	strbuf_addch(result, ')');
> +out:
>  	free(place);
>  	return ret;
>  }
>  
> +/* Show an event */
> +static int show_perf_probe_event(struct perf_probe_event *pev,
> +				 const char *module, bool use_stdout)
> +{
> +	struct strbuf buf = STRBUF_INIT;
> +	int ret;
> +
> +	ret = perf_probe_event__sprintf(pev, module, &buf);
> +	if (ret >= 0) {
> +		if (use_stdout)
> +			printf("%s\n", buf.buf);
> +		else
> +			pr_info("%s\n", buf.buf);
> +	}
> +	strbuf_release(&buf);
> +
> +	return ret;
> +}
> +
>  static bool filter_probe_trace_event(struct probe_trace_event *tev,
>  				     struct strfilter *filter)
>  {
> @@ -2200,9 +2220,10 @@ static int __show_perf_probe_events(int fd, bool is_kprobe,
>  				goto next;
>  			ret = convert_to_perf_probe_event(&tev, &pev,
>  								is_kprobe);
> -			if (ret >= 0)
> -				ret = show_perf_probe_event(&pev,
> -							    tev.point.module);
> +			if (ret < 0)
> +				goto next;
> +			ret = show_perf_probe_event(&pev, tev.point.module,
> +						    false);
>  		}
>  next:
>  		clear_perf_probe_event(&pev);
> @@ -2463,7 +2484,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  		group = pev->group;
>  		pev->event = tev->event;
>  		pev->group = tev->group;
> -		show_perf_probe_event(pev, tev->point.module);
> +		show_perf_probe_event(pev, tev->point.module, true);
>  		/* Trick here - restore current event/group */
>  		pev->event = (char *)event;
>  		pev->group = (char *)group;
> 

Re: [PATCH perf/core 1/4] [BUGFIX] perf probe: List probes in stdout

[Masami Hiramatsu]

On 2015/06/13 4:12, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 12, 2015 at 02:08:13PM +0900, Masami Hiramatsu escreveu:
>> Since commit 5e17b28f1e24 ("perf probe: Add --quiet option to
>> suppress output result message") have replaced printf with pr_info,
>> perf probe -l outputs its result in stderr. However, that is not
>> what the commit expected.
>> e.g.
>>   -----
>>   # perf probe -l > /dev/null
>>     probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
>>   -----
>> With this fix,
>>   -----
>>   # perf probe -l > list
>>   # cat list
>>     probe:vfs_read       (on vfs_read@ksrc/linux-3/fs/read_write.c)
> 
> Nope, with this fix just applied:

Oops, I missed to rewrite...

[...]
>> @@ -2200,9 +2220,10 @@ static int __show_perf_probe_events(int fd, bool is_kprobe,
>>  				goto next;
>>  			ret = convert_to_perf_probe_event(&tev, &pev,
>>  								is_kprobe);
>> -			if (ret >= 0)
>> -				ret = show_perf_probe_event(&pev,
>> -							    tev.point.module);
>> +			if (ret < 0)
>> +				goto next;
>> +			ret = show_perf_probe_event(&pev, tev.point.module,
>> +						    false);
>>  		}
>>  next:
>>  		clear_perf_probe_event(&pev);
>> @@ -2463,7 +2484,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>>  		group = pev->group;
>>  		pev->event = tev->event;
>>  		pev->group = tev->group;
>> -		show_perf_probe_event(pev, tev->point.module);
>> +		show_perf_probe_event(pev, tev->point.module, true);
>>  		/* Trick here - restore current event/group */
>>  		pev->event = (char *)event;
>>  		pev->group = (char *)group;
>>
> 

These true/false are opposite!


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH perf/core 2/4] [BUGFIX] perf probe: Cut off the postfixes from event name

[Masami Hiramatsu]

Cut off the postfixes which gcc added for optimized routines
from the event name automatically generated from symbol name,
since *probe-events doesn't accept it.
Those symbols will be used if we don't use debuginfo to find
target functions.

E.g. without this fix;
  -----
  # perf probe -va alloc_buf.isra.23
  probe-definition(0): alloc_buf.isra.23
  symbol:alloc_buf.isra.23 file:(null) line:0 offset:0 return:0 lazy:(null)
  [...]
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  Added new event:
  Writing event: p:probe/alloc_buf.isra.23 _text+4869328
  Failed to write event: Invalid argument
    Error: Failed to add events. Reason: Invalid argument (Code: -22)
  -----
With this fix;
  -----
  perf probe -va alloc_buf.isra.23
  probe-definition(0): alloc_buf.isra.23
  symbol:alloc_buf.isra.23 file:(null) line:0 offset:0 return:0 lazy:(null)
  [...]
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  Added new event:
  Writing event: p:probe/alloc_buf _text+4869328
    probe:alloc_buf      (on alloc_buf.isra.23)

  You can now use it in all perf tools, such as:

  	perf record -e probe:alloc_buf -aR sleep 1

  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 710830c..7aa3efe 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2337,6 +2337,7 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 			      struct strlist *namelist, bool allow_suffix)
 {
 	int i, ret;
+	char *p;
 
 	if (*base == '.')
 		base++;
@@ -2347,6 +2348,10 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 		pr_debug("snprintf() failed: %d\n", ret);
 		return ret;
 	}
+	/* Cut off the postfixes (e.g. .const, .isra)*/
+	p = strchr(buf, '.');
+	if (p && p != buf)
+		*p = '\0';
 	if (!strlist__has_entry(namelist, buf))
 		return 0;
 

Re: [PATCH perf/core 2/4] [BUGFIX] perf probe: Cut off the postfixes from event name

[Arnaldo Carvalho de Melo]

Em Fri, Jun 12, 2015 at 02:08:20PM +0900, Masami Hiramatsu escreveu:
> Cut off the postfixes which gcc added for optimized routines
> from the event name automatically generated from symbol name,
> since *probe-events doesn't accept it.
> Those symbols will be used if we don't use debuginfo to find
> target functions.

Tested and applied.

- Arnaldo

[tip:perf/core] perf probe: Cut off the gcc optimization postfixes from function name

[tip-bot for Masami Hiramatsu]

Commit-ID:  35a23ff928b066b00a826d0a9ed9411b8ab479ef
Gitweb:     http://git.kernel.org/tip/35a23ff928b066b00a826d0a9ed9411b8ab479ef
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Fri, 12 Jun 2015 14:08:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 12 Jun 2015 16:14:48 -0300

perf probe: Cut off the gcc optimization postfixes from function name

Cut off the postfixes which gcc added for optimized routines from the
event name automatically generated from symbol name, since *probe-events
doesn't accept it.  Those symbols will be used if we don't use debuginfo
to find target functions.

E.g. without this fix;
  -----
  # perf probe -va alloc_buf.isra.23
  probe-definition(0): alloc_buf.isra.23
  symbol:alloc_buf.isra.23 file:(null) line:0 offset:0 return:0 lazy:(null)
  [...]
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  Added new event:
  Writing event: p:probe/alloc_buf.isra.23 _text+4869328
  Failed to write event: Invalid argument
    Error: Failed to add events. Reason: Invalid argument (Code: -22)
  -----
With this fix;
  -----
  perf probe -va alloc_buf.isra.23
  probe-definition(0): alloc_buf.isra.23
  symbol:alloc_buf.isra.23 file:(null) line:0 offset:0 return:0 lazy:(null)
  [...]
  Opening /sys/kernel/debug/tracing/kprobe_events write=1
  Added new event:
  Writing event: p:probe/alloc_buf _text+4869328
    probe:alloc_buf      (on alloc_buf.isra.23)

  You can now use it in all perf tools, such as:

  	perf record -e probe:alloc_buf -aR sleep 1

  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Naohiro Aota <naota@elisp.net>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20150612050820.20548.41625.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/probe-event.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index d4cf50b..daa24a2 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2316,6 +2316,7 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 			      struct strlist *namelist, bool allow_suffix)
 {
 	int i, ret;
+	char *p;
 
 	if (*base == '.')
 		base++;
@@ -2326,6 +2327,10 @@ static int get_new_event_name(char *buf, size_t len, const char *base,
 		pr_debug("snprintf() failed: %d\n", ret);
 		return ret;
 	}
+	/* Cut off the postfixes (e.g. .const, .isra)*/
+	p = strchr(buf, '.');
+	if (p && p != buf)
+		*p = '\0';
 	if (!strlist__has_entry(namelist, buf))
 		return 0;
 

[PATCH perf/core 3/4] [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map

[Masami Hiramatsu]

Fix to check both of non-exist symbols and kprobe blacklist symbols at
symbol-map based converting too.

E.g. without this fix, perf-probe failes to write the event.
  ----
  # perf probe vfs_caches_init_early
  Added new event:
  Failed to write event: Invalid argument
    Error: Failed to add events.
  ----

This fixes it to catch the error before adding probes.
  ----
  # perf probe vfs_caches_init_early
  vfs_caches_init_early is out of .text, skip it.
    Error: Failed to add events.
  ----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |  113 ++++++++++++++++++++++++++---------------
 1 file changed, 71 insertions(+), 42 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 7aa3efe..aa2479e 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -246,6 +246,20 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 		clear_probe_trace_event(tevs + i);
 }
 
+static bool kprobe_blacklist__listed(unsigned long address);
+static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
+{
+	/* Get the address of _etext for checking non-probable text symbol */
+	if (kernel_get_symbol_address_by_name("_etext", false) < address)
+		pr_warning("%s is out of .text, skip it.\n", symbol);
+	else if (kprobe_blacklist__listed(address))
+		pr_warning("%s is blacklisted function, skip it.\n", symbol);
+	else
+		return false;
+
+	return true;
+}
+
 #ifdef HAVE_DWARF_SUPPORT
 
 static int kernel_get_module_dso(const char *module, struct dso **pdso)
@@ -559,7 +573,6 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 					   bool uprobe)
 {
 	struct ref_reloc_sym *reloc_sym;
-	u64 etext_addr;
 	char *tmp;
 	int i, skipped = 0;
 
@@ -575,31 +588,28 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
 		pr_warning("Relocated base symbol is not found!\n");
 		return -EINVAL;
 	}
-	/* Get the address of _etext for checking non-probable text symbol */
-	etext_addr = kernel_get_symbol_address_by_name("_etext", false);
 
 	for (i = 0; i < ntevs; i++) {
-		if (tevs[i].point.address && !tevs[i].point.retprobe) {
-			/* If we found a wrong one, mark it by NULL symbol */
-			if (etext_addr < tevs[i].point.address) {
-				pr_warning("%s+%lu is out of .text, skip it.\n",
-				   tevs[i].point.symbol, tevs[i].point.offset);
-				tmp = NULL;
-				skipped++;
-			} else {
-				tmp = strdup(reloc_sym->name);
-				if (!tmp)
-					return -ENOMEM;
-			}
-			/* If we have no realname, use symbol for it */
-			if (!tevs[i].point.realname)
-				tevs[i].point.realname = tevs[i].point.symbol;
-			else
-				free(tevs[i].point.symbol);
-			tevs[i].point.symbol = tmp;
-			tevs[i].point.offset = tevs[i].point.address -
-					       reloc_sym->unrelocated_addr;
+		if (!tevs[i].point.address || tevs[i].point.retprobe)
+			continue;
+		/* If we found a wrong one, mark it by NULL symbol */
+		if (kprobe_warn_out_range(tevs[i].point.symbol,
+					  tevs[i].point.address)) {
+			tmp = NULL;
+			skipped++;
+		} else {
+			tmp = strdup(reloc_sym->name);
+			if (!tmp)
+				return -ENOMEM;
 		}
+		/* If we have no realname, use symbol for it */
+		if (!tevs[i].point.realname)
+			tevs[i].point.realname = tevs[i].point.symbol;
+		else
+			free(tevs[i].point.symbol);
+		tevs[i].point.symbol = tmp;
+		tevs[i].point.offset = tevs[i].point.address -
+				       reloc_sym->unrelocated_addr;
 	}
 	return skipped;
 }
@@ -2126,6 +2136,27 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
 	return NULL;
 }
 
+static LIST_HEAD(kprobe_blacklist);
+
+static void kprobe_blacklist__init(void)
+{
+	if (!list_empty(&kprobe_blacklist))
+		return;
+
+	if (kprobe_blacklist__load(&kprobe_blacklist) < 0)
+		pr_debug("No kprobe blacklist support, ignored\n");
+}
+
+static void kprobe_blacklist__release(void)
+{
+	kprobe_blacklist__delete(&kprobe_blacklist);
+}
+
+static bool kprobe_blacklist__listed(unsigned long address)
+{
+	return !!kprobe_blacklist__find_by_address(&kprobe_blacklist, address);
+}
+
 static int perf_probe_event__sprintf(struct perf_probe_event *pev,
 				     const char *module,
 				     struct strbuf *result)
@@ -2409,8 +2440,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	char buf[64];
 	const char *event, *group;
 	struct strlist *namelist;
-	LIST_HEAD(blacklist);
-	struct kprobe_blacklist_node *node;
 	bool safename;
 
 	if (pev->uprobes)
@@ -2430,28 +2459,15 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		ret = -ENOMEM;
 		goto close_out;
 	}
-	/* Get kprobe blacklist if exists */
-	if (!pev->uprobes) {
-		ret = kprobe_blacklist__load(&blacklist);
-		if (ret < 0)
-			pr_debug("No kprobe blacklist support, ignored\n");
-	}
 
 	safename = (pev->point.function && !strisglob(pev->point.function));
 	ret = 0;
 	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
 	for (i = 0; i < ntevs; i++) {
 		tev = &tevs[i];
-		/* Skip if the symbol is out of .text (marked previously) */
+		/* Skip if the symbol is out of .text or blacklisted */
 		if (!tev->point.symbol)
 			continue;
-		/* Ensure that the address is NOT blacklisted */
-		node = kprobe_blacklist__find_by_address(&blacklist,
-							 tev->point.address);
-		if (node) {
-			pr_warning("Warning: Skipped probing on blacklisted function: %s\n", node->symbol);
-			continue;
-		}
 
 		if (pev->event)
 			event = pev->event;
@@ -2513,7 +2529,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 			 tev->event);
 	}
 
-	kprobe_blacklist__delete(&blacklist);
 	strlist__delete(namelist);
 close_out:
 	close(fd);
@@ -2563,7 +2578,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 	struct perf_probe_point *pp = &pev->point;
 	struct probe_trace_point *tp;
 	int num_matched_functions;
-	int ret, i, j;
+	int ret, i, j, skipped = 0;
 
 	map = get_target_map(pev->target, pev->uprobes);
 	if (!map) {
@@ -2631,7 +2646,12 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 		}
 		/* Add one probe point */
 		tp->address = map->unmap_ip(map, sym->start) + pp->offset;
-		if (reloc_sym) {
+		/* If we found a wrong one, mark it by NULL symbol */
+		if (!pev->uprobes &&
+		    kprobe_warn_out_range(sym->name, tp->address)) {
+			tp->symbol = NULL;	/* Skip it */
+			skipped++;
+		} else if (reloc_sym) {
 			tp->symbol = strdup_or_goto(reloc_sym->name, nomem_out);
 			tp->offset = tp->address - reloc_sym->addr;
 		} else {
@@ -2667,6 +2687,10 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
 		}
 		arch__fix_tev_from_maps(pev, tev, map);
 	}
+	if (ret == skipped) {
+		ret = -ENOENT;
+		goto err_out;
+	}
 
 out:
 	put_target_map(map, pev->uprobes);
@@ -2737,6 +2761,9 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 	/* Loop 1: convert all events */
 	for (i = 0; i < npevs; i++) {
 		pkgs[i].pev = &pevs[i];
+		/* Init kprobe blacklist if needed */
+		if (!pkgs[i].pev->uprobes)
+			kprobe_blacklist__init();
 		/* Convert with or without debuginfo */
 		ret  = convert_to_probe_trace_events(pkgs[i].pev,
 						     &pkgs[i].tevs);
@@ -2744,6 +2771,8 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
 			goto end;
 		pkgs[i].ntevs = ret;
 	}
+	/* This just release blacklist only if allocated */
+	kprobe_blacklist__release();
 
 	/* Loop 2: add all events */
 	for (i = 0; i < npevs; i++) {

Re: [PATCH perf/core 3/4] [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map

[Arnaldo Carvalho de Melo]

Em Fri, Jun 12, 2015 at 02:08:27PM +0900, Masami Hiramatsu escreveu:
> Fix to check both of non-exist symbols and kprobe blacklist symbols at
> symbol-map based converting too.
> 
> E.g. without this fix, perf-probe failes to write the event.
>   ----
>   # perf probe vfs_caches_init_early
>   Added new event:
>   Failed to write event: Invalid argument
>     Error: Failed to add events.
>   ----
> 
> This fixes it to catch the error before adding probes.
>   ----
>   # perf probe vfs_caches_init_early
>   vfs_caches_init_early is out of .text, skip it.
>     Error: Failed to add events.
>   ----

Not applying to my perf/core branch, maybe it needs the first patch in
this series, that is not applied so far?

- Arnaldo
 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/util/probe-event.c |  113 ++++++++++++++++++++++++++---------------
>  1 file changed, 71 insertions(+), 42 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index 7aa3efe..aa2479e 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -246,6 +246,20 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
>  		clear_probe_trace_event(tevs + i);
>  }
>  
> +static bool kprobe_blacklist__listed(unsigned long address);
> +static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
> +{
> +	/* Get the address of _etext for checking non-probable text symbol */
> +	if (kernel_get_symbol_address_by_name("_etext", false) < address)
> +		pr_warning("%s is out of .text, skip it.\n", symbol);
> +	else if (kprobe_blacklist__listed(address))
> +		pr_warning("%s is blacklisted function, skip it.\n", symbol);
> +	else
> +		return false;
> +
> +	return true;
> +}
> +
>  #ifdef HAVE_DWARF_SUPPORT
>  
>  static int kernel_get_module_dso(const char *module, struct dso **pdso)
> @@ -559,7 +573,6 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
>  					   bool uprobe)
>  {
>  	struct ref_reloc_sym *reloc_sym;
> -	u64 etext_addr;
>  	char *tmp;
>  	int i, skipped = 0;
>  
> @@ -575,31 +588,28 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
>  		pr_warning("Relocated base symbol is not found!\n");
>  		return -EINVAL;
>  	}
> -	/* Get the address of _etext for checking non-probable text symbol */
> -	etext_addr = kernel_get_symbol_address_by_name("_etext", false);
>  
>  	for (i = 0; i < ntevs; i++) {
> -		if (tevs[i].point.address && !tevs[i].point.retprobe) {
> -			/* If we found a wrong one, mark it by NULL symbol */
> -			if (etext_addr < tevs[i].point.address) {
> -				pr_warning("%s+%lu is out of .text, skip it.\n",
> -				   tevs[i].point.symbol, tevs[i].point.offset);
> -				tmp = NULL;
> -				skipped++;
> -			} else {
> -				tmp = strdup(reloc_sym->name);
> -				if (!tmp)
> -					return -ENOMEM;
> -			}
> -			/* If we have no realname, use symbol for it */
> -			if (!tevs[i].point.realname)
> -				tevs[i].point.realname = tevs[i].point.symbol;
> -			else
> -				free(tevs[i].point.symbol);
> -			tevs[i].point.symbol = tmp;
> -			tevs[i].point.offset = tevs[i].point.address -
> -					       reloc_sym->unrelocated_addr;
> +		if (!tevs[i].point.address || tevs[i].point.retprobe)
> +			continue;
> +		/* If we found a wrong one, mark it by NULL symbol */
> +		if (kprobe_warn_out_range(tevs[i].point.symbol,
> +					  tevs[i].point.address)) {
> +			tmp = NULL;
> +			skipped++;
> +		} else {
> +			tmp = strdup(reloc_sym->name);
> +			if (!tmp)
> +				return -ENOMEM;
>  		}
> +		/* If we have no realname, use symbol for it */
> +		if (!tevs[i].point.realname)
> +			tevs[i].point.realname = tevs[i].point.symbol;
> +		else
> +			free(tevs[i].point.symbol);
> +		tevs[i].point.symbol = tmp;
> +		tevs[i].point.offset = tevs[i].point.address -
> +				       reloc_sym->unrelocated_addr;
>  	}
>  	return skipped;
>  }
> @@ -2126,6 +2136,27 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
>  	return NULL;
>  }
>  
> +static LIST_HEAD(kprobe_blacklist);
> +
> +static void kprobe_blacklist__init(void)
> +{
> +	if (!list_empty(&kprobe_blacklist))
> +		return;
> +
> +	if (kprobe_blacklist__load(&kprobe_blacklist) < 0)
> +		pr_debug("No kprobe blacklist support, ignored\n");
> +}
> +
> +static void kprobe_blacklist__release(void)
> +{
> +	kprobe_blacklist__delete(&kprobe_blacklist);
> +}
> +
> +static bool kprobe_blacklist__listed(unsigned long address)
> +{
> +	return !!kprobe_blacklist__find_by_address(&kprobe_blacklist, address);
> +}
> +
>  static int perf_probe_event__sprintf(struct perf_probe_event *pev,
>  				     const char *module,
>  				     struct strbuf *result)
> @@ -2409,8 +2440,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  	char buf[64];
>  	const char *event, *group;
>  	struct strlist *namelist;
> -	LIST_HEAD(blacklist);
> -	struct kprobe_blacklist_node *node;
>  	bool safename;
>  
>  	if (pev->uprobes)
> @@ -2430,28 +2459,15 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  		ret = -ENOMEM;
>  		goto close_out;
>  	}
> -	/* Get kprobe blacklist if exists */
> -	if (!pev->uprobes) {
> -		ret = kprobe_blacklist__load(&blacklist);
> -		if (ret < 0)
> -			pr_debug("No kprobe blacklist support, ignored\n");
> -	}
>  
>  	safename = (pev->point.function && !strisglob(pev->point.function));
>  	ret = 0;
>  	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
>  	for (i = 0; i < ntevs; i++) {
>  		tev = &tevs[i];
> -		/* Skip if the symbol is out of .text (marked previously) */
> +		/* Skip if the symbol is out of .text or blacklisted */
>  		if (!tev->point.symbol)
>  			continue;
> -		/* Ensure that the address is NOT blacklisted */
> -		node = kprobe_blacklist__find_by_address(&blacklist,
> -							 tev->point.address);
> -		if (node) {
> -			pr_warning("Warning: Skipped probing on blacklisted function: %s\n", node->symbol);
> -			continue;
> -		}
>  
>  		if (pev->event)
>  			event = pev->event;
> @@ -2513,7 +2529,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>  			 tev->event);
>  	}
>  
> -	kprobe_blacklist__delete(&blacklist);
>  	strlist__delete(namelist);
>  close_out:
>  	close(fd);
> @@ -2563,7 +2578,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>  	struct perf_probe_point *pp = &pev->point;
>  	struct probe_trace_point *tp;
>  	int num_matched_functions;
> -	int ret, i, j;
> +	int ret, i, j, skipped = 0;
>  
>  	map = get_target_map(pev->target, pev->uprobes);
>  	if (!map) {
> @@ -2631,7 +2646,12 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>  		}
>  		/* Add one probe point */
>  		tp->address = map->unmap_ip(map, sym->start) + pp->offset;
> -		if (reloc_sym) {
> +		/* If we found a wrong one, mark it by NULL symbol */
> +		if (!pev->uprobes &&
> +		    kprobe_warn_out_range(sym->name, tp->address)) {
> +			tp->symbol = NULL;	/* Skip it */
> +			skipped++;
> +		} else if (reloc_sym) {
>  			tp->symbol = strdup_or_goto(reloc_sym->name, nomem_out);
>  			tp->offset = tp->address - reloc_sym->addr;
>  		} else {
> @@ -2667,6 +2687,10 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>  		}
>  		arch__fix_tev_from_maps(pev, tev, map);
>  	}
> +	if (ret == skipped) {
> +		ret = -ENOENT;
> +		goto err_out;
> +	}
>  
>  out:
>  	put_target_map(map, pev->uprobes);
> @@ -2737,6 +2761,9 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  	/* Loop 1: convert all events */
>  	for (i = 0; i < npevs; i++) {
>  		pkgs[i].pev = &pevs[i];
> +		/* Init kprobe blacklist if needed */
> +		if (!pkgs[i].pev->uprobes)
> +			kprobe_blacklist__init();
>  		/* Convert with or without debuginfo */
>  		ret  = convert_to_probe_trace_events(pkgs[i].pev,
>  						     &pkgs[i].tevs);
> @@ -2744,6 +2771,8 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>  			goto end;
>  		pkgs[i].ntevs = ret;
>  	}
> +	/* This just release blacklist only if allocated */
> +	kprobe_blacklist__release();
>  
>  	/* Loop 2: add all events */
>  	for (i = 0; i < npevs; i++) {
> 

Re: [PATCH perf/core 3/4] [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map

[Masami Hiramatsu]

On 2015/06/13 4:17, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 12, 2015 at 02:08:27PM +0900, Masami Hiramatsu escreveu:
>> Fix to check both of non-exist symbols and kprobe blacklist symbols at
>> symbol-map based converting too.
>>
>> E.g. without this fix, perf-probe failes to write the event.
>>   ----
>>   # perf probe vfs_caches_init_early
>>   Added new event:
>>   Failed to write event: Invalid argument
>>     Error: Failed to add events.
>>   ----
>>
>> This fixes it to catch the error before adding probes.
>>   ----
>>   # perf probe vfs_caches_init_early
>>   vfs_caches_init_early is out of .text, skip it.
>>     Error: Failed to add events.
>>   ----
> 
> Not applying to my perf/core branch, maybe it needs the first patch in
> this series, that is not applied so far?

Yes, I'll resend the series without 2/4, with fixing previous bug...

Thank you!!

> 
> - Arnaldo
>  
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/util/probe-event.c |  113 ++++++++++++++++++++++++++---------------
>>  1 file changed, 71 insertions(+), 42 deletions(-)
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index 7aa3efe..aa2479e 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -246,6 +246,20 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
>>  		clear_probe_trace_event(tevs + i);
>>  }
>>  
>> +static bool kprobe_blacklist__listed(unsigned long address);
>> +static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
>> +{
>> +	/* Get the address of _etext for checking non-probable text symbol */
>> +	if (kernel_get_symbol_address_by_name("_etext", false) < address)
>> +		pr_warning("%s is out of .text, skip it.\n", symbol);
>> +	else if (kprobe_blacklist__listed(address))
>> +		pr_warning("%s is blacklisted function, skip it.\n", symbol);
>> +	else
>> +		return false;
>> +
>> +	return true;
>> +}
>> +
>>  #ifdef HAVE_DWARF_SUPPORT
>>  
>>  static int kernel_get_module_dso(const char *module, struct dso **pdso)
>> @@ -559,7 +573,6 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
>>  					   bool uprobe)
>>  {
>>  	struct ref_reloc_sym *reloc_sym;
>> -	u64 etext_addr;
>>  	char *tmp;
>>  	int i, skipped = 0;
>>  
>> @@ -575,31 +588,28 @@ static int post_process_probe_trace_events(struct probe_trace_event *tevs,
>>  		pr_warning("Relocated base symbol is not found!\n");
>>  		return -EINVAL;
>>  	}
>> -	/* Get the address of _etext for checking non-probable text symbol */
>> -	etext_addr = kernel_get_symbol_address_by_name("_etext", false);
>>  
>>  	for (i = 0; i < ntevs; i++) {
>> -		if (tevs[i].point.address && !tevs[i].point.retprobe) {
>> -			/* If we found a wrong one, mark it by NULL symbol */
>> -			if (etext_addr < tevs[i].point.address) {
>> -				pr_warning("%s+%lu is out of .text, skip it.\n",
>> -				   tevs[i].point.symbol, tevs[i].point.offset);
>> -				tmp = NULL;
>> -				skipped++;
>> -			} else {
>> -				tmp = strdup(reloc_sym->name);
>> -				if (!tmp)
>> -					return -ENOMEM;
>> -			}
>> -			/* If we have no realname, use symbol for it */
>> -			if (!tevs[i].point.realname)
>> -				tevs[i].point.realname = tevs[i].point.symbol;
>> -			else
>> -				free(tevs[i].point.symbol);
>> -			tevs[i].point.symbol = tmp;
>> -			tevs[i].point.offset = tevs[i].point.address -
>> -					       reloc_sym->unrelocated_addr;
>> +		if (!tevs[i].point.address || tevs[i].point.retprobe)
>> +			continue;
>> +		/* If we found a wrong one, mark it by NULL symbol */
>> +		if (kprobe_warn_out_range(tevs[i].point.symbol,
>> +					  tevs[i].point.address)) {
>> +			tmp = NULL;
>> +			skipped++;
>> +		} else {
>> +			tmp = strdup(reloc_sym->name);
>> +			if (!tmp)
>> +				return -ENOMEM;
>>  		}
>> +		/* If we have no realname, use symbol for it */
>> +		if (!tevs[i].point.realname)
>> +			tevs[i].point.realname = tevs[i].point.symbol;
>> +		else
>> +			free(tevs[i].point.symbol);
>> +		tevs[i].point.symbol = tmp;
>> +		tevs[i].point.offset = tevs[i].point.address -
>> +				       reloc_sym->unrelocated_addr;
>>  	}
>>  	return skipped;
>>  }
>> @@ -2126,6 +2136,27 @@ kprobe_blacklist__find_by_address(struct list_head *blacklist,
>>  	return NULL;
>>  }
>>  
>> +static LIST_HEAD(kprobe_blacklist);
>> +
>> +static void kprobe_blacklist__init(void)
>> +{
>> +	if (!list_empty(&kprobe_blacklist))
>> +		return;
>> +
>> +	if (kprobe_blacklist__load(&kprobe_blacklist) < 0)
>> +		pr_debug("No kprobe blacklist support, ignored\n");
>> +}
>> +
>> +static void kprobe_blacklist__release(void)
>> +{
>> +	kprobe_blacklist__delete(&kprobe_blacklist);
>> +}
>> +
>> +static bool kprobe_blacklist__listed(unsigned long address)
>> +{
>> +	return !!kprobe_blacklist__find_by_address(&kprobe_blacklist, address);
>> +}
>> +
>>  static int perf_probe_event__sprintf(struct perf_probe_event *pev,
>>  				     const char *module,
>>  				     struct strbuf *result)
>> @@ -2409,8 +2440,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>>  	char buf[64];
>>  	const char *event, *group;
>>  	struct strlist *namelist;
>> -	LIST_HEAD(blacklist);
>> -	struct kprobe_blacklist_node *node;
>>  	bool safename;
>>  
>>  	if (pev->uprobes)
>> @@ -2430,28 +2459,15 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>>  		ret = -ENOMEM;
>>  		goto close_out;
>>  	}
>> -	/* Get kprobe blacklist if exists */
>> -	if (!pev->uprobes) {
>> -		ret = kprobe_blacklist__load(&blacklist);
>> -		if (ret < 0)
>> -			pr_debug("No kprobe blacklist support, ignored\n");
>> -	}
>>  
>>  	safename = (pev->point.function && !strisglob(pev->point.function));
>>  	ret = 0;
>>  	pr_info("Added new event%s\n", (ntevs > 1) ? "s:" : ":");
>>  	for (i = 0; i < ntevs; i++) {
>>  		tev = &tevs[i];
>> -		/* Skip if the symbol is out of .text (marked previously) */
>> +		/* Skip if the symbol is out of .text or blacklisted */
>>  		if (!tev->point.symbol)
>>  			continue;
>> -		/* Ensure that the address is NOT blacklisted */
>> -		node = kprobe_blacklist__find_by_address(&blacklist,
>> -							 tev->point.address);
>> -		if (node) {
>> -			pr_warning("Warning: Skipped probing on blacklisted function: %s\n", node->symbol);
>> -			continue;
>> -		}
>>  
>>  		if (pev->event)
>>  			event = pev->event;
>> @@ -2513,7 +2529,6 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
>>  			 tev->event);
>>  	}
>>  
>> -	kprobe_blacklist__delete(&blacklist);
>>  	strlist__delete(namelist);
>>  close_out:
>>  	close(fd);
>> @@ -2563,7 +2578,7 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>>  	struct perf_probe_point *pp = &pev->point;
>>  	struct probe_trace_point *tp;
>>  	int num_matched_functions;
>> -	int ret, i, j;
>> +	int ret, i, j, skipped = 0;
>>  
>>  	map = get_target_map(pev->target, pev->uprobes);
>>  	if (!map) {
>> @@ -2631,7 +2646,12 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>>  		}
>>  		/* Add one probe point */
>>  		tp->address = map->unmap_ip(map, sym->start) + pp->offset;
>> -		if (reloc_sym) {
>> +		/* If we found a wrong one, mark it by NULL symbol */
>> +		if (!pev->uprobes &&
>> +		    kprobe_warn_out_range(sym->name, tp->address)) {
>> +			tp->symbol = NULL;	/* Skip it */
>> +			skipped++;
>> +		} else if (reloc_sym) {
>>  			tp->symbol = strdup_or_goto(reloc_sym->name, nomem_out);
>>  			tp->offset = tp->address - reloc_sym->addr;
>>  		} else {
>> @@ -2667,6 +2687,10 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
>>  		}
>>  		arch__fix_tev_from_maps(pev, tev, map);
>>  	}
>> +	if (ret == skipped) {
>> +		ret = -ENOENT;
>> +		goto err_out;
>> +	}
>>  
>>  out:
>>  	put_target_map(map, pev->uprobes);
>> @@ -2737,6 +2761,9 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>>  	/* Loop 1: convert all events */
>>  	for (i = 0; i < npevs; i++) {
>>  		pkgs[i].pev = &pevs[i];
>> +		/* Init kprobe blacklist if needed */
>> +		if (!pkgs[i].pev->uprobes)
>> +			kprobe_blacklist__init();
>>  		/* Convert with or without debuginfo */
>>  		ret  = convert_to_probe_trace_events(pkgs[i].pev,
>>  						     &pkgs[i].tevs);
>> @@ -2744,6 +2771,8 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs)
>>  			goto end;
>>  		pkgs[i].ntevs = ret;
>>  	}
>> +	/* This just release blacklist only if allocated */
>> +	kprobe_blacklist__release();
>>  
>>  	/* Loop 2: add all events */
>>  	for (i = 0; i < npevs; i++) {
>>
> 


-- 
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: masami.hiramatsu.pt@hitachi.com

[PATCH perf/core 4/4] [BUGFIX] perf probe: Show usage even if the last event is skipped

[Masami Hiramatsu]

When the last part of converted events are blacklisted or out-of-text,
those are skipped and perf probe doesn't show usage examples.
This fixes it to show the example even if the last part of event list
is skipped.

E.g. without this patch, events are added, but suddenly end;
  ----
  # perf probe vfs_*
  vfs_caches_init_early is out of .text, skip it.
  vfs_caches_init is out of .text, skip it.
  Added new events:
    probe:vfs_fallocate  (on vfs_*)
    probe:vfs_open       (on vfs_*)
  ...
    probe:vfs_dentry_acceptable (on vfs_*)
    probe:vfs_load_quota_inode (on vfs_*)
  #
  ----
With this fix;
  ----
  # perf probe vfs_*
  vfs_caches_init_early is out of .text, skip it.
  vfs_caches_init is out of .text, skip it.
  Added new events:
    probe:vfs_fallocate  (on vfs_*)
  ...
    probe:vfs_load_quota_inode (on vfs_*)

  You can now use it in all perf tools, such as:

	perf record -e probe:vfs_load_quota_inode -aR sleep 1

  #
  ----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/probe-event.c |   35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index aa2479e..626bc6c5 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -2157,7 +2157,8 @@ static bool kprobe_blacklist__listed(unsigned long address)
 	return !!kprobe_blacklist__find_by_address(&kprobe_blacklist, address);
 }
 
-static int perf_probe_event__sprintf(struct perf_probe_event *pev,
+static int perf_probe_event__sprintf(const char *group, const char *event,
+				     struct perf_probe_event *pev,
 				     const char *module,
 				     struct strbuf *result)
 {
@@ -2170,7 +2171,7 @@ static int perf_probe_event__sprintf(struct perf_probe_event *pev,
 	if (!place)
 		return -EINVAL;
 
-	ret = e_snprintf(buf, 128, "%s:%s", pev->group, pev->event);
+	ret = e_snprintf(buf, 128, "%s:%s", group, event);
 	if (ret < 0)
 		goto out;
 
@@ -2195,13 +2196,14 @@ out:
 }
 
 /* Show an event */
-static int show_perf_probe_event(struct perf_probe_event *pev,
+static int show_perf_probe_event(const char *group, const char *event,
+				 struct perf_probe_event *pev,
 				 const char *module, bool use_stdout)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
 
-	ret = perf_probe_event__sprintf(pev, module, &buf);
+	ret = perf_probe_event__sprintf(group, event, pev, module, &buf);
 	if (ret >= 0) {
 		if (use_stdout)
 			printf("%s\n", buf.buf);
@@ -2253,7 +2255,8 @@ static int __show_perf_probe_events(int fd, bool is_kprobe,
 								is_kprobe);
 			if (ret < 0)
 				goto next;
-			ret = show_perf_probe_event(&pev, tev.point.module,
+			ret = show_perf_probe_event(pev.group, pev.event,
+						    &pev, tev.point.module,
 						    false);
 		}
 next:
@@ -2438,7 +2441,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 	int i, fd, ret;
 	struct probe_trace_event *tev = NULL;
 	char buf[64];
-	const char *event, *group;
+	const char *event = NULL, *group = NULL;
 	struct strlist *namelist;
 	bool safename;
 
@@ -2500,15 +2503,12 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		/* Add added event name to namelist */
 		strlist__add(namelist, event);
 
-		/* Trick here - save current event/group */
-		event = pev->event;
-		group = pev->group;
-		pev->event = tev->event;
-		pev->group = tev->group;
-		show_perf_probe_event(pev, tev->point.module, true);
-		/* Trick here - restore current event/group */
-		pev->event = (char *)event;
-		pev->group = (char *)group;
+		/* We use tev's name for showing new events */
+		show_perf_probe_event(tev->group, tev->event, pev,
+				      tev->point.module, true);
+		/* Save the last valid name */
+		event = tev->event;
+		group = tev->group;
 
 		/*
 		 * Probes after the first probe which comes from same
@@ -2522,11 +2522,10 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
 		warn_uprobe_event_compat(tev);
 
 	/* Note that it is possible to skip all events because of blacklist */
-	if (ret >= 0 && tev->event) {
+	if (ret >= 0 && event) {
 		/* Show how to use the event. */
 		pr_info("\nYou can now use it in all perf tools, such as:\n\n");
-		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", tev->group,
-			 tev->event);
+		pr_info("\tperf record -e %s:%s -aR sleep 1\n\n", group, event);
 	}
 
 	strlist__delete(namelist);

Re: [PATCH perf/core 0/4] perf-probe bugfixes

[Arnaldo Carvalho de Melo]

Em Fri, Jun 12, 2015 at 02:08:06PM +0900, Masami Hiramatsu escreveu:
> Masami Hiramatsu (4):
>       [BUGFIX] perf probe: List probes in stdout
>       [BUGFIX] perf probe: Cut off the postfixes from event name
>       [BUGFIX] perf probe: Check non-probe-able symbols when using symbol map
>       [BUGFIX] perf probe: Show usage even if th last event is skipped

Aplied the second, it is in my perf/core branch, please see the other
messages about the other patches.

- Arnaldo