From: Jiri Olsa <jolsa@redhat.com>
To: Agustin Vega-Frias <agustinv@codeaurora.org>
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
Arnaldo Carvalho de Melo <acme@kernel.org>,
Andi Kleen <ak@linux.intel.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Namhyung Kim <namhyung@kernel.org>,
timur@codeaurora.org
Subject: Re: [RFC V2 1/3] perf, tools: Support wildcards on pmu name in dynamic pmu events
Date: Sat, 3 Mar 2018 15:34:36 +0100 [thread overview]
Message-ID: <20180303143436.GA14563@krava> (raw)
In-Reply-To: <1520034092-35275-2-git-send-email-agustinv@codeaurora.org>
On Fri, Mar 02, 2018 at 06:41:30PM -0500, Agustin Vega-Frias wrote:
SNIP
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 655ecff..a1a01b1 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -175,7 +175,7 @@ bpf_source [^,{}]+\.c[a-zA-Z0-9._]*
> 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 [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() */
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index e81a20e..c528469 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -8,6 +8,7 @@
>
> #define YYDEBUG 1
>
> +#include <fnmatch.h>
> #include <linux/compiler.h>
> #include <linux/list.h>
> #include <linux/types.h>
> @@ -241,7 +242,7 @@ PE_NAME opt_event_config
> if (!strncmp(name, "uncore_", 7) &&
> strncmp($1, "uncore_", 7))
> name += 7;
> - if (!strncmp($1, name, strlen($1))) {
> + if (!strncmp($1, name, strlen($1)) || !fnmatch($1, name, 0)) {
could we now get rid of the strncmp in here and keep the
glob matching only? I find it confusing now that following
commands give me same results:
- [root@krava perf]# ./perf stat -e 'cbox/clockticks/' --no-merge -a sleep 1
Performance counter stats for 'system wide':
<not supported> uncore_cbox_1/clockticks/
281,474,957,674,239 uncore_cbox_0/clockticks/
1.000958335 seconds time elapsed
- [root@krava perf]# ./perf stat -e '*cbox*/clockticks/' --no-merge -a sleep 1
Performance counter stats for 'system wide':
<not supported> uncore_cbox_1/clockticks/
5,427,337 uncore_cbox_0/clockticks/
1.000962724 seconds time elapsed
- [root@krava perf]# ./perf stat -e 'cbox*/clockticks/' --no-merge -a sleep 1
Performance counter stats for 'system wide':
<not supported> uncore_cbox_1/clockticks/
281,474,969,621,374 uncore_cbox_0/clockticks/
1.001026179 seconds time elapsed
and this one fails:
- [root@krava perf]# ./perf stat -e '*cbox/clockticks/' --no-merge -a sleep 1
event syntax error: '*cbox/clockticks/'
\___ Cannot find PMU `*cbox'. Missing kernel support?
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
despite the fact that it makes as much sense as the previous one: perf stat -e 'cbox*/clockticks/'
I'd think let's keep just the glob matching, so it's clear
you what you use wildcards for.. thoughts?
thanks,
jirka
WARNING: multiple messages have this Message-ID (diff)
From: jolsa@redhat.com (Jiri Olsa)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC V2 1/3] perf, tools: Support wildcards on pmu name in dynamic pmu events
Date: Sat, 3 Mar 2018 15:34:36 +0100 [thread overview]
Message-ID: <20180303143436.GA14563@krava> (raw)
In-Reply-To: <1520034092-35275-2-git-send-email-agustinv@codeaurora.org>
On Fri, Mar 02, 2018 at 06:41:30PM -0500, Agustin Vega-Frias wrote:
SNIP
>
> diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
> index 655ecff..a1a01b1 100644
> --- a/tools/perf/util/parse-events.l
> +++ b/tools/perf/util/parse-events.l
> @@ -175,7 +175,7 @@ bpf_source [^,{}]+\.c[a-zA-Z0-9._]*
> 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 [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() */
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index e81a20e..c528469 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -8,6 +8,7 @@
>
> #define YYDEBUG 1
>
> +#include <fnmatch.h>
> #include <linux/compiler.h>
> #include <linux/list.h>
> #include <linux/types.h>
> @@ -241,7 +242,7 @@ PE_NAME opt_event_config
> if (!strncmp(name, "uncore_", 7) &&
> strncmp($1, "uncore_", 7))
> name += 7;
> - if (!strncmp($1, name, strlen($1))) {
> + if (!strncmp($1, name, strlen($1)) || !fnmatch($1, name, 0)) {
could we now get rid of the strncmp in here and keep the
glob matching only? I find it confusing now that following
commands give me same results:
- [root at krava perf]# ./perf stat -e 'cbox/clockticks/' --no-merge -a sleep 1
Performance counter stats for 'system wide':
<not supported> uncore_cbox_1/clockticks/
281,474,957,674,239 uncore_cbox_0/clockticks/
1.000958335 seconds time elapsed
- [root at krava perf]# ./perf stat -e '*cbox*/clockticks/' --no-merge -a sleep 1
Performance counter stats for 'system wide':
<not supported> uncore_cbox_1/clockticks/
5,427,337 uncore_cbox_0/clockticks/
1.000962724 seconds time elapsed
- [root at krava perf]# ./perf stat -e 'cbox*/clockticks/' --no-merge -a sleep 1
Performance counter stats for 'system wide':
<not supported> uncore_cbox_1/clockticks/
281,474,969,621,374 uncore_cbox_0/clockticks/
1.001026179 seconds time elapsed
and this one fails:
- [root at krava perf]# ./perf stat -e '*cbox/clockticks/' --no-merge -a sleep 1
event syntax error: '*cbox/clockticks/'
\___ Cannot find PMU `*cbox'. Missing kernel support?
Run 'perf list' for a list of valid events
Usage: perf stat [<options>] [<command>]
-e, --event <event> event selector. use 'perf list' to list available events
despite the fact that it makes as much sense as the previous one: perf stat -e 'cbox*/clockticks/'
I'd think let's keep just the glob matching, so it's clear
you what you use wildcards for.. thoughts?
thanks,
jirka
next prev parent reply other threads:[~2018-03-03 14:34 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-02 23:41 [RFC V2 0/3] perf stat: improvements for handling of multiple PMUs Agustin Vega-Frias
2018-03-02 23:41 ` Agustin Vega-Frias
2018-03-02 23:41 ` [RFC V2 1/3] perf, tools: Support wildcards on pmu name in dynamic pmu events Agustin Vega-Frias
2018-03-02 23:41 ` Agustin Vega-Frias
2018-03-03 14:34 ` Jiri Olsa [this message]
2018-03-03 14:34 ` Jiri Olsa
2018-03-04 17:12 ` Andi Kleen
2018-03-04 17:12 ` Andi Kleen
2018-03-04 18:10 ` Jiri Olsa
2018-03-04 18:10 ` Jiri Olsa
2018-03-05 15:08 ` Agustin Vega-Frias
2018-03-05 15:08 ` Agustin Vega-Frias
2018-03-05 17:55 ` Andi Kleen
2018-03-05 17:55 ` Andi Kleen
2018-03-05 19:09 ` Jiri Olsa
2018-03-05 19:09 ` Jiri Olsa
2018-03-05 20:10 ` Agustin Vega-Frias
2018-03-05 20:10 ` Agustin Vega-Frias
2018-03-05 21:51 ` Jiri Olsa
2018-03-05 21:51 ` Jiri Olsa
2018-03-02 23:41 ` [RFC V2 2/3] perf, tools: Display pmu name when printing unmerged events in stat Agustin Vega-Frias
2018-03-02 23:41 ` Agustin Vega-Frias
2018-03-02 23:41 ` [RFC V2 3/3] perf pmu: Auto-merge PMU events created by prefix or glob match Agustin Vega-Frias
2018-03-02 23:41 ` Agustin Vega-Frias
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180303143436.GA14563@krava \
--to=jolsa@redhat.com \
--cc=acme@kernel.org \
--cc=agustinv@codeaurora.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=timur@codeaurora.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.