linux-kernel - Re: [PATCH 2/2] perf, tools: Don't force MetricExprs to lower case

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: <20171009141258.GE28623@kernel.org>
Date:   Mon, 9 Oct 2017 11:12:58 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Andi Kleen <andi@...stfloor.org>
Cc:     Jiri Olsa <jolsa@...hat.com>, Jiri Olsa <jolsa@...nel.org>,
        Wang Nan <wangnan0@...wei.com>, linux-kernel@...r.kernel.org,
        Andi Kleen <ak@...ux.intel.com>, He Kuang <hekuang@...wei.com>,
        Alexei Starovoitov <ast@...com>
Subject: Re: [PATCH 2/2] perf, tools: Don't force MetricExprs to lower case

Em Mon, Oct 09, 2017 at 07:07:29AM -0700, Andi Kleen escreveu:
> On Mon, Oct 09, 2017 at 03:41:51PM +0200, Jiri Olsa wrote:
> > On Wed, Oct 04, 2017 at 09:27:11AM -0700, Andi Kleen wrote:
> > > On Wed, Oct 04, 2017 at 12:30:52PM +0200, Jiri Olsa wrote:
> > > > On Tue, Oct 03, 2017 at 01:06:05PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > Em Tue, Sep 12, 2017 at 12:56:43PM -0700, Andi Kleen escreveu:
> > > > > > From: Andi Kleen <ak@...ux.intel.com>
> > > > > > 
> > > > > > There are still problems with BPF misinterpreting some events
> > > > > > that include .c. An earlier fix made it work for stand alone
> > > > > > aliases, but it still fails for more complex constructs.
> > > > > 
> > > > > Hi Wang, Jiri,
> > > > > 
> > > > > 	Can you please take a look at this and see if there is something
> > > > > we can do to help Andi?
> > > > > 
> > > > > - Arnaldo
> > > > >  
> > > > > > REJECT keeps trying and trying a shorter string until
> > > > > > .c is matched and it appears like a valid BPF path.
> > > > > > 
> > > > > > % perf stat -e cpu/uops_executed.core,cmask=1/ true
> > > > > > bpf: builtin compilation failed: -95, try external compiler
> > > > > > ERROR: problems with path cpu/uops_executed.c: No such file or directory
> > > > > > event syntax error: 'cpu/uops_executed.core,cmask=1/'
> > > > > >                      \___ Failed to load cpu/uops_executed.c from source: Error when compiling BPF scriptlet
> > > > > > 
> > > > > > I tried to fix it, but it exceeds my flex knowledge, because
> > > > > > REJECT does not interact well with BEGIN states.
> > > > > > 
> > > > > > The BPF syntax in its current form really causes an ambigious
> > > > > > grammar.
> > > > 
> > > > right, it looks like we allow whole path (including / char)
> > > > for BPF file, which messes up with out pmu/.../ syntax
> > > > 
> > > > do we need that? (Cc-ed some bpf folks)
> > > > 
> > > > if not attached patch seems to fix things.. otherwise
> > > > we need to come up with another fix
> > > 
> > > I tried similar patches, but I always ran into more complex
> > > situations where it still matched incorrectly.
> > > 
> > > e.g. try it with cpu/uops_executed.core,... vs uops_executed.core
> > 
> > hm, both works for me with the change:
> > 
> >   perf stat -e cpu/uops_executed.core/ ls
> >   perf stat -e uops_executed.core ls
> 
> Ok. If it works it's fine for me.
> 
> > 
> > > The only real fix would be probably to add some unique 
> > > prefix for BPF, but that would break all existing users.
> > 
> > 
> > yea, there was no response from bpf folks, but it's  probably not an optio
> > 
> > how about checking if the file exist like below..
> 
> I presume that would interact badly with good error messages for typos
> for file names.

Right :-\

Perhaps we can be clever and use something like:

[root@...et bpf]# git pussh
git: 'pussh' is not a git command. See 'git --help'.

The most similar command is
	push
[root@...et bpf]# 

I.e. try to detect typos instead of plain using stat(), or perhaps
reverting to consider it a file when no match for other event takes
place?

- Arnaldo