linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
@ 2018-06-04  6:50 Alexey Budankov
  2018-06-04  7:56 ` Jiri Olsa
  2018-06-07  8:18 ` [tip:perf/urgent] perf record: Enable " tip-bot for Alexey Budankov
  0 siblings, 2 replies; 12+ messages in thread
From: Alexey Budankov @ 2018-06-04  6:50 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo
  Cc: Alexander Shishkin, Jiri Olsa, Namhyung Kim, Andi Kleen,
	linux-kernel, linux-perf-users


Enable complex event names containing [.:=,] symbols to be encoded into Perf 
trace using name= modifier e.g. like this:

perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex

Below is how it looks like in the report output. Please note explicit escaped 
quoting at cmdline string in the header so that thestring can be directly reused
for another collection in shell:

perf report --header

# ========
...
# cmdline : /root/abudanko/kernel/tip/tools/perf/perf record -v -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex 
# event : name = OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM, , type = 4, size = 112, config = 0x100003c, { sample_period, sample_freq } = 3500000, sample_type = IP|TID|TIME, disabled = 1, inh
...
# ========
#
#
# Total Lost Samples: 0
#
# Samples: 24K of event 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM'
# Event count (approx.): 86492000000
#
# Overhead  Command  Shared Object     Symbol                                        
# ........  .......  ................  ..............................................
#
    14.75%  futex    [kernel.vmlinux]  [k] __entry_trampoline_start
...

perf stat -e cpu/name=\'CPU_CLK_UNHALTED.THREAD:cmask=0x1\',period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex

10000000 process context switches in 16678890291ns (1667.9ns/ctxsw)

 Performance counter stats for './futex':

    88,095,770,571      CPU_CLK_UNHALTED.THREAD:cmask=0x1                                   

      16.679542407 seconds time elapsed

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
---
Changes in v3:
- updated event name syntax in perf list docs;
- added name modifier description into perf record docs;

Changes in v2:
- aligned break according to coding guidelines;
- put comment regarding expected quoting rules; 
---
 tools/perf/Documentation/perf-list.txt   |  6 +++++-
 tools/perf/Documentation/perf-record.txt |  3 +++
 tools/perf/util/header.c                 | 20 ++++++++++++++++++--
 tools/perf/util/parse-events.l           | 18 +++++++++++++++++-
 4 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 2549c34a7895..11300dbe35c5 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -124,7 +124,11 @@ The available PMUs and their raw parameters can be listed with
 For example the raw event "LSD.UOPS" core pmu event above could
 be specified as
 
-  perf stat -e cpu/event=0xa8,umask=0x1,name=LSD.UOPS_CYCLES,cmask=1/ ...
+  perf stat -e cpu/event=0xa8,umask=0x1,name=LSD.UOPS_CYCLES,cmask=0x1/ ...
+
+  or using extended name syntax
+
+  perf stat -e cpu/event=0xa8,umask=0x1,cmask=0x1,name=\'LSD.UOPS_CYCLES:cmask=0x1\'/ ...
 
 PER SOCKET PMUS
 ---------------
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index cc37b3a4be76..04168da4268e 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -57,6 +57,9 @@ OPTIONS
 			 FP mode, "dwarf" for DWARF mode, "lbr" for LBR mode and
 			 "no" for disable callgraph.
 	  - 'stack-size': user stack size for dwarf mode
+	  - 'name' : User defined event name. Single quotes (') may be used to
+		    escape symbols in the name from parsing by shell and tool
+		    like this: name=\'CPU_CLK_UNHALTED.THREAD:cmask=0x1\'.
 
           See the linkperf:perf-list[1] man page for more parameters.
 
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a8bff2178fbc..08176e8cb3b4 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1459,8 +1459,24 @@ static void print_cmdline(struct feat_fd *ff, FILE *fp)
 
 	fprintf(fp, "# cmdline : ");
 
-	for (i = 0; i < nr; i++)
-		fprintf(fp, "%s ", ff->ph->env.cmdline_argv[i]);
+	for (i = 0; i < nr; i++) {
+		char *argv_i = strdup(ff->ph->env.cmdline_argv[i]);
+		if (!argv_i) {
+			fprintf(fp, "%s ", ff->ph->env.cmdline_argv[i]);
+		} else {
+			char *mem = argv_i;
+			do {
+				char *quote = strchr(argv_i, '\'');
+				if (!quote)
+					break;
+				*quote++ = '\0';
+				fprintf(fp, "%s\\\'", argv_i);
+				argv_i = quote;
+			} while (1);
+			fprintf(fp, "%s ", argv_i);
+			free(mem);
+		}
+	}
 	fputc('\n', fp);
 }
 
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index a1a01b1ac8b8..5f761f3ed0f3 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -53,7 +53,21 @@ static int str(yyscan_t scanner, int token)
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
 	char *text = parse_events_get_text(scanner);
 
-	yylval->str = strdup(text);
+	if (text[0] != '\'') {
+		yylval->str = strdup(text);
+	} else {
+		/*
+		 * If a text tag specified on the command line
+		 * contains opening single quite ' then it is
+		 * expected that the tag ends with single quote
+		 * as well, like this:
+		 *     name=\'CPU_CLK_UNHALTED.THREAD:cmask=1\'
+		 * quotes need to be escaped to bypass shell
+		 * processing.
+		 */
+		yylval->str = strndup(&text[1], strlen(text) - 2);
+	}
+
 	return token;
 }
 
@@ -176,6 +190,7 @@ num_dec		[0-9]+
 num_hex		0x[a-fA-F0-9]+
 num_raw_hex	[a-fA-F0-9]+
 name		[a-zA-Z_*?\[\]][a-zA-Z0-9_*?.\[\]]*
+name_tag	[\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
 name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
 drv_cfg_term	[a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
 /* If you add a modifier you need to update check_modifier() */
@@ -344,6 +359,7 @@ r{num_raw_hex}		{ return raw(yyscanner); }
 {bpf_object}		{ if (!isbpf(yyscanner)) { USER_REJECT }; return str(yyscanner, PE_BPF_OBJECT); }
 {bpf_source}		{ if (!isbpf(yyscanner)) { USER_REJECT }; return str(yyscanner, PE_BPF_SOURCE); }
 {name}			{ return pmu_str_check(yyscanner); }
+{name_tag}		{ return str(yyscanner, PE_NAME); }
 "/"			{ BEGIN(config); return '/'; }
 -			{ return '-'; }
 ,			{ BEGIN(event); return ','; }

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04  6:50 [PATCH v3]: perf record: enable arbitrary event names thru name= modifier Alexey Budankov
@ 2018-06-04  7:56 ` Jiri Olsa
  2018-06-04 13:58   ` Arnaldo Carvalho de Melo
  2018-06-07  8:18 ` [tip:perf/urgent] perf record: Enable " tip-bot for Alexey Budankov
  1 sibling, 1 reply; 12+ messages in thread
From: Jiri Olsa @ 2018-06-04  7:56 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Namhyung Kim, Andi Kleen, linux-kernel,
	linux-perf-users

On Mon, Jun 04, 2018 at 09:50:56AM +0300, Alexey Budankov wrote:
> 
> Enable complex event names containing [.:=,] symbols to be encoded into Perf 
> trace using name= modifier e.g. like this:
> 
> perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
> 		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
> 
> Below is how it looks like in the report output. Please note explicit escaped 
> quoting at cmdline string in the header so that thestring can be directly reused
> for another collection in shell:
> 
> perf report --header
> 
> # ========
> ...
> # cmdline : /root/abudanko/kernel/tip/tools/perf/perf record -v -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex 
> # event : name = OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM, , type = 4, size = 112, config = 0x100003c, { sample_period, sample_freq } = 3500000, sample_type = IP|TID|TIME, disabled = 1, inh
> ...
> # ========
> #
> #
> # Total Lost Samples: 0
> #
> # Samples: 24K of event 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM'
> # Event count (approx.): 86492000000
> #
> # Overhead  Command  Shared Object     Symbol                                        
> # ........  .......  ................  ..............................................
> #
>     14.75%  futex    [kernel.vmlinux]  [k] __entry_trampoline_start
> ...
> 
> perf stat -e cpu/name=\'CPU_CLK_UNHALTED.THREAD:cmask=0x1\',period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
> 
> 10000000 process context switches in 16678890291ns (1667.9ns/ctxsw)
> 
>  Performance counter stats for './futex':
> 
>     88,095,770,571      CPU_CLK_UNHALTED.THREAD:cmask=0x1                                   
> 
>       16.679542407 seconds time elapsed
> 
> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
> ---
> Changes in v3:
> - updated event name syntax in perf list docs;
> - added name modifier description into perf record docs;
> 

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04  7:56 ` Jiri Olsa
@ 2018-06-04 13:58   ` Arnaldo Carvalho de Melo
  2018-06-04 14:19     ` Alexey Budankov
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-06-04 13:58 UTC (permalink / raw)
  To: Jiri Olsa, Alexey Budankov
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
	Andi Kleen, linux-kernel, linux-perf-users

Em Mon, Jun 04, 2018 at 09:56:02AM +0200, Jiri Olsa escreveu:
> On Mon, Jun 04, 2018 at 09:50:56AM +0300, Alexey Budankov wrote:
> > Enable complex event names containing [.:=,] symbols to be encoded into Perf 
> > trace using name= modifier e.g. like this:

> > perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
> > 		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex

> > Below is how it looks like in the report output. Please note explicit escaped 
> > quoting at cmdline string in the header so that thestring can be directly reused
> > for another collection in shell:

Applied, but there are other places where we show event names, such as:

[root@jouet ~]# perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk
^C[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 1.599 MB perf.data (704 samples) ]

[root@jouet ~]# perf evlist
OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM
[root@jouet ~]# perf evlist -v
OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM: type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
[root@jouet ~]# 

That I used to check if the period, etc were correctly set, etc. Perhaps
we should add that \'\' there as well?

Also please consider adding an entry to tools/perf/tests/attr/ to make
sure this is checked everytime we run 'perf test attr' or plain 'perf
test'.

Those can be followup patches, so I'm applying this one, thanks.

- Arnaldo

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04 13:58   ` Arnaldo Carvalho de Melo
@ 2018-06-04 14:19     ` Alexey Budankov
  2018-06-04 14:23       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Alexey Budankov @ 2018-06-04 14:19 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Peter Zijlstra, Ingo Molnar, Alexander Shishkin, Namhyung Kim,
	Andi Kleen, linux-kernel, linux-perf-users

Hi,

On 04.06.2018 16:58, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 04, 2018 at 09:56:02AM +0200, Jiri Olsa escreveu:
>> On Mon, Jun 04, 2018 at 09:50:56AM +0300, Alexey Budankov wrote:
>>> Enable complex event names containing [.:=,] symbols to be encoded into Perf 
>>> trace using name= modifier e.g. like this:
> 
>>> perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
>>> 		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
> 
>>> Below is how it looks like in the report output. Please note explicit escaped 
>>> quoting at cmdline string in the header so that thestring can be directly reused
>>> for another collection in shell:
> 
> Applied, but there are other places where we show event names, such as:
> 
> [root@jouet ~]# perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk
> ^C[ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 1.599 MB perf.data (704 samples) ]
> 
> [root@jouet ~]# perf evlist
> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM
> [root@jouet ~]# perf evlist -v
> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM: type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
> [root@jouet ~]# 
> 
> That I used to check if the period, etc were correctly set, etc. Perhaps
> we should add that \'\' there as well?

Like this?

name: 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM', type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1

Thanks,
Alexey

> 
> Also please consider adding an entry to tools/perf/tests/attr/ to make
> sure this is checked everytime we run 'perf test attr' or plain 'perf
> test'.
> 
> Those can be followup patches, so I'm applying this one, thanks.
> 
> - Arnaldo
> 

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04 14:19     ` Alexey Budankov
@ 2018-06-04 14:23       ` Arnaldo Carvalho de Melo
  2018-06-04 14:51         ` Alexey Budankov
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-06-04 14:23 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel, linux-perf-users

Em Mon, Jun 04, 2018 at 05:19:11PM +0300, Alexey Budankov escreveu:
> Hi,
> 
> On 04.06.2018 16:58, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jun 04, 2018 at 09:56:02AM +0200, Jiri Olsa escreveu:
> >> On Mon, Jun 04, 2018 at 09:50:56AM +0300, Alexey Budankov wrote:
> >>> Enable complex event names containing [.:=,] symbols to be encoded into Perf 
> >>> trace using name= modifier e.g. like this:
> > 
> >>> perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
> >>> 		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
> > 
> >>> Below is how it looks like in the report output. Please note explicit escaped 
> >>> quoting at cmdline string in the header so that thestring can be directly reused
> >>> for another collection in shell:
> > 
> > Applied, but there are other places where we show event names, such as:
> > 
> > [root@jouet ~]# perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk
> > ^C[ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 1.599 MB perf.data (704 samples) ]
> > 
> > [root@jouet ~]# perf evlist
> > OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM
> > [root@jouet ~]# perf evlist -v
> > OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM: type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
> > [root@jouet ~]# 
> > 
> > That I used to check if the period, etc were correctly set, etc. Perhaps
> > we should add that \'\' there as well?
> 
> Like this?
> 
> name: 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM', type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1

Nope, I mean with the same intent you had when showing escaped single
quotes in the 'perf report' header, that users would copy'n'paste and
get something that works on the command line?

- Arnaldo
 
> Thanks,
> Alexey
> 
> > 
> > Also please consider adding an entry to tools/perf/tests/attr/ to make
> > sure this is checked everytime we run 'perf test attr' or plain 'perf
> > test'.
> > 
> > Those can be followup patches, so I'm applying this one, thanks.
> > 
> > - Arnaldo
> > 

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04 14:23       ` Arnaldo Carvalho de Melo
@ 2018-06-04 14:51         ` Alexey Budankov
  2018-06-04 14:58           ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Alexey Budankov @ 2018-06-04 14:51 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel, linux-perf-users

Hi,
On 04.06.2018 17:23, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 04, 2018 at 05:19:11PM +0300, Alexey Budankov escreveu:
>> Hi,
>>
>> On 04.06.2018 16:58, Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Jun 04, 2018 at 09:56:02AM +0200, Jiri Olsa escreveu:
>>>> On Mon, Jun 04, 2018 at 09:50:56AM +0300, Alexey Budankov wrote:
>>>>> Enable complex event names containing [.:=,] symbols to be encoded into Perf 
>>>>> trace using name= modifier e.g. like this:
>>>
>>>>> perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
>>>>> 		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
>>>
>>>>> Below is how it looks like in the report output. Please note explicit escaped 
>>>>> quoting at cmdline string in the header so that thestring can be directly reused
>>>>> for another collection in shell:
>>>
>>> Applied, but there are other places where we show event names, such as:
>>>
>>> [root@jouet ~]# perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk
>>> ^C[ perf record: Woken up 1 times to write data ]
>>> [ perf record: Captured and wrote 1.599 MB perf.data (704 samples) ]
>>>
>>> [root@jouet ~]# perf evlist
>>> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM
>>> [root@jouet ~]# perf evlist -v
>>> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM: type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
>>> [root@jouet ~]# 
>>>
>>> That I used to check if the period, etc were correctly set, etc. Perhaps
>>> we should add that \'\' there as well?
>>
>> Like this?
>>
>> name: 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM', type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
> 
> Nope, I mean with the same intent you had when showing escaped single
> quotes in the 'perf report' header, that users would copy'n'paste and
> get something that works on the command line?

Well ok, if there are other cases aside perf report that tool shows full 
command line ready for reusing then it definitely makes sense to print 
event names quoted there. If such cases were pointed out then they also 
could be addressed along with the unit/regression testing mentioned above.

Thanks,
Alexey

> 
> - Arnaldo
>  
>> Thanks,
>> Alexey
>>
>>>
>>> Also please consider adding an entry to tools/perf/tests/attr/ to make
>>> sure this is checked everytime we run 'perf test attr' or plain 'perf
>>> test'.
>>>
>>> Those can be followup patches, so I'm applying this one, thanks.
>>>
>>> - Arnaldo
>>>
> 

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04 14:51         ` Alexey Budankov
@ 2018-06-04 14:58           ` Arnaldo Carvalho de Melo
  2018-06-04 15:22             ` Alexey Budankov
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-06-04 14:58 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel, linux-perf-users

Em Mon, Jun 04, 2018 at 05:51:03PM +0300, Alexey Budankov escreveu:
> Hi,
> On 04.06.2018 17:23, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jun 04, 2018 at 05:19:11PM +0300, Alexey Budankov escreveu:
> >> Hi,
> >>
> >> On 04.06.2018 16:58, Arnaldo Carvalho de Melo wrote:
> >>> Em Mon, Jun 04, 2018 at 09:56:02AM +0200, Jiri Olsa escreveu:
> >>>> On Mon, Jun 04, 2018 at 09:50:56AM +0300, Alexey Budankov wrote:
> >>>>> Enable complex event names containing [.:=,] symbols to be encoded into Perf 
> >>>>> trace using name= modifier e.g. like this:
> >>>
> >>>>> perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
> >>>>> 		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
> >>>
> >>>>> Below is how it looks like in the report output. Please note explicit escaped 
> >>>>> quoting at cmdline string in the header so that thestring can be directly reused
> >>>>> for another collection in shell:
> >>>
> >>> Applied, but there are other places where we show event names, such as:
> >>>
> >>> [root@jouet ~]# perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk
> >>> ^C[ perf record: Woken up 1 times to write data ]
> >>> [ perf record: Captured and wrote 1.599 MB perf.data (704 samples) ]
> >>>
> >>> [root@jouet ~]# perf evlist
> >>> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM
> >>> [root@jouet ~]# perf evlist -v
> >>> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM: type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
> >>> [root@jouet ~]# 
> >>>
> >>> That I used to check if the period, etc were correctly set, etc. Perhaps
> >>> we should add that \'\' there as well?
> >>
> >> Like this?
> >>
> >> name: 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM', type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
> > 
> > Nope, I mean with the same intent you had when showing escaped single
> > quotes in the 'perf report' header, that users would copy'n'paste and
> > get something that works on the command line?
> 
> Well ok, if there are other cases aside perf report that tool shows full 
> command line ready for reusing then it definitely makes sense to print 

Ok, lets leave it like you did, i.e. just when command lines appear and
people are likely to copy'n'paste them to repeat it.

> event names quoted there. If such cases were pointed out then they also 
> could be addressed along with the unit/regression testing mentioned above.

So lests stick to just the unit/regression testing, can you take a look
at that?

- Arnaldo

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04 14:58           ` Arnaldo Carvalho de Melo
@ 2018-06-04 15:22             ` Alexey Budankov
  2018-06-04 19:16               ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Alexey Budankov @ 2018-06-04 15:22 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel, linux-perf-users

On 04.06.2018 17:58, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 04, 2018 at 05:51:03PM +0300, Alexey Budankov escreveu:
>> Hi,
>> On 04.06.2018 17:23, Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Jun 04, 2018 at 05:19:11PM +0300, Alexey Budankov escreveu:
>>>> Hi,
>>>>
>>>> On 04.06.2018 16:58, Arnaldo Carvalho de Melo wrote:
>>>>> Em Mon, Jun 04, 2018 at 09:56:02AM +0200, Jiri Olsa escreveu:
>>>>>> On Mon, Jun 04, 2018 at 09:50:56AM +0300, Alexey Budankov wrote:
>>>>>>> Enable complex event names containing [.:=,] symbols to be encoded into Perf 
>>>>>>> trace using name= modifier e.g. like this:
>>>>>
>>>>>>> perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
>>>>>>> 		period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
>>>>>
>>>>>>> Below is how it looks like in the report output. Please note explicit escaped 
>>>>>>> quoting at cmdline string in the header so that thestring can be directly reused
>>>>>>> for another collection in shell:
>>>>>
>>>>> Applied, but there are other places where we show event names, such as:
>>>>>
>>>>> [root@jouet ~]# perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk
>>>>> ^C[ perf record: Woken up 1 times to write data ]
>>>>> [ perf record: Captured and wrote 1.599 MB perf.data (704 samples) ]
>>>>>
>>>>> [root@jouet ~]# perf evlist
>>>>> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM
>>>>> [root@jouet ~]# perf evlist -v
>>>>> OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM: type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
>>>>> [root@jouet ~]# 
>>>>>
>>>>> That I used to check if the period, etc were correctly set, etc. Perhaps
>>>>> we should add that \'\' there as well?
>>>>
>>>> Like this?
>>>>
>>>> name: 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM', type: 4, size: 112, config: 0x100003c, { sample_period, sample_freq }: 3500000, sample_type: IP|TID|TIME|CPU, disabled: 1, inherit: 1, pinned: 1, exclude_hv: 1, mmap: 1, comm: 1, task: 1, sample_id_all: 1, mmap2: 1, comm_exec: 1
>>>
>>> Nope, I mean with the same intent you had when showing escaped single
>>> quotes in the 'perf report' header, that users would copy'n'paste and
>>> get something that works on the command line?
>>
>> Well ok, if there are other cases aside perf report that tool shows full 
>> command line ready for reusing then it definitely makes sense to print 
> 
> Ok, lets leave it like you did, i.e. just when command lines appear and
> people are likely to copy'n'paste them to repeat it.
> 
>> event names quoted there. If such cases were pointed out then they also 
>> could be addressed along with the unit/regression testing mentioned above.
> 
> So lests stick to just the unit/regression testing, can you take a look
> at that?

Sure. Is it enough to run perf record with some quoted event name to make 
sure it runs successfully?

Thanks,
Alexey

> 
> - Arnaldo
> 

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04 15:22             ` Alexey Budankov
@ 2018-06-04 19:16               ` Arnaldo Carvalho de Melo
  2018-06-05  6:00                 ` Alexey Budankov
  0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-06-04 19:16 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel, linux-perf-users

Em Mon, Jun 04, 2018 at 06:22:43PM +0300, Alexey Budankov escreveu:
> On 04.06.2018 17:58, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jun 04, 2018 at 05:51:03PM +0300, Alexey Budankov escreveu:
> >> event names quoted there. If such cases were pointed out then they also 
> >> could be addressed along with the unit/regression testing mentioned above.

> > So lests stick to just the unit/regression testing, can you take a look
> > at that?
 
> Sure. Is it enough to run perf record with some quoted event name to make 
> sure it runs successfully?

I mentioned tools/perf/tests/attr/, for checking if the perf_event_attr
was setup the way you intended, and tools/perf/tests/parse-events.c,
which probably is what we want here, see, for instance, this cset I've
added recently to have we testing intel_pt event parsing:

$ git show b3f58c8da64bc63bd0c0a06a4e2cf258a3d20be6
commit b3f58c8da64bc63bd0c0a06a4e2cf258a3d20be6
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Fri May 11 11:48:54 2018 -0300

    perf tests parse-events: Add intel_pt parse test
    
    To avoid regressions such as the one fixed by 4a35a9027f64 ("Revert
    "perf pmu: Fix pmu events parsing rule""), where '-e intel_pt//u' got
    broken, with this new entry in this 'perf tests' subtest, we would have
    caught it before pushing upstream.
    
    Acked-by: Jiri Olsa <jolsa@kernel.org>
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
    Cc: Andi Kleen <ak@linux.intel.com>
    Cc: David Ahern <dsahern@gmail.com>
    Cc: Jiri Olsa <jolsa@kernel.org>
    Cc: Kan Liang <kan.liang@linux.intel.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Wang Nan <wangnan0@huawei.com>
    Link: https://lkml.kernel.org/n/tip-kw62fys9bwdgsp722so2ln1l@git.kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 18b06444f230..6829dd416a99 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1309,6 +1309,14 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
 	return 0;
 }
 
+static int test__intel_pt(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel = perf_evlist__first(evlist);
+
+	TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "intel_pt//u") == 0);
+	return 0;
+}
+
 static int count_tracepoints(void)
 {
 	struct dirent *events_ent;
@@ -1637,6 +1645,11 @@ static struct evlist_test test__events[] = {
 		.check = test__checkevent_config_cache,
 		.id    = 51,
 	},
+	{
+		.name  = "intel_pt//u",
+		.check = test__intel_pt,
+		.id    = 52,
+	},
 };
 
 static struct evlist_test test__events_pmu[] = {

See also this one:

static int
test__checkevent_breakpoint_len_rw_modifier(struct perf_evlist *evlist)
{
        struct perf_evsel *evsel = perf_evlist__first(evlist);

        TEST_ASSERT_VAL("wrong exclude_user", !evsel->attr.exclude_user);
        TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
        TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
        TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);

        return test__checkevent_breakpoint_rw(evlist);
}
        {
                .name  = "mem:0/4:rw:u",
                .check = test__checkevent_breakpoint_len_rw_modifier,
                .id    = 44
        },


I.e. have as many TEST_ASSERT_VAL as you need, checking how parsing the
event name ends up setting up the evsel, evlist and perf_event_attr.

- Arnaldo

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-04 19:16               ` Arnaldo Carvalho de Melo
@ 2018-06-05  6:00                 ` Alexey Budankov
  2018-06-05 15:33                   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: Alexey Budankov @ 2018-06-05  6:00 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel, linux-perf-users

Hi,
On 04.06.2018 22:16, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 04, 2018 at 06:22:43PM +0300, Alexey Budankov escreveu:
>> On 04.06.2018 17:58, Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Jun 04, 2018 at 05:51:03PM +0300, Alexey Budankov escreveu:
>>>> event names quoted there. If such cases were pointed out then they also 
>>>> could be addressed along with the unit/regression testing mentioned above.
> 
>>> So lests stick to just the unit/regression testing, can you take a look
>>> at that?
>  
>> Sure. Is it enough to run perf record with some quoted event name to make 
>> sure it runs successfully?
> 
> I mentioned tools/perf/tests/attr/, for checking if the perf_event_attr
> was setup the way you intended, and tools/perf/tests/parse-events.c,
> which probably is what we want here, see, for instance, this cset I've
> added recently to have we testing intel_pt event parsing:
> 
> $ git show b3f58c8da64bc63bd0c0a06a4e2cf258a3d20be6
> commit b3f58c8da64bc63bd0c0a06a4e2cf258a3d20be6
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Fri May 11 11:48:54 2018 -0300
> 
>     perf tests parse-events: Add intel_pt parse test
>     
>     To avoid regressions such as the one fixed by 4a35a9027f64 ("Revert
>     "perf pmu: Fix pmu events parsing rule""), where '-e intel_pt//u' got
>     broken, with this new entry in this 'perf tests' subtest, we would have
>     caught it before pushing upstream.
>     
>     Acked-by: Jiri Olsa <jolsa@kernel.org>
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>     Cc: Andi Kleen <ak@linux.intel.com>
>     Cc: David Ahern <dsahern@gmail.com>
>     Cc: Jiri Olsa <jolsa@kernel.org>
>     Cc: Kan Liang <kan.liang@linux.intel.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Cc: Wang Nan <wangnan0@huawei.com>
>     Link: https://lkml.kernel.org/n/tip-kw62fys9bwdgsp722so2ln1l@git.kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 18b06444f230..6829dd416a99 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1309,6 +1309,14 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
>  	return 0;
>  }
>  
> +static int test__intel_pt(struct perf_evlist *evlist)
> +{
> +	struct perf_evsel *evsel = perf_evlist__first(evlist);
> +
> +	TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "intel_pt//u") == 0);
> +	return 0;
> +}
> +
>  static int count_tracepoints(void)
>  {
>  	struct dirent *events_ent;
> @@ -1637,6 +1645,11 @@ static struct evlist_test test__events[] = {
>  		.check = test__checkevent_config_cache,
>  		.id    = 51,
>  	},
> +	{
> +		.name  = "intel_pt//u",
> +		.check = test__intel_pt,
> +		.id    = 52,
> +	},
>  };
>  
>  static struct evlist_test test__events_pmu[] = {
> 
> See also this one:
> 
> static int
> test__checkevent_breakpoint_len_rw_modifier(struct perf_evlist *evlist)
> {
>         struct perf_evsel *evsel = perf_evlist__first(evlist);
> 
>         TEST_ASSERT_VAL("wrong exclude_user", !evsel->attr.exclude_user);
>         TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
>         TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
>         TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
> 
>         return test__checkevent_breakpoint_rw(evlist);
> }
>         {
>                 .name  = "mem:0/4:rw:u",
>                 .check = test__checkevent_breakpoint_len_rw_modifier,
>                 .id    = 44
>         },
> 
> 
> I.e. have as many TEST_ASSERT_VAL as you need, checking how parsing the
> event name ends up setting up the evsel, evlist and perf_event_attr.

Ok, I see. But please expect delay in delivery since I am on vacations till 06/18.

Thanks,
Alexey

> 
> - Arnaldo
> 

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

* Re: [PATCH v3]: perf record: enable arbitrary event names thru name= modifier
  2018-06-05  6:00                 ` Alexey Budankov
@ 2018-06-05 15:33                   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-06-05 15:33 UTC (permalink / raw)
  To: Alexey Budankov
  Cc: Jiri Olsa, Peter Zijlstra, Ingo Molnar, Alexander Shishkin,
	Namhyung Kim, Andi Kleen, linux-kernel, linux-perf-users

Em Tue, Jun 05, 2018 at 09:00:17AM +0300, Alexey Budankov escreveu:
> Hi,
> On 04.06.2018 22:16, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Jun 04, 2018 at 06:22:43PM +0300, Alexey Budankov escreveu:
> >> On 04.06.2018 17:58, Arnaldo Carvalho de Melo wrote:
> >>> Em Mon, Jun 04, 2018 at 05:51:03PM +0300, Alexey Budankov escreveu:
> >>>> event names quoted there. If such cases were pointed out then they also 
> >>>> could be addressed along with the unit/regression testing mentioned above.
> > 
> >>> So lests stick to just the unit/regression testing, can you take a look
> >>> at that?
> >  
> >> Sure. Is it enough to run perf record with some quoted event name to make 
> >> sure it runs successfully?
> > 
> > I mentioned tools/perf/tests/attr/, for checking if the perf_event_attr
> > was setup the way you intended, and tools/perf/tests/parse-events.c,
> > which probably is what we want here, see, for instance, this cset I've
> > added recently to have we testing intel_pt event parsing:
> > 
> > $ git show b3f58c8da64bc63bd0c0a06a4e2cf258a3d20be6
> > commit b3f58c8da64bc63bd0c0a06a4e2cf258a3d20be6
> > Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Date:   Fri May 11 11:48:54 2018 -0300
> > 
> >     perf tests parse-events: Add intel_pt parse test
> >     
> >     To avoid regressions such as the one fixed by 4a35a9027f64 ("Revert
> >     "perf pmu: Fix pmu events parsing rule""), where '-e intel_pt//u' got
> >     broken, with this new entry in this 'perf tests' subtest, we would have
> >     caught it before pushing upstream.
> >     
> >     Acked-by: Jiri Olsa <jolsa@kernel.org>
> >     Cc: Adrian Hunter <adrian.hunter@intel.com>
> >     Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> >     Cc: Andi Kleen <ak@linux.intel.com>
> >     Cc: David Ahern <dsahern@gmail.com>
> >     Cc: Jiri Olsa <jolsa@kernel.org>
> >     Cc: Kan Liang <kan.liang@linux.intel.com>
> >     Cc: Namhyung Kim <namhyung@kernel.org>
> >     Cc: Peter Zijlstra <peterz@infradead.org>
> >     Cc: Wang Nan <wangnan0@huawei.com>
> >     Link: https://lkml.kernel.org/n/tip-kw62fys9bwdgsp722so2ln1l@git.kernel.org
> >     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> > index 18b06444f230..6829dd416a99 100644
> > --- a/tools/perf/tests/parse-events.c
> > +++ b/tools/perf/tests/parse-events.c
> > @@ -1309,6 +1309,14 @@ static int test__checkevent_config_cache(struct perf_evlist *evlist)
> >  	return 0;
> >  }
> >  
> > +static int test__intel_pt(struct perf_evlist *evlist)
> > +{
> > +	struct perf_evsel *evsel = perf_evlist__first(evlist);
> > +
> > +	TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "intel_pt//u") == 0);
> > +	return 0;
> > +}
> > +
> >  static int count_tracepoints(void)
> >  {
> >  	struct dirent *events_ent;
> > @@ -1637,6 +1645,11 @@ static struct evlist_test test__events[] = {
> >  		.check = test__checkevent_config_cache,
> >  		.id    = 51,
> >  	},
> > +	{
> > +		.name  = "intel_pt//u",
> > +		.check = test__intel_pt,
> > +		.id    = 52,
> > +	},
> >  };
> >  
> >  static struct evlist_test test__events_pmu[] = {
> > 
> > See also this one:
> > 
> > static int
> > test__checkevent_breakpoint_len_rw_modifier(struct perf_evlist *evlist)
> > {
> >         struct perf_evsel *evsel = perf_evlist__first(evlist);
> > 
> >         TEST_ASSERT_VAL("wrong exclude_user", !evsel->attr.exclude_user);
> >         TEST_ASSERT_VAL("wrong exclude_kernel", evsel->attr.exclude_kernel);
> >         TEST_ASSERT_VAL("wrong exclude_hv", evsel->attr.exclude_hv);
> >         TEST_ASSERT_VAL("wrong precise_ip", !evsel->attr.precise_ip);
> > 
> >         return test__checkevent_breakpoint_rw(evlist);
> > }
> >         {
> >                 .name  = "mem:0/4:rw:u",
> >                 .check = test__checkevent_breakpoint_len_rw_modifier,
> >                 .id    = 44
> >         },
> > 
> > 
> > I.e. have as many TEST_ASSERT_VAL as you need, checking how parsing the
> > event name ends up setting up the evsel, evlist and perf_event_attr.
> 
> Ok, I see. But please expect delay in delivery since I am on vacations till 06/18.

np.

- Arnaldo

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

* [tip:perf/urgent] perf record: Enable arbitrary event names thru name= modifier
  2018-06-04  6:50 [PATCH v3]: perf record: enable arbitrary event names thru name= modifier Alexey Budankov
  2018-06-04  7:56 ` Jiri Olsa
@ 2018-06-07  8:18 ` tip-bot for Alexey Budankov
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for Alexey Budankov @ 2018-06-07  8:18 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, ak, alexander.shishkin, jolsa, peterz, acme,
	namhyung, tglx, alexey.budankov, hpa, mingo

Commit-ID:  f92da71280fb8da3a7c489e08a096f0b8715f939
Gitweb:     https://git.kernel.org/tip/f92da71280fb8da3a7c489e08a096f0b8715f939
Author:     Alexey Budankov <alexey.budankov@linux.intel.com>
AuthorDate: Mon, 4 Jun 2018 09:50:56 +0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 6 Jun 2018 12:52:04 -0300

perf record: Enable arbitrary event names thru name= modifier

Enable complex event names containing [.:=,] symbols to be encoded into Perf
trace using name= modifier e.g. like this:

  perf record -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',\
		  period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex

Below is how it looks like in the report output. Please note explicit escaped
quoting at cmdline string in the header so that thestring can be directly reused
for another collection in shell:

perf report --header

  # ========
  ...
  # cmdline : /root/abudanko/kernel/tip/tools/perf/perf record -v -e cpu/name=\'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM\',period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex
  # event : name = OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM, , type = 4, size = 112, config = 0x100003c, { sample_period, sample_freq } = 3500000, sample_type = IP|TID|TIME, disabled = 1, inh
  ...
  # ========
  #
  #
  # Total Lost Samples: 0
  #
  # Samples: 24K of event 'OFFCORE_RESPONSE:request=DEMAND_RFO:response=L3_HIT.SNOOP_HITM'
  # Event count (approx.): 86492000000
  #
  # Overhead  Command  Shared Object     Symbol
  # ........  .......  ................  ..............................................
  #
      14.75%  futex    [kernel.vmlinux]  [k] __entry_trampoline_start
...

  perf stat -e cpu/name=\'CPU_CLK_UNHALTED.THREAD:cmask=0x1\',period=0x3567e0,event=0x3c,cmask=0x1/Duk ./futex

  10000000 process context switches in 16678890291ns (1667.9ns/ctxsw)

   Performance counter stats for './futex':

      88,095,770,571      CPU_CLK_UNHALTED.THREAD:cmask=0x1

        16.679542407 seconds time elapsed

Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com>
Acked-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/c194b060-761d-0d50-3b21-bb4ed680002d@linux.intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-list.txt   |  6 +++++-
 tools/perf/Documentation/perf-record.txt |  3 +++
 tools/perf/util/header.c                 | 20 ++++++++++++++++++--
 tools/perf/util/parse-events.l           | 18 +++++++++++++++++-
 4 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 2549c34a7895..11300dbe35c5 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -124,7 +124,11 @@ The available PMUs and their raw parameters can be listed with
 For example the raw event "LSD.UOPS" core pmu event above could
 be specified as
 
-  perf stat -e cpu/event=0xa8,umask=0x1,name=LSD.UOPS_CYCLES,cmask=1/ ...
+  perf stat -e cpu/event=0xa8,umask=0x1,name=LSD.UOPS_CYCLES,cmask=0x1/ ...
+
+  or using extended name syntax
+
+  perf stat -e cpu/event=0xa8,umask=0x1,cmask=0x1,name=\'LSD.UOPS_CYCLES:cmask=0x1\'/ ...
 
 PER SOCKET PMUS
 ---------------
diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index cc37b3a4be76..04168da4268e 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -57,6 +57,9 @@ OPTIONS
 			 FP mode, "dwarf" for DWARF mode, "lbr" for LBR mode and
 			 "no" for disable callgraph.
 	  - 'stack-size': user stack size for dwarf mode
+	  - 'name' : User defined event name. Single quotes (') may be used to
+		    escape symbols in the name from parsing by shell and tool
+		    like this: name=\'CPU_CLK_UNHALTED.THREAD:cmask=0x1\'.
 
           See the linkperf:perf-list[1] man page for more parameters.
 
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 2625cc38a0d6..540cd2dcd3e7 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1459,8 +1459,24 @@ static void print_cmdline(struct feat_fd *ff, FILE *fp)
 
 	fprintf(fp, "# cmdline : ");
 
-	for (i = 0; i < nr; i++)
-		fprintf(fp, "%s ", ff->ph->env.cmdline_argv[i]);
+	for (i = 0; i < nr; i++) {
+		char *argv_i = strdup(ff->ph->env.cmdline_argv[i]);
+		if (!argv_i) {
+			fprintf(fp, "%s ", ff->ph->env.cmdline_argv[i]);
+		} else {
+			char *mem = argv_i;
+			do {
+				char *quote = strchr(argv_i, '\'');
+				if (!quote)
+					break;
+				*quote++ = '\0';
+				fprintf(fp, "%s\\\'", argv_i);
+				argv_i = quote;
+			} while (1);
+			fprintf(fp, "%s ", argv_i);
+			free(mem);
+		}
+	}
 	fputc('\n', fp);
 }
 
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index a1a01b1ac8b8..5f761f3ed0f3 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -53,7 +53,21 @@ static int str(yyscan_t scanner, int token)
 	YYSTYPE *yylval = parse_events_get_lval(scanner);
 	char *text = parse_events_get_text(scanner);
 
-	yylval->str = strdup(text);
+	if (text[0] != '\'') {
+		yylval->str = strdup(text);
+	} else {
+		/*
+		 * If a text tag specified on the command line
+		 * contains opening single quite ' then it is
+		 * expected that the tag ends with single quote
+		 * as well, like this:
+		 *     name=\'CPU_CLK_UNHALTED.THREAD:cmask=1\'
+		 * quotes need to be escaped to bypass shell
+		 * processing.
+		 */
+		yylval->str = strndup(&text[1], strlen(text) - 2);
+	}
+
 	return token;
 }
 
@@ -176,6 +190,7 @@ num_dec		[0-9]+
 num_hex		0x[a-fA-F0-9]+
 num_raw_hex	[a-fA-F0-9]+
 name		[a-zA-Z_*?\[\]][a-zA-Z0-9_*?.\[\]]*
+name_tag	[\'][a-zA-Z_*?\[\]][a-zA-Z0-9_*?\-,\.\[\]:=]*[\']
 name_minus	[a-zA-Z_*?][a-zA-Z0-9\-_*?.:]*
 drv_cfg_term	[a-zA-Z0-9_\.]+(=[a-zA-Z0-9_*?\.:]+)?
 /* If you add a modifier you need to update check_modifier() */
@@ -344,6 +359,7 @@ r{num_raw_hex}		{ return raw(yyscanner); }
 {bpf_object}		{ if (!isbpf(yyscanner)) { USER_REJECT }; return str(yyscanner, PE_BPF_OBJECT); }
 {bpf_source}		{ if (!isbpf(yyscanner)) { USER_REJECT }; return str(yyscanner, PE_BPF_SOURCE); }
 {name}			{ return pmu_str_check(yyscanner); }
+{name_tag}		{ return str(yyscanner, PE_NAME); }
 "/"			{ BEGIN(config); return '/'; }
 -			{ return '-'; }
 ,			{ BEGIN(event); return ','; }

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

end of thread, other threads:[~2018-06-07  8:18 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-04  6:50 [PATCH v3]: perf record: enable arbitrary event names thru name= modifier Alexey Budankov
2018-06-04  7:56 ` Jiri Olsa
2018-06-04 13:58   ` Arnaldo Carvalho de Melo
2018-06-04 14:19     ` Alexey Budankov
2018-06-04 14:23       ` Arnaldo Carvalho de Melo
2018-06-04 14:51         ` Alexey Budankov
2018-06-04 14:58           ` Arnaldo Carvalho de Melo
2018-06-04 15:22             ` Alexey Budankov
2018-06-04 19:16               ` Arnaldo Carvalho de Melo
2018-06-05  6:00                 ` Alexey Budankov
2018-06-05 15:33                   ` Arnaldo Carvalho de Melo
2018-06-07  8:18 ` [tip:perf/urgent] perf record: Enable " tip-bot for Alexey Budankov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).