[PATCH 0/3] perf-probe: Fix __init function and blacklist checking

[PATCH 0/3] perf-probe: Fix __init function and blacklist checking

[Masami Hiramatsu]

Hi,

Here is a series of fixes related to __init function and
blacklist checking routines. Arnaldo noticed me some cases
which don't check the __init function checking. I found that
the blacklist checking is also not working with KASLR, and
also skipped probes are shown in result list unexpectedly.

Thank you,

---

Masami Hiramatsu (3):
      perf-probe: Fix to check blacklist address correctly
      perf-probe: Check address correctness by map instead of _etext
      perf-probe: Do not show the skipped events


 tools/perf/builtin-probe.c    |    3 +++
 tools/perf/util/probe-event.c |   46 +++++++++++++++++++++++++----------------
 2 files changed, 31 insertions(+), 18 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH 1/3] perf-probe: Fix to check blacklist address correctly

[Masami Hiramatsu]

Fix to check kprobe blacklist address correctly with
relocated address by adjusting debuginfo address.

Since the address in the debuginfo is same as objdump,
it is different from relocated kernel address with KASLR.
Thus, the perf-probe always misses to catch the
blacklisted addresses.

Without this patch, perf probe can not detect the blacklist
addresses on KASLR enabled kernel.

=========
  # perf probe kprobe_dispatcher
  Failed to write event: Invalid argument
    Error: Failed to add events.
=========

With this patch, it correctly shows the error message.

=========
  # perf probe kprobe_dispatcher
  kprobe_dispatcher is blacklisted function, skip it.
  Probe point 'kprobe_dispatcher' not found.
    Error: Failed to add events.
=========

Fixes: 9aaf5a5f479b ("perf probe: Check kprobes blacklist when adding new events")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
---
 tools/perf/util/probe-event.c |   21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index eea132f512b0..f75df63309be 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -102,7 +102,7 @@ void exit_probe_symbol_maps(void)
 	symbol__exit();
 }
 
-static struct ref_reloc_sym *kernel_get_ref_reloc_sym(void)
+static struct ref_reloc_sym *kernel_get_ref_reloc_sym(struct map **pmap)
 {
 	/* kmap->ref_reloc_sym should be set if host_machine is initialized */
 	struct kmap *kmap;
@@ -114,6 +114,10 @@ static struct ref_reloc_sym *kernel_get_ref_reloc_sym(void)
 	kmap = map__kmap(map);
 	if (!kmap)
 		return NULL;
+
+	if (pmap)
+		*pmap = map;
+
 	return kmap->ref_reloc_sym;
 }
 
@@ -125,7 +129,7 @@ static int kernel_get_symbol_address_by_name(const char *name, u64 *addr,
 	struct map *map;
 
 	/* ref_reloc_sym is just a label. Need a special fix*/
-	reloc_sym = kernel_get_ref_reloc_sym();
+	reloc_sym = kernel_get_ref_reloc_sym(NULL);
 	if (reloc_sym && strcmp(name, reloc_sym->name) == 0)
 		*addr = (reloc) ? reloc_sym->addr : reloc_sym->unrelocated_addr;
 	else {
@@ -745,6 +749,7 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 				       int ntevs)
 {
 	struct ref_reloc_sym *reloc_sym;
+	struct map *map;
 	char *tmp;
 	int i, skipped = 0;
 
@@ -753,7 +758,7 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 		return post_process_offline_probe_trace_events(tevs, ntevs,
 						symbol_conf.vmlinux_name);
 
-	reloc_sym = kernel_get_ref_reloc_sym();
+	reloc_sym = kernel_get_ref_reloc_sym(&map);
 	if (!reloc_sym) {
 		pr_warning("Relocated base symbol is not found!\n");
 		return -EINVAL;
@@ -764,9 +769,13 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
 			continue;
 		if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
 			continue;
-		/* If we found a wrong one, mark it by NULL symbol */
+		/*
+		 * If we found a wrong one, mark it by NULL symbol.
+		 * Since addresses in debuginfo is same as objdump, we need
+		 * to convert it to addresses on memory.
+		 */
 		if (kprobe_warn_out_range(tevs[i].point.symbol,
-					  tevs[i].point.address)) {
+			map__objdump_2mem(map, tevs[i].point.address))) {
 			tmp = NULL;
 			skipped++;
 		} else {
@@ -2936,7 +2945,7 @@ 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 && !pev->target &&
 			(!pp->retprobe || kretprobe_offset_is_supported())) {
-		reloc_sym = kernel_get_ref_reloc_sym();
+		reloc_sym = kernel_get_ref_reloc_sym(NULL);
 		if (!reloc_sym) {
 			pr_warning("Relocated base symbol is not found!\n");
 			ret = -EINVAL;

Re: [PATCH 1/3] perf-probe: Fix to check blacklist address correctly

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 9aaf5a5f479b ("perf probe: Check kprobes blacklist when adding new events").

The bot has tested the following trees: v5.6.7, v5.4.35, v4.19.118, v4.14.177, v4.9.220, v4.4.220.

v5.6.7: Build OK!
v5.4.35: Build OK!
v4.19.118: Build OK!
v4.14.177: Failed to apply! Possible dependencies:
    Unable to calculate

v4.9.220: Failed to apply! Possible dependencies:
    3da3ea7a8e20 ("perf probe: Factor out the ftrace README scanning")
    7ab31d94bff9 ("perf kretprobes: Offset from reloc_sym if kernel supports it")
    e491bc2f0dd9 ("perf probe: Generalize probe event file open routine")

v4.4.220: Failed to apply! Possible dependencies:
    053a3989e12f ("perf report/top: Add --raw-trace option")
    1c20b1d15473 ("perf probe: Show trace event definition")
    2d8f0f18a5c3 ("perf tools: Add stat round user level event")
    361459f163fa ("perf tools: Skip dynamic fields not defined for current event")
    374fb9e362f6 ("perf tools: Add stat config user level event")
    3938bad44ed2 ("perf tools: Remove needless 'extern' from function prototypes")
    428aff82e92a ("perf probe: Ignore vmlinux buildid if offline kernel is given")
    54430101d2af ("perf hists: Introduce hist_entry__filter()")
    5f3339d2e83c ("perf thread_map: Add thread_map user level event")
    60517d28fbd9 ("perf tools: Try to show pretty printed output for dynamic sort keys")
    6640b6c227fc ("perf cpu_map: Add cpu_map user level event")
    723928340c9d ("perf hist: Save raw_data/size for tracepoint events")
    a34bb6a08d60 ("perf tools: Add 'trace' sort key")
    b8f8eb84f483 ("perf tools: Remove misplaced __maybe_unused")
    c7c2a5e40f17 ("perf tools: Add dynamic sort key for tracepoint events")
    d7b617f51be4 ("perf tools: Pass perf_hpp_list all the way through setup_sort_list")
    d80518c90bb2 ("perf tools: Add stat user level event")
    fd36f3dd7933 ("perf hist: Pass struct sample to __hists__add_entry()")
    ffe777254cce ("perf tools: Add event_update user level event")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

Re: [PATCH 1/3] perf-probe: Fix to check blacklist address correctly

[Arnaldo Carvalho de Melo]

Em Thu, Apr 23, 2020 at 08:01:04PM +0900, Masami Hiramatsu escreveu:
> Fix to check kprobe blacklist address correctly with
> relocated address by adjusting debuginfo address.
> 
> Since the address in the debuginfo is same as objdump,
> it is different from relocated kernel address with KASLR.
> Thus, the perf-probe always misses to catch the
> blacklisted addresses.

Thanks, applied, sorry for the delay,

- Arnaldo
 
> Without this patch, perf probe can not detect the blacklist
> addresses on KASLR enabled kernel.
> 
> =========
>   # perf probe kprobe_dispatcher
>   Failed to write event: Invalid argument
>     Error: Failed to add events.
> =========
> 
> With this patch, it correctly shows the error message.
> 
> =========
>   # perf probe kprobe_dispatcher
>   kprobe_dispatcher is blacklisted function, skip it.
>   Probe point 'kprobe_dispatcher' not found.
>     Error: Failed to add events.
> =========
> 
> Fixes: 9aaf5a5f479b ("perf probe: Check kprobes blacklist when adding new events")
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  tools/perf/util/probe-event.c |   21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index eea132f512b0..f75df63309be 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -102,7 +102,7 @@ void exit_probe_symbol_maps(void)
>  	symbol__exit();
>  }
>  
> -static struct ref_reloc_sym *kernel_get_ref_reloc_sym(void)
> +static struct ref_reloc_sym *kernel_get_ref_reloc_sym(struct map **pmap)
>  {
>  	/* kmap->ref_reloc_sym should be set if host_machine is initialized */
>  	struct kmap *kmap;
> @@ -114,6 +114,10 @@ static struct ref_reloc_sym *kernel_get_ref_reloc_sym(void)
>  	kmap = map__kmap(map);
>  	if (!kmap)
>  		return NULL;
> +
> +	if (pmap)
> +		*pmap = map;
> +
>  	return kmap->ref_reloc_sym;
>  }
>  
> @@ -125,7 +129,7 @@ static int kernel_get_symbol_address_by_name(const char *name, u64 *addr,
>  	struct map *map;
>  
>  	/* ref_reloc_sym is just a label. Need a special fix*/
> -	reloc_sym = kernel_get_ref_reloc_sym();
> +	reloc_sym = kernel_get_ref_reloc_sym(NULL);
>  	if (reloc_sym && strcmp(name, reloc_sym->name) == 0)
>  		*addr = (reloc) ? reloc_sym->addr : reloc_sym->unrelocated_addr;
>  	else {
> @@ -745,6 +749,7 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
>  				       int ntevs)
>  {
>  	struct ref_reloc_sym *reloc_sym;
> +	struct map *map;
>  	char *tmp;
>  	int i, skipped = 0;
>  
> @@ -753,7 +758,7 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
>  		return post_process_offline_probe_trace_events(tevs, ntevs,
>  						symbol_conf.vmlinux_name);
>  
> -	reloc_sym = kernel_get_ref_reloc_sym();
> +	reloc_sym = kernel_get_ref_reloc_sym(&map);
>  	if (!reloc_sym) {
>  		pr_warning("Relocated base symbol is not found!\n");
>  		return -EINVAL;
> @@ -764,9 +769,13 @@ post_process_kernel_probe_trace_events(struct probe_trace_event *tevs,
>  			continue;
>  		if (tevs[i].point.retprobe && !kretprobe_offset_is_supported())
>  			continue;
> -		/* If we found a wrong one, mark it by NULL symbol */
> +		/*
> +		 * If we found a wrong one, mark it by NULL symbol.
> +		 * Since addresses in debuginfo is same as objdump, we need
> +		 * to convert it to addresses on memory.
> +		 */
>  		if (kprobe_warn_out_range(tevs[i].point.symbol,
> -					  tevs[i].point.address)) {
> +			map__objdump_2mem(map, tevs[i].point.address))) {
>  			tmp = NULL;
>  			skipped++;
>  		} else {
> @@ -2936,7 +2945,7 @@ 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 && !pev->target &&
>  			(!pp->retprobe || kretprobe_offset_is_supported())) {
> -		reloc_sym = kernel_get_ref_reloc_sym();
> +		reloc_sym = kernel_get_ref_reloc_sym(NULL);
>  		if (!reloc_sym) {
>  			pr_warning("Relocated base symbol is not found!\n");
>  			ret = -EINVAL;
> 

-- 

- Arnaldo

[PATCH 2/3] perf-probe: Check address correctness by map instead of _etext

[Masami Hiramatsu]

Since commit 03db8b583d1c ("perf tools: Fix maps__find_symbol_by_name()")
introduced map address range check in maps__find_symbol_by_name(),
we can not get "_etext" from kernel map because _etext is placed
on the edge of the kernel .text section (= kernel map in perf.)

To fix this issue, this checks the address correctness
by map address range information (map->start and map->end)
instead of using _etext address.

This can cause an error if the target inlined function is
embedded in both __init function and normal function.

For exaample, request_resource() is a normal function but also
embedded in __init reserve_setup(). In this case, the probe point
in reserve_setup() must be skipped. However, without this fix,
it failes to setup all probe points.
================
  # ./perf probe -v request_resource
  probe-definition(0): request_resource
  symbol:request_resource file:(null) line:0 offset:0 return:0 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /usr/lib/debug/lib/modules/5.5.17-200.fc31.x86_64/vmlinux for symbols
  Open Debuginfo file: /usr/lib/debug/lib/modules/5.5.17-200.fc31.x86_64/vmlinux
  Try to find probe point from debuginfo.
  Matched function: request_resource [15e29ad]
  found inline addr: 0xffffffff82fbf892
  Probe point found: reserve_setup+204
  found inline addr: 0xffffffff810e9790
  Probe point found: request_resource+0
  Found 2 probe_trace_events.
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Opening /sys/kernel/debug/tracing//README write=0
  Writing event: p:probe/request_resource _text+33290386
  Failed to write event: Invalid argument
    Error: Failed to add events. Reason: Invalid argument (Code: -22)
================

With this fix,

================
  # ./perf probe request_resource
  reserve_setup is out of .text, skip it.
  Added new events:
    (null):(null)        (on request_resource)
    probe:request_resource (on request_resource)

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

  	perf record -e probe:request_resource -aR sleep 1

================

Fixes: 03db8b583d1c ("perf tools: Fix maps__find_symbol_by_name()")
Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
---
 tools/perf/util/probe-event.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index f75df63309be..a5387e03e365 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -236,21 +236,22 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
 static bool kprobe_blacklist__listed(unsigned long address);
 static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
 {
-	u64 etext_addr = 0;
-	int ret;
-
-	/* Get the address of _etext for checking non-probable text symbol */
-	ret = kernel_get_symbol_address_by_name("_etext", &etext_addr,
-						false, false);
+	struct map *map;
+	bool ret = false;
 
-	if (ret == 0 && etext_addr < address)
-		pr_warning("%s is out of .text, skip it.\n", symbol);
-	else if (kprobe_blacklist__listed(address))
+	map = kernel_get_module_map(NULL);
+	if (map) {
+		ret = address <= map->start || map->end < address;
+		if (ret)
+			pr_warning("%s is out of .text, skip it.\n", symbol);
+		map__put(map);
+	}
+	if (!ret && kprobe_blacklist__listed(address)) {
 		pr_warning("%s is blacklisted function, skip it.\n", symbol);
-	else
-		return false;
+		ret = true;
+	}
 
-	return true;
+	return ret;
 }
 
 /*

Re: [PATCH 2/3] perf-probe: Check address correctness by map instead of _etext

[Arnaldo Carvalho de Melo]

Em Thu, Apr 23, 2020 at 08:01:13PM +0900, Masami Hiramatsu escreveu:
> Since commit 03db8b583d1c ("perf tools: Fix maps__find_symbol_by_name()")
> introduced map address range check in maps__find_symbol_by_name(),
> we can not get "_etext" from kernel map because _etext is placed
> on the edge of the kernel .text section (= kernel map in perf.)
> 
> To fix this issue, this checks the address correctness
> by map address range information (map->start and map->end)
> instead of using _etext address.
> 
> This can cause an error if the target inlined function is
> embedded in both __init function and normal function.
> 
> For exaample, request_resource() is a normal function but also
> embedded in __init reserve_setup(). In this case, the probe point
> in reserve_setup() must be skipped. However, without this fix,
> it failes to setup all probe points.
> ================
>   # ./perf probe -v request_resource
>   probe-definition(0): request_resource
>   symbol:request_resource file:(null) line:0 offset:0 return:0 lazy:(null)
>   0 arguments
>   Looking at the vmlinux_path (8 entries long)
>   Using /usr/lib/debug/lib/modules/5.5.17-200.fc31.x86_64/vmlinux for symbols
>   Open Debuginfo file: /usr/lib/debug/lib/modules/5.5.17-200.fc31.x86_64/vmlinux
>   Try to find probe point from debuginfo.
>   Matched function: request_resource [15e29ad]
>   found inline addr: 0xffffffff82fbf892
>   Probe point found: reserve_setup+204
>   found inline addr: 0xffffffff810e9790
>   Probe point found: request_resource+0
>   Found 2 probe_trace_events.
>   Opening /sys/kernel/debug/tracing//kprobe_events write=1
>   Opening /sys/kernel/debug/tracing//README write=0
>   Writing event: p:probe/request_resource _text+33290386
>   Failed to write event: Invalid argument
>     Error: Failed to add events. Reason: Invalid argument (Code: -22)
> ================
> 
> With this fix,
> 
> ================
>   # ./perf probe request_resource
>   reserve_setup is out of .text, skip it.
>   Added new events:
>     (null):(null)        (on request_resource)

But what is this (null):(null) probe? Confusing :-)

Thanks for working on this!

- Arnaldo

>     probe:request_resource (on request_resource)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:request_resource -aR sleep 1
> 
> ================
> 
> Fixes: 03db8b583d1c ("perf tools: Fix maps__find_symbol_by_name()")
> Reported-by: Arnaldo Carvalho de Melo <acme@kernel.org>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  tools/perf/util/probe-event.c |   25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index f75df63309be..a5387e03e365 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -236,21 +236,22 @@ static void clear_probe_trace_events(struct probe_trace_event *tevs, int ntevs)
>  static bool kprobe_blacklist__listed(unsigned long address);
>  static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
>  {
> -	u64 etext_addr = 0;
> -	int ret;
> -
> -	/* Get the address of _etext for checking non-probable text symbol */
> -	ret = kernel_get_symbol_address_by_name("_etext", &etext_addr,
> -						false, false);
> +	struct map *map;
> +	bool ret = false;
>  
> -	if (ret == 0 && etext_addr < address)
> -		pr_warning("%s is out of .text, skip it.\n", symbol);
> -	else if (kprobe_blacklist__listed(address))
> +	map = kernel_get_module_map(NULL);
> +	if (map) {
> +		ret = address <= map->start || map->end < address;
> +		if (ret)
> +			pr_warning("%s is out of .text, skip it.\n", symbol);
> +		map__put(map);
> +	}
> +	if (!ret && kprobe_blacklist__listed(address)) {
>  		pr_warning("%s is blacklisted function, skip it.\n", symbol);
> -	else
> -		return false;
> +		ret = true;
> +	}
>  
> -	return true;
> +	return ret;
>  }
>  
>  /*
> 

-- 

- Arnaldo

[PATCH 3/3] perf-probe: Do not show the skipped events

[Masami Hiramatsu]

When a probe point is expanded to several places (like inlined) and
if some of them are skipped because of blacklisted or __init function,
those trace_events has no event name. It must be skipped while showing
results.

Without this fix, you can see "(null):(null)" on the list,
===========
  # ./perf probe request_resource
  reserve_setup is out of .text, skip it.
  Added new events:
    (null):(null)        (on request_resource)
    probe:request_resource (on request_resource)

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

  	perf record -e probe:request_resource -aR sleep 1

===========

With this fix, it is ignored.
===========
  # ./perf probe request_resource
  reserve_setup is out of .text, skip it.
  Added new events:
    probe:request_resource (on request_resource)

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

  	perf record -e probe:request_resource -aR sleep 1

===========

Fixes: 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
---
 tools/perf/builtin-probe.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 70548df2abb9..6b1507566770 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -364,6 +364,9 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
 
 		for (k = 0; k < pev->ntevs; k++) {
 			struct probe_trace_event *tev = &pev->tevs[k];
+			/* Skipped events have no event name */
+			if (!tev->event)
+				continue;
 
 			/* We use tev's name for showing new events */
 			show_perf_probe_event(tev->group, tev->event, pev,

Re: [PATCH 3/3] perf-probe: Do not show the skipped events

[Arnaldo Carvalho de Melo]

Em Thu, Apr 23, 2020 at 08:01:22PM +0900, Masami Hiramatsu escreveu:
> When a probe point is expanded to several places (like inlined) and
> if some of them are skipped because of blacklisted or __init function,
> those trace_events has no event name. It must be skipped while showing
> results.
> 
> Without this fix, you can see "(null):(null)" on the list,
> ===========

Ok, you broke the patch in two, I think its better to combine both, ok?

- Arnaldo

>   # ./perf probe request_resource
>   reserve_setup is out of .text, skip it.
>   Added new events:
>     (null):(null)        (on request_resource)
>     probe:request_resource (on request_resource)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:request_resource -aR sleep 1
> 
> ===========
> 
> With this fix, it is ignored.
> ===========
>   # ./perf probe request_resource
>   reserve_setup is out of .text, skip it.
>   Added new events:
>     probe:request_resource (on request_resource)
> 
>   You can now use it in all perf tools, such as:
> 
>   	perf record -e probe:request_resource -aR sleep 1
> 
> ===========
> 
> Fixes: 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text")
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  tools/perf/builtin-probe.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index 70548df2abb9..6b1507566770 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -364,6 +364,9 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
>  
>  		for (k = 0; k < pev->ntevs; k++) {
>  			struct probe_trace_event *tev = &pev->tevs[k];
> +			/* Skipped events have no event name */
> +			if (!tev->event)
> +				continue;
>  
>  			/* We use tev's name for showing new events */
>  			show_perf_probe_event(tev->group, tev->event, pev,
> 

-- 

- Arnaldo

Re: [PATCH 3/3] perf-probe: Do not show the skipped events

[Masami Hiramatsu]

On Thu, 23 Apr 2020 11:01:39 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Thu, Apr 23, 2020 at 08:01:22PM +0900, Masami Hiramatsu escreveu:
> > When a probe point is expanded to several places (like inlined) and
> > if some of them are skipped because of blacklisted or __init function,
> > those trace_events has no event name. It must be skipped while showing
> > results.
> > 
> > Without this fix, you can see "(null):(null)" on the list,
> > ===========
> 
> Ok, you broke the patch in two, I think its better to combine both, ok?

No, if an inlined function is embedded in blacklisted areas, it also
shows same "(null):(null)" without [2/3].

Reordering the patches is OK, but this is still an independent fix.

Thank you,

> 
> - Arnaldo
> 
> >   # ./perf probe request_resource
> >   reserve_setup is out of .text, skip it.
> >   Added new events:
> >     (null):(null)        (on request_resource)
> >     probe:request_resource (on request_resource)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >   	perf record -e probe:request_resource -aR sleep 1
> > 
> > ===========
> > 
> > With this fix, it is ignored.
> > ===========
> >   # ./perf probe request_resource
> >   reserve_setup is out of .text, skip it.
> >   Added new events:
> >     probe:request_resource (on request_resource)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >   	perf record -e probe:request_resource -aR sleep 1
> > 
> > ===========
> > 
> > Fixes: 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text")
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: stable@vger.kernel.org
> > ---
> >  tools/perf/builtin-probe.c |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > index 70548df2abb9..6b1507566770 100644
> > --- a/tools/perf/builtin-probe.c
> > +++ b/tools/perf/builtin-probe.c
> > @@ -364,6 +364,9 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
> >  
> >  		for (k = 0; k < pev->ntevs; k++) {
> >  			struct probe_trace_event *tev = &pev->tevs[k];
> > +			/* Skipped events have no event name */
> > +			if (!tev->event)
> > +				continue;
> >  
> >  			/* We use tev's name for showing new events */
> >  			show_perf_probe_event(tev->group, tev->event, pev,
> > 
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH 3/3] perf-probe: Do not show the skipped events

[Arnaldo Carvalho de Melo]

Em Fri, Apr 24, 2020 at 08:33:05AM +0900, Masami Hiramatsu escreveu:
> On Thu, 23 Apr 2020 11:01:39 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Thu, Apr 23, 2020 at 08:01:22PM +0900, Masami Hiramatsu escreveu:
> > > When a probe point is expanded to several places (like inlined) and
> > > if some of them are skipped because of blacklisted or __init function,
> > > those trace_events has no event name. It must be skipped while showing
> > > results.
> > > 
> > > Without this fix, you can see "(null):(null)" on the list,
> > > ===========
> > 
> > Ok, you broke the patch in two, I think its better to combine both, ok?
> 
> No, if an inlined function is embedded in blacklisted areas, it also
> shows same "(null):(null)" without [2/3].
> 
> Reordering the patches is OK, but this is still an independent fix.

Ok, so I'll try reordering, so that we don't see it in the cset log for
the other fix.

Thanks for the clarification,

- Arnaldo
 
> Thank you,
> 
> > 
> > - Arnaldo
> > 
> > >   # ./perf probe request_resource
> > >   reserve_setup is out of .text, skip it.
> > >   Added new events:
> > >     (null):(null)        (on request_resource)
> > >     probe:request_resource (on request_resource)
> > > 
> > >   You can now use it in all perf tools, such as:
> > > 
> > >   	perf record -e probe:request_resource -aR sleep 1
> > > 
> > > ===========
> > > 
> > > With this fix, it is ignored.
> > > ===========
> > >   # ./perf probe request_resource
> > >   reserve_setup is out of .text, skip it.
> > >   Added new events:
> > >     probe:request_resource (on request_resource)
> > > 
> > >   You can now use it in all perf tools, such as:
> > > 
> > >   	perf record -e probe:request_resource -aR sleep 1
> > > 
> > > ===========
> > > 
> > > Fixes: 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text")
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  tools/perf/builtin-probe.c |    3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > > index 70548df2abb9..6b1507566770 100644
> > > --- a/tools/perf/builtin-probe.c
> > > +++ b/tools/perf/builtin-probe.c
> > > @@ -364,6 +364,9 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
> > >  
> > >  		for (k = 0; k < pev->ntevs; k++) {
> > >  			struct probe_trace_event *tev = &pev->tevs[k];
> > > +			/* Skipped events have no event name */
> > > +			if (!tev->event)
> > > +				continue;
> > >  
> > >  			/* We use tev's name for showing new events */
> > >  			show_perf_probe_event(tev->group, tev->event, pev,
> > > 
> > 
> > -- 
> > 
> > - Arnaldo
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

-- 

- Arnaldo

Re: [PATCH 0/3] perf-probe: Fix __init function and blacklist checking

[Arnaldo Carvalho de Melo]

Em Thu, Apr 23, 2020 at 08:00:54PM +0900, Masami Hiramatsu escreveu:
> Hi,
> 
> Here is a series of fixes related to __init function and
> blacklist checking routines. Arnaldo noticed me some cases
> which don't check the __init function checking. I found that
> the blacklist checking is also not working with KASLR, and
> also skipped probes are shown in result list unexpectedly.

thanks, tested and applied.

- Arnaldo
 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (3):
>       perf-probe: Fix to check blacklist address correctly
>       perf-probe: Check address correctness by map instead of _etext
>       perf-probe: Do not show the skipped events
> 
> 
>  tools/perf/builtin-probe.c    |    3 +++
>  tools/perf/util/probe-event.c |   46 +++++++++++++++++++++++++----------------
>  2 files changed, 31 insertions(+), 18 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

-- 

- Arnaldo