[PATCH] perf tools: Add missing Intel CPU events to parser

[PATCH] perf tools: Add missing Intel CPU events to parser

[Adrian Hunter]

perf list expects CPU events to be parseable by name, e.g.

    # perf list | grep el-capacity-read
      el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]

But the event parser does not recognize them that way, e.g.

    # perf test -v "Parse event"
    <SNIP>
    running test 54 'cycles//u'
    running test 55 'cycles:k'
    running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
    running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
    running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
    running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
    -> cpu/event=0,umask=0x11/
    -> cpu/event=0,umask=0x13/
    -> cpu/event=0x54,umask=0x1/
    failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
    event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
                           \___ parser error test child finished with 1
    ---- end ----
    Parse event definition strings: FAILED!

Fix by adding missing Intel CPU events to the event parser.
Missing events were found by using:

    grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/parse-events.l | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 7b1c8ee537cf..a4012613cdcf 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -342,11 +342,25 @@ bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUT
 	 * Because the prefix cycles is mixed up with cpu-cycles.
 	 * loads and stores are mixed up with cache event
 	 */
-cycles-ct					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-cycles-t					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-loads					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-stores					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-topdown-[a-z-]+					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+cycles-ct				|
+cycles-t				|
+mem-loads				|
+mem-stores				|
+topdown-[a-z-]+				|
+tx-start				|
+tx-commit				|
+tx-abort				|
+tx-capacity				|
+tx-conflict				|
+el-start				|
+el-commit				|
+el-abort				|
+el-capacity				|
+el-conflict				|
+tx-capacity-read			|
+tx-capacity-write			|
+el-capacity-read			|
+el-capacity-write			{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
 
 L1-dcache|l1-d|l1d|L1-data		|
 L1-icache|l1-i|l1i|L1-instruction	|
-- 
2.17.1

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Jiri Olsa]

On Tue, Mar 24, 2020 at 05:04:43PM +0200, Adrian Hunter wrote:
> perf list expects CPU events to be parseable by name, e.g.
> 
>     # perf list | grep el-capacity-read
>       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]
> 
> But the event parser does not recognize them that way, e.g.
> 
>     # perf test -v "Parse event"
>     <SNIP>
>     running test 54 'cycles//u'
>     running test 55 'cycles:k'
>     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
>     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
>     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
>     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
>     -> cpu/event=0,umask=0x11/
>     -> cpu/event=0,umask=0x13/
>     -> cpu/event=0x54,umask=0x1/
>     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
>     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
>                            \___ parser error test child finished with 1
>     ---- end ----
>     Parse event definition strings: FAILED!
> 
> Fix by adding missing Intel CPU events to the event parser.
> Missing events were found by using:
> 
>     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Arnaldo Carvalho de Melo]

Em Wed, Mar 25, 2020 at 11:33:45AM +0100, Jiri Olsa escreveu:
> On Tue, Mar 24, 2020 at 05:04:43PM +0200, Adrian Hunter wrote:
> > perf list expects CPU events to be parseable by name, e.g.

> >     # perf list | grep el-capacity-read
> >       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]

> > But the event parser does not recognize them that way, e.g.

> >     # perf test -v "Parse event"
> >     <SNIP>
> >     running test 54 'cycles//u'
> >     running test 55 'cycles:k'
> >     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
> >     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
> >     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
> >     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
> >     -> cpu/event=0,umask=0x11/
> >     -> cpu/event=0,umask=0x13/
> >     -> cpu/event=0x54,umask=0x1/
> >     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
> >     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
> >                            \___ parser error test child finished with 1
> >     ---- end ----
> >     Parse event definition strings: FAILED!

> > Fix by adding missing Intel CPU events to the event parser.
> > Missing events were found by using:

> >     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
> > 
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> Acked-by: Jiri Olsa <jolsa@redhat.com>

So, I'm not being able to reproduce this, what an I missing?

[root@seventh ~]# grep -m1 'model name' /proc/cpuinfo
model name	: Intel(R) Core(TM) i5-7500 CPU @ 3.40GHz
[root@seventh ~]#

[root@seventh ~]# perf list | grep el-capacity
  el-capacity OR cpu/el-capacity/                    [Kernel PMU event]
[root@seventh ~]# perf test -v "Parse event" |& grep el-capacity
el-capacity -> cpu/event=0x54,umask=0x2/
[root@seventh ~]# perf stat -e el-capacity:u,cpu/event=el-capacity/u
^C
 Performance counter stats for 'system wide':

                 0      el-capacity:u
                 0      cpu/event=el-capacity/u

       2.315736828 seconds time elapsed


[root@seventh ~]#

[root@seventh ~]# perf test -v "Parse event" |& grep el-capacity -B5
running test 55 'cycles:k'
running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
el-capacity -> cpu/event=0x54,umask=0x2/
[root@seventh ~]# perf test -v "Parse event" |& grep el-capacity -B5 -A5
running test 55 'cycles:k'
running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
el-capacity -> cpu/event=0x54,umask=0x2/
el-conflict -> cpu/event=0x54,umask=0x1/
el-start -> cpu/event=0xc8,umask=0x1/
tx-abort -> cpu/event=0xc9,umask=0x4/
topdown-slots-issued -> cpu/event=0xe,umask=0x1/
tx-capacity -> cpu/event=0x54,umask=0x2/
[root@seventh ~]#

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Jiri Olsa]

On Wed, Mar 25, 2020 at 10:15:49AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 25, 2020 at 11:33:45AM +0100, Jiri Olsa escreveu:
> > On Tue, Mar 24, 2020 at 05:04:43PM +0200, Adrian Hunter wrote:
> > > perf list expects CPU events to be parseable by name, e.g.
> 
> > >     # perf list | grep el-capacity-read
> > >       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]
> 
> > > But the event parser does not recognize them that way, e.g.
> 
> > >     # perf test -v "Parse event"
> > >     <SNIP>
> > >     running test 54 'cycles//u'
> > >     running test 55 'cycles:k'
> > >     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
> > >     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
> > >     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
> > >     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
> > >     -> cpu/event=0,umask=0x11/
> > >     -> cpu/event=0,umask=0x13/
> > >     -> cpu/event=0x54,umask=0x1/
> > >     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
> > >     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
> > >                            \___ parser error test child finished with 1
> > >     ---- end ----
> > >     Parse event definition strings: FAILED!
> 
> > > Fix by adding missing Intel CPU events to the event parser.
> > > Missing events were found by using:
> 
> > >     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
> > > 
> > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > 
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
> 
> So, I'm not being able to reproduce this, what an I missing?

I think you need to be on some really recent intel
which defines events which we did not covered yet
like el-capacity-write in icelake

jirka

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Arnaldo Carvalho de Melo]

Em Wed, Mar 25, 2020 at 02:53:50PM +0100, Jiri Olsa escreveu:
> On Wed, Mar 25, 2020 at 10:15:49AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Mar 25, 2020 at 11:33:45AM +0100, Jiri Olsa escreveu:
> > > On Tue, Mar 24, 2020 at 05:04:43PM +0200, Adrian Hunter wrote:
> > > > perf list expects CPU events to be parseable by name, e.g.
> > 
> > > >     # perf list | grep el-capacity-read
> > > >       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]
> > 
> > > > But the event parser does not recognize them that way, e.g.
> > 
> > > >     # perf test -v "Parse event"
> > > >     <SNIP>
> > > >     running test 54 'cycles//u'
> > > >     running test 55 'cycles:k'
> > > >     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
> > > >     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
> > > >     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
> > > >     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
> > > >     -> cpu/event=0,umask=0x11/
> > > >     -> cpu/event=0,umask=0x13/
> > > >     -> cpu/event=0x54,umask=0x1/
> > > >     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
> > > >     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
> > > >                            \___ parser error test child finished with 1
> > > >     ---- end ----
> > > >     Parse event definition strings: FAILED!
> > 
> > > > Fix by adding missing Intel CPU events to the event parser.
> > > > Missing events were found by using:
> > 
> > > >     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
> > > > 
> > > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > > 
> > > Acked-by: Jiri Olsa <jolsa@redhat.com>
> > 
> > So, I'm not being able to reproduce this, what an I missing?
> 
> I think you need to be on some really recent intel
> which defines events which we did not covered yet
> like el-capacity-write in icelake

That is why I tried with el-capacity, which is moved to the parser as
well, I've replaced el-capacity-read, which I don't have in this Kaby
Lake machine, with el-capacity, that is present:

[root@seventh ~]# perf list | grep el-capacity
  el-capacity OR cpu/el-capacity/                    [Kernel PMU event]
[root@seventh ~]#

- Arnaldo

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Adrian Hunter]

On 25/03/20 4:22 pm, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 25, 2020 at 02:53:50PM +0100, Jiri Olsa escreveu:
>> On Wed, Mar 25, 2020 at 10:15:49AM -0300, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Mar 25, 2020 at 11:33:45AM +0100, Jiri Olsa escreveu:
>>>> On Tue, Mar 24, 2020 at 05:04:43PM +0200, Adrian Hunter wrote:
>>>>> perf list expects CPU events to be parseable by name, e.g.
>>>
>>>>>     # perf list | grep el-capacity-read
>>>>>       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]
>>>
>>>>> But the event parser does not recognize them that way, e.g.
>>>
>>>>>     # perf test -v "Parse event"
>>>>>     <SNIP>
>>>>>     running test 54 'cycles//u'
>>>>>     running test 55 'cycles:k'
>>>>>     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
>>>>>     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
>>>>>     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
>>>>>     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
>>>>>     -> cpu/event=0,umask=0x11/
>>>>>     -> cpu/event=0,umask=0x13/
>>>>>     -> cpu/event=0x54,umask=0x1/
>>>>>     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
>>>>>     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
>>>>>                            \___ parser error test child finished with 1
>>>>>     ---- end ----
>>>>>     Parse event definition strings: FAILED!
>>>
>>>>> Fix by adding missing Intel CPU events to the event parser.
>>>>> Missing events were found by using:
>>>
>>>>>     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
>>>>>
>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>>
>>>> Acked-by: Jiri Olsa <jolsa@redhat.com>
>>>
>>> So, I'm not being able to reproduce this, what an I missing?
>>
>> I think you need to be on some really recent intel
>> which defines events which we did not covered yet
>> like el-capacity-write in icelake
> 
> That is why I tried with el-capacity, which is moved to the parser as
> well, I've replaced el-capacity-read, which I don't have in this Kaby
> Lake machine, with el-capacity, that is present:
> 
> [root@seventh ~]# perf list | grep el-capacity
>   el-capacity OR cpu/el-capacity/                    [Kernel PMU event]
> [root@seventh ~]#

I just checked that and it seems to be a "feature" of the parser that it
gets confused between el-capacity and el-capacity-read.

Making them explicit in parse-events.l makes the problem go away, but I
wonder now if the parser could be better in this regard.

Any ideas?

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Jiri Olsa]

On Wed, Mar 25, 2020 at 04:24:58PM +0200, Adrian Hunter wrote:
> On 25/03/20 4:22 pm, Arnaldo Carvalho de Melo wrote:
> > Em Wed, Mar 25, 2020 at 02:53:50PM +0100, Jiri Olsa escreveu:
> >> On Wed, Mar 25, 2020 at 10:15:49AM -0300, Arnaldo Carvalho de Melo wrote:
> >>> Em Wed, Mar 25, 2020 at 11:33:45AM +0100, Jiri Olsa escreveu:
> >>>> On Tue, Mar 24, 2020 at 05:04:43PM +0200, Adrian Hunter wrote:
> >>>>> perf list expects CPU events to be parseable by name, e.g.
> >>>
> >>>>>     # perf list | grep el-capacity-read
> >>>>>       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]
> >>>
> >>>>> But the event parser does not recognize them that way, e.g.
> >>>
> >>>>>     # perf test -v "Parse event"
> >>>>>     <SNIP>
> >>>>>     running test 54 'cycles//u'
> >>>>>     running test 55 'cycles:k'
> >>>>>     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
> >>>>>     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
> >>>>>     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
> >>>>>     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
> >>>>>     -> cpu/event=0,umask=0x11/
> >>>>>     -> cpu/event=0,umask=0x13/
> >>>>>     -> cpu/event=0x54,umask=0x1/
> >>>>>     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
> >>>>>     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
> >>>>>                            \___ parser error test child finished with 1
> >>>>>     ---- end ----
> >>>>>     Parse event definition strings: FAILED!
> >>>
> >>>>> Fix by adding missing Intel CPU events to the event parser.
> >>>>> Missing events were found by using:
> >>>
> >>>>>     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
> >>>>>
> >>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >>>>
> >>>> Acked-by: Jiri Olsa <jolsa@redhat.com>
> >>>
> >>> So, I'm not being able to reproduce this, what an I missing?
> >>
> >> I think you need to be on some really recent intel
> >> which defines events which we did not covered yet
> >> like el-capacity-write in icelake
> > 
> > That is why I tried with el-capacity, which is moved to the parser as
> > well, I've replaced el-capacity-read, which I don't have in this Kaby
> > Lake machine, with el-capacity, that is present:
> > 
> > [root@seventh ~]# perf list | grep el-capacity
> >   el-capacity OR cpu/el-capacity/                    [Kernel PMU event]
> > [root@seventh ~]#
> 
> I just checked that and it seems to be a "feature" of the parser that it
> gets confused between el-capacity and el-capacity-read.
> 
> Making them explicit in parse-events.l makes the problem go away, but I
> wonder now if the parser could be better in this regard.

so we have that PRE/SUFFIX logic that allows us
to specify any sysfs event term as standalone event

the lexer in this case below was used to handle special cases..
and IIUC think having more than one '-' is one of them

could you check if the patch below will fix that for you?

jirka


---
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 7b1c8ee537cf..347eb3e6794a 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -342,11 +342,15 @@ bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUT
 	 * Because the prefix cycles is mixed up with cpu-cycles.
 	 * loads and stores are mixed up with cache event
 	 */
-cycles-ct					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-cycles-t					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-loads					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-stores					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-topdown-[a-z-]+					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+cycles-ct				|
+cycles-t				|
+mem-loads				|
+mem-stores				|
+topdown-[a-z-]+				|
+tx-capacity-read			|
+tx-capacity-write			|
+el-capacity-read			|
+el-capacity-write			{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
 
 L1-dcache|l1-d|l1d|L1-data		|
 L1-icache|l1-i|l1i|L1-instruction	|

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Adrian Hunter]

On 25/03/20 5:22 pm, Jiri Olsa wrote:
> On Wed, Mar 25, 2020 at 04:24:58PM +0200, Adrian Hunter wrote:
>> On 25/03/20 4:22 pm, Arnaldo Carvalho de Melo wrote:
>>> Em Wed, Mar 25, 2020 at 02:53:50PM +0100, Jiri Olsa escreveu:
>>>> On Wed, Mar 25, 2020 at 10:15:49AM -0300, Arnaldo Carvalho de Melo wrote:
>>>>> Em Wed, Mar 25, 2020 at 11:33:45AM +0100, Jiri Olsa escreveu:
>>>>>> On Tue, Mar 24, 2020 at 05:04:43PM +0200, Adrian Hunter wrote:
>>>>>>> perf list expects CPU events to be parseable by name, e.g.
>>>>>
>>>>>>>     # perf list | grep el-capacity-read
>>>>>>>       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]
>>>>>
>>>>>>> But the event parser does not recognize them that way, e.g.
>>>>>
>>>>>>>     # perf test -v "Parse event"
>>>>>>>     <SNIP>
>>>>>>>     running test 54 'cycles//u'
>>>>>>>     running test 55 'cycles:k'
>>>>>>>     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
>>>>>>>     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
>>>>>>>     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
>>>>>>>     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
>>>>>>>     -> cpu/event=0,umask=0x11/
>>>>>>>     -> cpu/event=0,umask=0x13/
>>>>>>>     -> cpu/event=0x54,umask=0x1/
>>>>>>>     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
>>>>>>>     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
>>>>>>>                            \___ parser error test child finished with 1
>>>>>>>     ---- end ----
>>>>>>>     Parse event definition strings: FAILED!
>>>>>
>>>>>>> Fix by adding missing Intel CPU events to the event parser.
>>>>>>> Missing events were found by using:
>>>>>
>>>>>>>     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
>>>>>>>
>>>>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>>>>
>>>>>> Acked-by: Jiri Olsa <jolsa@redhat.com>
>>>>>
>>>>> So, I'm not being able to reproduce this, what an I missing?
>>>>
>>>> I think you need to be on some really recent intel
>>>> which defines events which we did not covered yet
>>>> like el-capacity-write in icelake
>>>
>>> That is why I tried with el-capacity, which is moved to the parser as
>>> well, I've replaced el-capacity-read, which I don't have in this Kaby
>>> Lake machine, with el-capacity, that is present:
>>>
>>> [root@seventh ~]# perf list | grep el-capacity
>>>   el-capacity OR cpu/el-capacity/                    [Kernel PMU event]
>>> [root@seventh ~]#
>>
>> I just checked that and it seems to be a "feature" of the parser that it
>> gets confused between el-capacity and el-capacity-read.
>>
>> Making them explicit in parse-events.l makes the problem go away, but I
>> wonder now if the parser could be better in this regard.
> 
> so we have that PRE/SUFFIX logic that allows us
> to specify any sysfs event term as standalone event
> 
> the lexer in this case below was used to handle special cases..
> and IIUC think having more than one '-' is one of them
> 
> could you check if the patch below will fix that for you?
> 
> jirka
> 
> 
> ---
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 7b1c8ee537cf..347eb3e6794a 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -342,11 +342,15 @@ bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUT
>  	 * Because the prefix cycles is mixed up with cpu-cycles.
>  	 * loads and stores are mixed up with cache event
>  	 */
> -cycles-ct					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> -cycles-t					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> -mem-loads					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> -mem-stores					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> -topdown-[a-z-]+					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> +cycles-ct				|
> +cycles-t				|
> +mem-loads				|
> +mem-stores				|
> +topdown-[a-z-]+				|
> +tx-capacity-read			|
> +tx-capacity-write			|
> +el-capacity-read			|
> +el-capacity-write			{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }

Yes that works, as does

tx-capacity-[a-z-]+
el-capacity-[a-z-]+

Do we have an explanation for why we cannot make it accept 3-part names
without handling them as special cases?

I just tried but it didn't work.

Re: [PATCH] perf tools: Add missing Intel CPU events to parser

[Jiri Olsa]

On Wed, Mar 25, 2020 at 07:10:44PM +0200, Adrian Hunter wrote:

SNIP

> > ---
> > diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> > index 7b1c8ee537cf..347eb3e6794a 100644
> > --- a/tools/perf/util/parse-events.l
> > +++ b/tools/perf/util/parse-events.l
> > @@ -342,11 +342,15 @@ bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUT
> >  	 * Because the prefix cycles is mixed up with cpu-cycles.
> >  	 * loads and stores are mixed up with cache event
> >  	 */
> > -cycles-ct					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> > -cycles-t					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> > -mem-loads					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> > -mem-stores					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> > -topdown-[a-z-]+					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> > +cycles-ct				|
> > +cycles-t				|
> > +mem-loads				|
> > +mem-stores				|
> > +topdown-[a-z-]+				|
> > +tx-capacity-read			|
> > +tx-capacity-write			|
> > +el-capacity-read			|
> > +el-capacity-write			{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
> 
> Yes that works, as does
> 
> tx-capacity-[a-z-]+
> el-capacity-[a-z-]+
> 
> Do we have an explanation for why we cannot make it accept 3-part names
> without handling them as special cases?

check the pmu_str_check, I guess it could be more generic,
but it gets complicated later on, so I'm not sure there's
some other obstacle..

CC-ing Kan Liang, who wrote that

jirka

> 
> I just tried but it didn't work.
> 

[PATCH V2] perf tools: Add missing Intel CPU events to parser

[Adrian Hunter]

perf list expects CPU events to be parseable by name, e.g.

    # perf list | grep el-capacity-read
      el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]

But the event parser does not recognize them that way, e.g.

    # perf test -v "Parse event"
    <SNIP>
    running test 54 'cycles//u'
    running test 55 'cycles:k'
    running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
    running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
    running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
    running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
    -> cpu/event=0,umask=0x11/
    -> cpu/event=0,umask=0x13/
    -> cpu/event=0x54,umask=0x1/
    failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
    event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
                           \___ parser error test child finished with 1
    ---- end ----
    Parse event definition strings: FAILED!

This happens because the parser splits names by '-' in order to deal
with cache events. For example 'L1-dcache' is a token in
parse-events.l which is matched to 'L1-dcache-load-miss' by the
following rule:

    PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT opt_event_config

And so there is special handling for 2-part PMU names i.e.

    PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc

but no handling for 3-part names, which are instead added as tokens e.g.

    topdown-[a-z-]+

While it would be possible to add a rule for 3-part names, that would
not work if the first parts were also a valid PMU name e.g.
'el-capacity-read' would be matched to 'el-capacity' before the parser
reached the 3rd part.

The parser would need significant change to rationalize all this, so
instead fix for now by adding missing Intel CPU events with 3-part names
to the event parser as tokens.

Missing events were found by using:

    grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---


Changes in V2:

	Add only 3-part names
	Clarify commit message


 tools/perf/util/parse-events.l | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 7b1c8ee537cf..baa48f28d57d 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -342,11 +342,13 @@ bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUT
 	 * Because the prefix cycles is mixed up with cpu-cycles.
 	 * loads and stores are mixed up with cache event
 	 */
-cycles-ct					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-cycles-t					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-loads					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-stores					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-topdown-[a-z-]+					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+cycles-ct				|
+cycles-t				|
+mem-loads				|
+mem-stores				|
+topdown-[a-z-]+				|
+tx-capacity-[a-z-]+			|
+el-capacity-[a-z-]+			{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
 
 L1-dcache|l1-d|l1d|L1-data		|
 L1-icache|l1-i|l1i|L1-instruction	|
-- 
2.17.1

Re: [PATCH V2] perf tools: Add missing Intel CPU events to parser

[Jiri Olsa]

On Thu, Mar 26, 2020 at 10:01:47AM +0200, Adrian Hunter wrote:
> perf list expects CPU events to be parseable by name, e.g.
> 
>     # perf list | grep el-capacity-read
>       el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]
> 
> But the event parser does not recognize them that way, e.g.
> 
>     # perf test -v "Parse event"
>     <SNIP>
>     running test 54 'cycles//u'
>     running test 55 'cycles:k'
>     running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
>     running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
>     running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
>     running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
>     -> cpu/event=0,umask=0x11/
>     -> cpu/event=0,umask=0x13/
>     -> cpu/event=0x54,umask=0x1/
>     failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
>     event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
>                            \___ parser error test child finished with 1
>     ---- end ----
>     Parse event definition strings: FAILED!
> 
> This happens because the parser splits names by '-' in order to deal
> with cache events. For example 'L1-dcache' is a token in
> parse-events.l which is matched to 'L1-dcache-load-miss' by the
> following rule:
> 
>     PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT opt_event_config
> 
> And so there is special handling for 2-part PMU names i.e.
> 
>     PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc
> 
> but no handling for 3-part names, which are instead added as tokens e.g.
> 
>     topdown-[a-z-]+
> 
> While it would be possible to add a rule for 3-part names, that would
> not work if the first parts were also a valid PMU name e.g.
> 'el-capacity-read' would be matched to 'el-capacity' before the parser
> reached the 3rd part.
> 
> The parser would need significant change to rationalize all this, so
> instead fix for now by adding missing Intel CPU events with 3-part names
> to the event parser as tokens.
> 
> Missing events were found by using:
> 
>     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

Acked-by: Jiri Olsa <jolsa@redhat.com>

thanks,
jirka

Re: [PATCH V2] perf tools: Add missing Intel CPU events to parser

[Arnaldo Carvalho de Melo]

Em Thu, Mar 26, 2020 at 10:25:45AM +0100, Jiri Olsa escreveu:
> On Thu, Mar 26, 2020 at 10:01:47AM +0200, Adrian Hunter wrote:
> > The parser would need significant change to rationalize all this, so
> > instead fix for now by adding missing Intel CPU events with 3-part names
> > to the event parser as tokens.

> > Missing events were found by using:

> >     grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c

> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

> Acked-by: Jiri Olsa <jolsa@redhat.com>

Thanks, applied.

- Arnaldo

[tip: perf/urgent] perf events parser: Add missing Intel CPU events to parser

[tip-bot2 for Adrian Hunter]

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     47327f56674d69a423f7167f8d4ea537cc1762cc
Gitweb:        https://git.kernel.org/tip/47327f56674d69a423f7167f8d4ea537cc1762cc
Author:        Adrian Hunter <adrian.hunter@intel.com>
AuthorDate:    Thu, 26 Mar 2020 10:01:47 +02:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Fri, 03 Apr 2020 09:37:56 -03:00

perf events parser: Add missing Intel CPU events to parser

perf list expects CPU events to be parseable by name, e.g.

    # perf list | grep el-capacity-read
      el-capacity-read OR cpu/el-capacity-read/          [Kernel PMU event]

But the event parser does not recognize them that way, e.g.

    # perf test -v "Parse event"
    <SNIP>
    running test 54 'cycles//u'
    running test 55 'cycles:k'
    running test 0 'cpu/config=10,config1,config2=3,period=1000/u'
    running test 1 'cpu/config=1,name=krava/u,cpu/config=2/u'
    running test 2 'cpu/config=1,call-graph=fp,time,period=100000/,cpu/config=2,call-graph=no,time=0,period=2000/'
    running test 3 'cpu/name='COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks',period=0x1,event=0x2/ukp'
    -> cpu/event=0,umask=0x11/
    -> cpu/event=0,umask=0x13/
    -> cpu/event=0x54,umask=0x1/
    failed to parse event 'el-capacity-read:u,cpu/event=el-capacity-read/u', err 1, str 'parser error'
    event syntax error: 'el-capacity-read:u,cpu/event=el-capacity-read/u'
                           \___ parser error test child finished with 1
    ---- end ----
    Parse event definition strings: FAILED!

This happens because the parser splits names by '-' in order to deal
with cache events. For example 'L1-dcache' is a token in
parse-events.l which is matched to 'L1-dcache-load-miss' by the
following rule:

    PE_NAME_CACHE_TYPE '-' PE_NAME_CACHE_OP_RESULT '-' PE_NAME_CACHE_OP_RESULT opt_event_config

And so there is special handling for 2-part PMU names i.e.

    PE_PMU_EVENT_PRE '-' PE_PMU_EVENT_SUF sep_dc

but no handling for 3-part names, which are instead added as tokens e.g.

    topdown-[a-z-]+

While it would be possible to add a rule for 3-part names, that would
not work if the first parts were also a valid PMU name e.g.
'el-capacity-read' would be matched to 'el-capacity' before the parser
reached the 3rd part.

The parser would need significant change to rationalize all this, so
instead fix for now by adding missing Intel CPU events with 3-part names
to the event parser as tokens.

Missing events were found by using:

    grep -r EVENT_ATTR_STR arch/x86/events/intel/core.c

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Link: http://lore.kernel.org/lkml/90c7ae07-c568-b6d3-f9c4-d0c1528a0610@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.l | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index 7b1c8ee..baa48f2 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -342,11 +342,13 @@ bpf-output					{ return sym(yyscanner, PERF_TYPE_SOFTWARE, PERF_COUNT_SW_BPF_OUT
 	 * Because the prefix cycles is mixed up with cpu-cycles.
 	 * loads and stores are mixed up with cache event
 	 */
-cycles-ct					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-cycles-t					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-loads					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-mem-stores					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
-topdown-[a-z-]+					{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
+cycles-ct				|
+cycles-t				|
+mem-loads				|
+mem-stores				|
+topdown-[a-z-]+				|
+tx-capacity-[a-z-]+			|
+el-capacity-[a-z-]+			{ return str(yyscanner, PE_KERNEL_PMU_EVENT); }
 
 L1-dcache|l1-d|l1d|L1-data		|
 L1-icache|l1-i|l1i|L1-instruction	|