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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200325152211.GA1908530@krava>
Date:   Wed, 25 Mar 2020 16:22:11 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Adrian Hunter <adrian.hunter@...el.com>
Cc:     Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf tools: Add missing Intel CPU events to parser

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@...el.com>
> >>>>
> >>>> Acked-by: Jiri Olsa <jolsa@...hat.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@...enth ~]# perf list | grep el-capacity
> >   el-capacity OR cpu/el-capacity/                    [Kernel PMU event]
> > [root@...enth ~]#
> 
> 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	|