linux-kernel - Re: [RFC V3 1/3] perf, tools: Support wildcards on pmu name in dynamic pmu events

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: <0ba091eaa6676498b4554978606d8016@codeaurora.org>
Date:   Wed, 07 Mar 2018 15:09:08 -0500
From:   Agustin Vega-Frias <agustinv@...eaurora.org>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:     Andi Kleen <ak@...ux.intel.com>, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>, timur@...eaurora.org
Subject: Re: [RFC V3 1/3] perf, tools: Support wildcards on pmu name in
 dynamic pmu events

On 2018-03-07 14:58, Arnaldo Carvalho de Melo wrote:
> Em Wed, Mar 07, 2018 at 02:49:50PM -0500, Agustin Vega-Frias escreveu:
>> On 2018-03-07 14:39, Arnaldo Carvalho de Melo wrote:
>> > Em Wed, Mar 07, 2018 at 04:05:43PM -0300, Arnaldo Carvalho de Melo
>> > escreveu:
>> > > Em Wed, Mar 07, 2018 at 04:05:09PM -0300, Arnaldo Carvalho de Melo
>> > > escreveu:
>> > > > Em Wed, Mar 07, 2018 at 10:54:15AM -0800, Andi Kleen escreveu:
>> > > > > > Sorry about that. That's probably because FNM_EXTMATCH is a GNU extension,
>> > > > > > not POSIX, and the Alpine and Android runtimes likely don't implement
>> > > > > > that...
>> > > > > > I'll send a fix reverting back to the strncmp to ignore the uncore_ prefix,
>> > > > > > and dropping that extension.
>> > > > >
>> > > > > Just don't set it? Even the basic glob patterns are useful.
>> > > >
>> > > > Or use:
>> > > >
>> > > > #ifndef FNM_EXTMATCH
>> > > > #define FNM_EXTMATCH 0
>> > > > #endif
>> > > >
>> > > > So on systems without it, its not used, while on GNU systems, we have
>> > > > that functionality (pretty fancy, someone may need that... ;-))
>> > >
>> > > I'll try this route, btw, no need to send more patches for now.
>> >
>> > So, with the patch at the end of this message, it works now in those
>> > systems:
>> >
>> > [root@...et ~]# dm
>> >    1 alpine:3.4                    : Ok   gcc (Alpine 5.3.0) 5.3.0
>> >    2 alpine:3.5                    : Ok   gcc (Alpine 6.2.1) 6.2.1
>> > 20160822
>> >    3 alpine:3.6                    : Ok   gcc (Alpine 6.3.0) 6.3.0
>> >    4 alpine:edge                   : Ok   gcc (Alpine 6.4.0) 6.4.0
>> >    5 amazonlinux:1                 : Ok   gcc (GCC) 4.8.5 20150623
>> > (Red Hat 4.8.5-11)
>> >    6 amazonlinux:2                 : Ok   gcc (GCC) 7.2.1 20170915
>> > (Red Hat 7.2.1-2)
>> >    7 android-ndk:r12b-arm          : Ok   arm-linux-androideabi-gcc
>> > (GCC) 4.9.x 20150123 (prerelease)
>> >    8 android-ndk:r15c-arm          : Ok   arm-linux-androideabi-gcc
>> > (GCC) 4.9.x 20150123 (prerelease)
>> >    9 centos:5                      : Ok   gcc (GCC) 4.1.2 20080704
>> > (Red Hat 4.1.2-55)
>> >   10 centos:6                      : Ok   gcc (GCC) 4.4.7 20120313
>> > (Red Hat 4.4.7-18)
>> >
>> > I stopped the test at this point to process some more patches, will
>> > restart the tests with those extra patches and if all goes well with the
>> > other 46 build environments, push upstream, thanks.
>> >
>> > - Arnaldo
>> >
>> > diff --git a/tools/perf/util/parse-events.y
>> > b/tools/perf/util/parse-events.y
>> > index 316ac073aa78..18473be7d787 100644
>> > --- a/tools/perf/util/parse-events.y
>> > +++ b/tools/perf/util/parse-events.y
>> > @@ -9,6 +9,13 @@
>> >  #define YYDEBUG 1
>> >
>> >  #include <fnmatch.h>
>> > +/*
>> > + * GNU extension, so better define it to 0 for systems such
>> > + * as Android and Alpine Linux.
>> > + */
>> > +#ifndef FNM_EXTMATCH
>> > +#define FNM_EXTMATCH 0
>> > +#endif
>> >  #include <linux/compiler.h>
>> >  #include <linux/list.h>
>> >  #include <linux/types.h>
>> 
>> Hey Jiri,
>> 
>> The downside is that, while the compilation now works on those
>> systems, the pattern will not work as intended in them :o(
>> Let me cook something and send it ASAP.
> 
> Well, do you think this is really a big problem? Even if we add nice
> docs?
> 
> I haven't tested something that works with this syntax on a capable
> system and then on one that doesn't, to see how it would behave,
> probably it would say something about a syntax error?
> 

Most likely, yes, it would be flagged as a syntax error in the event 
name.
I'd prefer we fix this. I have the new patch ready, do you want me to 
just
send you that patch instead of the series?

Thanks,
Agustín

-- 
Qualcomm Datacenter Technologies, Inc. on behalf of the Qualcomm 
Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a 
Linux Foundation Collaborative Project.