All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] perf util: Change intlist int to unsigned long
@ 2021-01-29  7:08 Jin Yao
  2021-01-29  7:08 ` [PATCH v2 2/2] perf script: Support filtering by hex address Jin Yao
  0 siblings, 1 reply; 5+ messages in thread
From: Jin Yao @ 2021-01-29  7:08 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

This is to let intlist support address.

One potential problem is it can't support negative number. But
so far, there is no such kind of use case.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 v2:
   New in v2.

 tools/perf/util/intlist.c     | 27 ++++++++++++++++-----------
 tools/perf/util/intlist.h     | 10 +++++-----
 tools/perf/util/probe-event.c |  2 +-
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/intlist.c b/tools/perf/util/intlist.c
index 84e5304e151a..934092199f89 100644
--- a/tools/perf/util/intlist.c
+++ b/tools/perf/util/intlist.c
@@ -13,7 +13,7 @@
 static struct rb_node *intlist__node_new(struct rblist *rblist __maybe_unused,
 					 const void *entry)
 {
-	int i = (int)((long)entry);
+	unsigned long i = (unsigned long)entry;
 	struct rb_node *rc = NULL;
 	struct int_node *node = malloc(sizeof(*node));
 
@@ -41,15 +41,20 @@ static void intlist__node_delete(struct rblist *rblist __maybe_unused,
 
 static int intlist__node_cmp(struct rb_node *rb_node, const void *entry)
 {
-	int i = (int)((long)entry);
+	unsigned long i = (unsigned long)entry;
 	struct int_node *node = container_of(rb_node, struct int_node, rb_node);
 
-	return node->i - i;
+	if (node->i > i)
+		return 1;
+	else if (node->i < i)
+		return -1;
+
+	return 0;
 }
 
-int intlist__add(struct intlist *ilist, int i)
+int intlist__add(struct intlist *ilist, unsigned long i)
 {
-	return rblist__add_node(&ilist->rblist, (void *)((long)i));
+	return rblist__add_node(&ilist->rblist, (void *)i);
 }
 
 void intlist__remove(struct intlist *ilist, struct int_node *node)
@@ -58,7 +63,7 @@ void intlist__remove(struct intlist *ilist, struct int_node *node)
 }
 
 static struct int_node *__intlist__findnew(struct intlist *ilist,
-					   int i, bool create)
+					   unsigned long i, bool create)
 {
 	struct int_node *node = NULL;
 	struct rb_node *rb_node;
@@ -67,9 +72,9 @@ static struct int_node *__intlist__findnew(struct intlist *ilist,
 		return NULL;
 
 	if (create)
-		rb_node = rblist__findnew(&ilist->rblist, (void *)((long)i));
+		rb_node = rblist__findnew(&ilist->rblist, (void *)i);
 	else
-		rb_node = rblist__find(&ilist->rblist, (void *)((long)i));
+		rb_node = rblist__find(&ilist->rblist, (void *)i);
 
 	if (rb_node)
 		node = container_of(rb_node, struct int_node, rb_node);
@@ -77,12 +82,12 @@ static struct int_node *__intlist__findnew(struct intlist *ilist,
 	return node;
 }
 
-struct int_node *intlist__find(struct intlist *ilist, int i)
+struct int_node *intlist__find(struct intlist *ilist, unsigned long i)
 {
 	return __intlist__findnew(ilist, i, false);
 }
 
-struct int_node *intlist__findnew(struct intlist *ilist, int i)
+struct int_node *intlist__findnew(struct intlist *ilist, unsigned long i)
 {
 	return __intlist__findnew(ilist, i, true);
 }
@@ -93,7 +98,7 @@ static int intlist__parse_list(struct intlist *ilist, const char *s)
 	int err;
 
 	do {
-		long value = strtol(s, &sep, 10);
+		unsigned long value = strtol(s, &sep, 10);
 		err = -EINVAL;
 		if (*sep != ',' && *sep != '\0')
 			break;
diff --git a/tools/perf/util/intlist.h b/tools/perf/util/intlist.h
index 5c19ee001299..e336b174d0c7 100644
--- a/tools/perf/util/intlist.h
+++ b/tools/perf/util/intlist.h
@@ -9,7 +9,7 @@
 
 struct int_node {
 	struct rb_node rb_node;
-	int i;
+	unsigned long i;
 	void *priv;
 };
 
@@ -21,13 +21,13 @@ struct intlist *intlist__new(const char *slist);
 void intlist__delete(struct intlist *ilist);
 
 void intlist__remove(struct intlist *ilist, struct int_node *in);
-int intlist__add(struct intlist *ilist, int i);
+int intlist__add(struct intlist *ilist, unsigned long i);
 
 struct int_node *intlist__entry(const struct intlist *ilist, unsigned int idx);
-struct int_node *intlist__find(struct intlist *ilist, int i);
-struct int_node *intlist__findnew(struct intlist *ilist, int i);
+struct int_node *intlist__find(struct intlist *ilist, unsigned long i);
+struct int_node *intlist__findnew(struct intlist *ilist, unsigned long i);
 
-static inline bool intlist__has_entry(struct intlist *ilist, int i)
+static inline bool intlist__has_entry(struct intlist *ilist, unsigned long i)
 {
 	return intlist__find(ilist, i) != NULL;
 }
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8eae2afff71a..137f19c5b686 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -1074,7 +1074,7 @@ static int __show_line_range(struct line_range *lr, const char *module,
 	}
 
 	intlist__for_each_entry(ln, lr->line_list) {
-		for (; ln->i > l; l++) {
+		for (; ln->i > (unsigned long)l; l++) {
 			ret = show_one_line(fp, l - lr->offset);
 			if (ret < 0)
 				goto end;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] perf script: Support filtering by hex address
  2021-01-29  7:08 [PATCH v2 1/2] perf util: Change intlist int to unsigned long Jin Yao
@ 2021-01-29  7:08 ` Jin Yao
  2021-02-05  9:49   ` Jiri Olsa
  0 siblings, 1 reply; 5+ messages in thread
From: Jin Yao @ 2021-01-29  7:08 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao

Perf-script supports '-S' or '--symbol' options to only list the
trace records in given symbols. Symbol is typically a name
or hex address. If it's hex address, it is the start address of
one symbol.

While it would be useful if we can filter trace records by any hex
address (not only the start address of symbol). So now we support
filtering trace records by more conditions, such as:
- symbol name
- start address of symbol
- any hexadecimal address
- address range

The comparison order is defined as:

1. symbol name comparison
2. symbol start address comparison.
3. any hexadecimal address comparison.
4. address range comparison.

Let's see some examples:

root@kbl-ppc:~# ./perf script -S 0xffffffff9ca77308
            perf 18123 [000] 6142863.075104:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [000] 6142863.075107:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [000] 6142863.075108:         10   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [001] 6142863.075156:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [001] 6142863.075158:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [001] 6142863.075159:         17   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [002] 6142863.075202:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [002] 6142863.075204:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [002] 6142863.075205:         16   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [003] 6142863.075250:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [003] 6142863.075252:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [003] 6142863.075253:         16   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])

Filter the traced records by hex address 0xffffffff9ca77308.

Easy to use, we support the hex address without '0x' prefix,
e.g.
./perf script -S ffffffff9ca77308

It has the same effect.

We also support to filter trace records by a address range.

root@kbl-ppc:~# ./perf script -S ffffffff9ca77304 --addr-range 16
            perf 18123 [000] 6142863.075104:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [000] 6142863.075107:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [000] 6142863.075108:         10   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [000] 6142863.075109:        273   cycles:  ffffffff9ca7730a native_write_msr+0xa ([kernel.kallsyms])
            perf 18123 [001] 6142863.075156:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [001] 6142863.075158:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [001] 6142863.075159:         17   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [001] 6142863.075160:        456   cycles:  ffffffff9ca7730f native_write_msr+0xf ([kernel.kallsyms])
            perf 18123 [002] 6142863.075202:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [002] 6142863.075204:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [002] 6142863.075205:         16   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [002] 6142863.075206:        436   cycles:  ffffffff9ca7730f native_write_msr+0xf ([kernel.kallsyms])
            perf 18123 [003] 6142863.075250:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [003] 6142863.075252:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [003] 6142863.075253:         16   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [003] 6142863.075254:        436   cycles:  ffffffff9ca7730f native_write_msr+0xf ([kernel.kallsyms])
            perf 18123 [004] 6142863.075299:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [004] 6142863.075301:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [004] 6142863.075302:         16   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [004] 6142863.075303:        431   cycles:  ffffffff9ca7730f native_write_msr+0xf ([kernel.kallsyms])
            perf 18123 [005] 6142863.075348:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [005] 6142863.075349:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [005] 6142863.075351:         17   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [005] 6142863.075352:        454   cycles:  ffffffff9ca7730f native_write_msr+0xf ([kernel.kallsyms])
            perf 18123 [006] 6142863.075390:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [006] 6142863.075392:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [006] 6142863.075393:         17   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [006] 6142863.075394:        459   cycles:  ffffffff9ca7730f native_write_msr+0xf ([kernel.kallsyms])
            perf 18123 [007] 6142863.075438:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [007] 6142863.075440:          1   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [007] 6142863.075441:         17   cycles:  ffffffff9ca77308 native_write_msr+0x8 ([kernel.kallsyms])
            perf 18123 [007] 6142863.075442:        466   cycles:  ffffffff9ca7730f native_write_msr+0xf ([kernel.kallsyms])

Filter the traced records from address range [0xffffffff9ca77304, 0xffffffff9ca77304 + 15].

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
 v2:
   Use intlist for address list.

 tools/perf/Documentation/perf-script.txt | 19 ++++++++++++
 tools/perf/builtin-script.c              |  2 ++
 tools/perf/util/event.c                  | 24 +++++++++++++++
 tools/perf/util/symbol.c                 | 39 ++++++++++++++++++++++++
 tools/perf/util/symbol_conf.h            |  4 ++-
 5 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-script.txt b/tools/perf/Documentation/perf-script.txt
index 60dae302db27..4c37f193a231 100644
--- a/tools/perf/Documentation/perf-script.txt
+++ b/tools/perf/Documentation/perf-script.txt
@@ -422,9 +422,28 @@ include::itrace.txt[]
 	Only consider the listed symbols. Symbols are typically a name
 	but they may also be hexadecimal address.
 
+	The hexadecimal address may be the start address of a symbol or
+	any other address to filter the trace records.
+
 	For example, to select the symbol noploop or the address 0x4007a0:
 	perf script --symbols=noploop,0x4007a0
 
+	Support filtering trace records by symbol name, start address of
+	symbol, any hexadecimal address and address range.
+
+	The comparison order is:
+	1. symbol name comparison
+	2. symbol start address comparison.
+	3. any hexadecimal address comparison.
+	4. address range comparison (see --addr-range).
+
+--addr-range::
+	Use with -S or --symbols to list traced records within address range.
+
+	For example, to list the traced records within the address range
+	[0x4007a0, 0x0x4007a9]:
+	perf script -S 0x4007a0 --addr-range 10
+
 --call-trace::
 	Show call stream for intel_pt traces. The CPUs are interleaved, but
 	can be filtered with -C.
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 9e995311a9b8..3d9e70e19196 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3534,6 +3534,8 @@ int cmd_script(int argc, const char **argv)
 		    "system-wide collection from all CPUs"),
 	OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
 		   "only consider these symbols"),
+	OPT_INTEGER(0, "addr-range", &symbol_conf.addr_range,
+		    "Use with -S to list traced records within address range"),
 	OPT_CALLBACK_OPTARG(0, "insn-trace", &itrace_synth_opts, NULL, NULL,
 			"Decode instructions from itrace", parse_insn_trace),
 	OPT_CALLBACK_OPTARG(0, "xed", NULL, NULL, NULL,
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index fbe8578e4c47..8cc70aa630cc 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -645,6 +645,19 @@ struct symbol *thread__find_symbol_fb(struct thread *thread, u8 cpumode,
 	return al->sym;
 }
 
+static bool check_address_range(struct intlist *addr_list, int addr_range,
+				unsigned long addr)
+{
+	struct int_node *pos;
+
+	intlist__for_each_entry(pos, addr_list) {
+		if (addr >= pos->i && addr < pos->i + addr_range)
+			return true;
+	}
+
+	return false;
+}
+
 /*
  * Callers need to drop the reference to al->thread, obtained in
  * machine__findnew_thread()
@@ -709,6 +722,17 @@ int machine__resolve(struct machine *machine, struct addr_location *al,
 			ret = strlist__has_entry(symbol_conf.sym_list,
 						al_addr_str);
 		}
+		if (!ret && symbol_conf.addr_list && al->map) {
+			unsigned long addr = al->map->unmap_ip(al->map, al->addr);
+
+			ret = intlist__has_entry(symbol_conf.addr_list, addr);
+			if (!ret && symbol_conf.addr_range) {
+				ret = check_address_range(symbol_conf.addr_list,
+							  symbol_conf.addr_range,
+							  addr);
+			}
+		}
+
 		if (!ret)
 			al->filtered |= (1 << HIST_FILTER__SYMBOL);
 	}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 64a039cbba1b..2c13f6acda7f 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2406,6 +2406,39 @@ int setup_intlist(struct intlist **list, const char *list_str,
 	return 0;
 }
 
+static int setup_addrlist(struct intlist **addr_list, struct strlist *sym_list)
+{
+	struct str_node *pos, *tmp;
+	unsigned long val;
+	char *sep;
+	int err = 0, i = 0;
+
+	*addr_list = intlist__new(NULL);
+	if (!*addr_list)
+		return -1;
+
+	strlist__for_each_entry_safe(pos, tmp, sym_list) {
+		val = strtoul(pos->s, &sep, 16);
+		if (*sep != ',' && *sep != '\0')
+			continue;
+
+		err = intlist__add(*addr_list, val);
+		if (err)
+			break;
+
+		strlist__remove(sym_list, pos);
+		i++;
+	}
+
+	if (err || (i == 0)) {
+		intlist__delete(*addr_list);
+		*addr_list = NULL;
+		return err;
+	}
+
+	return 0;
+}
+
 static bool symbol__read_kptr_restrict(void)
 {
 	bool value = false;
@@ -2489,6 +2522,10 @@ int symbol__init(struct perf_env *env)
 		       symbol_conf.sym_list_str, "symbol") < 0)
 		goto out_free_tid_list;
 
+	if (symbol_conf.sym_list &&
+	    setup_addrlist(&symbol_conf.addr_list, symbol_conf.sym_list) < 0)
+		goto out_free_sym_list;
+
 	if (setup_list(&symbol_conf.bt_stop_list,
 		       symbol_conf.bt_stop_list_str, "symbol") < 0)
 		goto out_free_sym_list;
@@ -2512,6 +2549,7 @@ int symbol__init(struct perf_env *env)
 
 out_free_sym_list:
 	strlist__delete(symbol_conf.sym_list);
+	intlist__delete(symbol_conf.addr_list);
 out_free_tid_list:
 	intlist__delete(symbol_conf.tid_list);
 out_free_pid_list:
@@ -2533,6 +2571,7 @@ void symbol__exit(void)
 	strlist__delete(symbol_conf.comm_list);
 	intlist__delete(symbol_conf.tid_list);
 	intlist__delete(symbol_conf.pid_list);
+	intlist__delete(symbol_conf.addr_list);
 	vmlinux_path__exit();
 	symbol_conf.sym_list = symbol_conf.dso_list = symbol_conf.comm_list = NULL;
 	symbol_conf.bt_stop_list = NULL;
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index b18f9c8dbb75..a70b3ec09dac 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -70,11 +70,13 @@ struct symbol_conf {
 			*sym_to_list,
 			*bt_stop_list;
 	struct intlist	*pid_list,
-			*tid_list;
+			*tid_list,
+			*addr_list;
 	const char	*symfs;
 	int		res_sample;
 	int		pad_output_len_dso;
 	int		group_sort_idx;
+	int		addr_range;
 };
 
 extern struct symbol_conf symbol_conf;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] perf script: Support filtering by hex address
  2021-01-29  7:08 ` [PATCH v2 2/2] perf script: Support filtering by hex address Jin Yao
@ 2021-02-05  9:49   ` Jiri Olsa
  2021-02-07  0:19     ` Jin, Yao
  2021-02-07  7:28     ` Jin, Yao
  0 siblings, 2 replies; 5+ messages in thread
From: Jiri Olsa @ 2021-02-05  9:49 UTC (permalink / raw)
  To: Jin Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

On Fri, Jan 29, 2021 at 03:08:54PM +0800, Jin Yao wrote:

SNIP

> +			}
> +		}
> +
>  		if (!ret)
>  			al->filtered |= (1 << HIST_FILTER__SYMBOL);
>  	}
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 64a039cbba1b..2c13f6acda7f 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -2406,6 +2406,39 @@ int setup_intlist(struct intlist **list, const char *list_str,
>  	return 0;
>  }
>  
> +static int setup_addrlist(struct intlist **addr_list, struct strlist *sym_list)
> +{
> +	struct str_node *pos, *tmp;
> +	unsigned long val;
> +	char *sep;
> +	int err = 0, i = 0;
> +
> +	*addr_list = intlist__new(NULL);
> +	if (!*addr_list)
> +		return -1;
> +
> +	strlist__for_each_entry_safe(pos, tmp, sym_list) {
> +		val = strtoul(pos->s, &sep, 16);
> +		if (*sep != ',' && *sep != '\0')
> +			continue;

I think you also need to check that val is valid and errno
(check other strtoul we call in perf)
because above could pass for:

	$ ./perf script -S ",7fd0f1b69a13"
	ls 651468 410180.795577:      90098 cycles:u:      7fd0f1b69a13 __GI___tunables_init+0x73 (/usr/lib64/ld-2.32.so)


plus check for " " so we could do:

	$ ./perf script -S "7fd0f1b69a13 ,7fd0f1b5e189"
	ls 651468 410180.796055:     190731 cycles:u:      7fd0f1b5e189 _dl_relocate_object+0x4b9 (/usr/lib64/ld-2.32.so)

	$ ./perf script -S "7fd0f1b69a13,7fd0f1b5e189"
	ls 651468 410180.795577:      90098 cycles:u:      7fd0f1b69a13 __GI___tunables_init+0x73 (/usr/lib64/ld-2.32.so)
	ls 651468 410180.796055:     190731 cycles:u:      7fd0f1b5e189 _dl_relocate_object+0x4b9 (/usr/lib64/ld-2.32.so)


thanks,
jirka


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] perf script: Support filtering by hex address
  2021-02-05  9:49   ` Jiri Olsa
@ 2021-02-07  0:19     ` Jin, Yao
  2021-02-07  7:28     ` Jin, Yao
  1 sibling, 0 replies; 5+ messages in thread
From: Jin, Yao @ 2021-02-07  0:19 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Hi Jiri,

On 2/5/2021 5:49 PM, Jiri Olsa wrote:
> On Fri, Jan 29, 2021 at 03:08:54PM +0800, Jin Yao wrote:
> 
> SNIP
> 
>> +			}
>> +		}
>> +
>>   		if (!ret)
>>   			al->filtered |= (1 << HIST_FILTER__SYMBOL);
>>   	}
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 64a039cbba1b..2c13f6acda7f 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -2406,6 +2406,39 @@ int setup_intlist(struct intlist **list, const char *list_str,
>>   	return 0;
>>   }
>>   
>> +static int setup_addrlist(struct intlist **addr_list, struct strlist *sym_list)
>> +{
>> +	struct str_node *pos, *tmp;
>> +	unsigned long val;
>> +	char *sep;
>> +	int err = 0, i = 0;
>> +
>> +	*addr_list = intlist__new(NULL);
>> +	if (!*addr_list)
>> +		return -1;
>> +
>> +	strlist__for_each_entry_safe(pos, tmp, sym_list) {
>> +		val = strtoul(pos->s, &sep, 16);
>> +		if (*sep != ',' && *sep != '\0')
>> +			continue;
> 
> I think you also need to check that val is valid and errno
> (check other strtoul we call in perf)
> because above could pass for:
> 
> 	$ ./perf script -S ",7fd0f1b69a13"
> 	ls 651468 410180.795577:      90098 cycles:u:      7fd0f1b69a13 __GI___tunables_init+0x73 (/usr/lib64/ld-2.32.so)
> 
> 
> plus check for " " so we could do:
> 
> 	$ ./perf script -S "7fd0f1b69a13 ,7fd0f1b5e189"
> 	ls 651468 410180.796055:     190731 cycles:u:      7fd0f1b5e189 _dl_relocate_object+0x4b9 (/usr/lib64/ld-2.32.so)
> 
> 	$ ./perf script -S "7fd0f1b69a13,7fd0f1b5e189"
> 	ls 651468 410180.795577:      90098 cycles:u:      7fd0f1b69a13 __GI___tunables_init+0x73 (/usr/lib64/ld-2.32.so)
> 	ls 651468 410180.796055:     190731 cycles:u:      7fd0f1b5e189 _dl_relocate_object+0x4b9 (/usr/lib64/ld-2.32.so)
> 
> 
> thanks,
> jirka
> 

Thanks for reminding. Yes, these cases I need to cover. I will prepare the v3.

Thanks
Jin Yao

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] perf script: Support filtering by hex address
  2021-02-05  9:49   ` Jiri Olsa
  2021-02-07  0:19     ` Jin, Yao
@ 2021-02-07  7:28     ` Jin, Yao
  1 sibling, 0 replies; 5+ messages in thread
From: Jin, Yao @ 2021-02-07  7:28 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
	kan.liang, yao.jin

Hi Jiri,

On 2/5/2021 5:49 PM, Jiri Olsa wrote:
> On Fri, Jan 29, 2021 at 03:08:54PM +0800, Jin Yao wrote:
> 
> SNIP
> 
>> +			}
>> +		}
>> +
>>   		if (!ret)
>>   			al->filtered |= (1 << HIST_FILTER__SYMBOL);
>>   	}
>> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
>> index 64a039cbba1b..2c13f6acda7f 100644
>> --- a/tools/perf/util/symbol.c
>> +++ b/tools/perf/util/symbol.c
>> @@ -2406,6 +2406,39 @@ int setup_intlist(struct intlist **list, const char *list_str,
>>   	return 0;
>>   }
>>   
>> +static int setup_addrlist(struct intlist **addr_list, struct strlist *sym_list)
>> +{
>> +	struct str_node *pos, *tmp;
>> +	unsigned long val;
>> +	char *sep;
>> +	int err = 0, i = 0;
>> +
>> +	*addr_list = intlist__new(NULL);
>> +	if (!*addr_list)
>> +		return -1;
>> +
>> +	strlist__for_each_entry_safe(pos, tmp, sym_list) {
>> +		val = strtoul(pos->s, &sep, 16);
>> +		if (*sep != ',' && *sep != '\0')
>> +			continue;
> 
> I think you also need to check that val is valid and errno
> (check other strtoul we call in perf)
> because above could pass for:
> 
> 	$ ./perf script -S ",7fd0f1b69a13"
> 	ls 651468 410180.795577:      90098 cycles:u:      7fd0f1b69a13 __GI___tunables_init+0x73 (/usr/lib64/ld-2.32.so)
> 
> 

Now I think this may be OK because that's not the pattern matching. It just matches the 7fd0f1b69a13 
in address list.

And we also need to support the address + symbol combination, such as,

$ ./perf script -S "ffffffff9a51de2a,put_cmsg"
          swapper     0 [007] 347304.359521:      44937   cycles:  ffffffff9a51de2a rcu_core+0x29a 
([kernel.kallsyms])
     avahi-daemon   580 [007] 347304.468122:      57176   cycles:  ffffffff9af4764e put_cmsg+0xde 
([kernel.kallsyms])

So the idea is when we can get a valid address from -S list, we add the address to intlist for 
address comparison otherwise we still leave it to symbol list for symbol comparison.

Anyway, I will post v3 patch for review.

Thanks
Jin Yao

> plus check for " " so we could do:
> 
> 	$ ./perf script -S "7fd0f1b69a13 ,7fd0f1b5e189"
> 	ls 651468 410180.796055:     190731 cycles:u:      7fd0f1b5e189 _dl_relocate_object+0x4b9 (/usr/lib64/ld-2.32.so)
> 
> 	$ ./perf script -S "7fd0f1b69a13,7fd0f1b5e189"
> 	ls 651468 410180.795577:      90098 cycles:u:      7fd0f1b69a13 __GI___tunables_init+0x73 (/usr/lib64/ld-2.32.so)
> 	ls 651468 410180.796055:     190731 cycles:u:      7fd0f1b5e189 _dl_relocate_object+0x4b9 (/usr/lib64/ld-2.32.so)
> 
> 
> thanks,
> jirka
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-02-07  7:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-29  7:08 [PATCH v2 1/2] perf util: Change intlist int to unsigned long Jin Yao
2021-01-29  7:08 ` [PATCH v2 2/2] perf script: Support filtering by hex address Jin Yao
2021-02-05  9:49   ` Jiri Olsa
2021-02-07  0:19     ` Jin, Yao
2021-02-07  7:28     ` Jin, Yao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.