* [PATCH 1/3] perf tools: Factor metric addition into add_metric function
2019-12-11 22:47 [RFC 0/3] perf tools: Add support for used defined metric Jiri Olsa
@ 2019-12-11 22:47 ` Jiri Olsa
2019-12-11 22:47 ` [PATCH 2/3] perf tools: Factor metric setup code into metricgroup__setup function Jiri Olsa
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-12-11 22:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Joe Mario, Andi Kleen,
Kajol Jain
Factoring metric addition into add_metric function,
so it can be used to add metric from different sources
in following patches.
Link: https://lkml.kernel.org/n/tip-qw1727kbewu315mz2h0y5xfm@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/metricgroup.c | 110 +++++++++++++++++++---------------
1 file changed, 62 insertions(+), 48 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 6a4d350d5cdb..1d01958c148d 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -396,13 +396,65 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
strlist__delete(metriclist);
}
+static int add_metric(const char *name, const char *expr, const char *unit,
+ struct strbuf *events, struct list_head *group_list)
+{
+ bool no_group = false;
+ struct egroup *eg;
+ const char **ids;
+ int idnum, j;
+
+ pr_debug("metric expr %s for %s\n", expr, name);
+
+ if (expr__find_other(expr, NULL, &ids, &idnum) < 0)
+ return -1;
+
+ if (events->len > 0)
+ strbuf_addf(events, ",");
+
+ for (j = 0; j < idnum; j++) {
+ pr_debug("found event %s\n", ids[j]);
+ /*
+ * Duration time maps to a software event and can make
+ * groups not count. Always use it outside a
+ * group.
+ */
+ if (!strcmp(ids[j], "duration_time")) {
+ if (j > 0)
+ strbuf_addf(events, "}:W,");
+ strbuf_addf(events, "duration_time");
+ no_group = true;
+ continue;
+ }
+ strbuf_addf(events, "%s%s",
+ j == 0 || no_group ? "{" : ",",
+ ids[j]);
+ no_group = false;
+ }
+
+ if (!no_group)
+ strbuf_addf(events, "}:W");
+
+ eg = malloc(sizeof(struct egroup));
+ if (!eg)
+ return -ENOMEM;
+
+ eg->ids = ids;
+ eg->idnum = idnum;
+ eg->metric_name = name;
+ eg->metric_expr = expr;
+ eg->metric_unit = unit;
+ list_add_tail(&eg->nd, group_list);
+ return 0;
+}
+
static int metricgroup__add_metric(const char *metric, struct strbuf *events,
struct list_head *group_list)
{
struct pmu_events_map *map = perf_pmu__find_map(NULL);
struct pmu_event *pe;
int ret = -EINVAL;
- int i, j;
+ int i;
if (!map)
return 0;
@@ -414,55 +466,17 @@ static int metricgroup__add_metric(const char *metric, struct strbuf *events,
break;
if (!pe->metric_expr)
continue;
- if (match_metric(pe->metric_group, metric) ||
- match_metric(pe->metric_name, metric)) {
- const char **ids;
- int idnum;
- struct egroup *eg;
- bool no_group = false;
- pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
-
- if (expr__find_other(pe->metric_expr,
- NULL, &ids, &idnum) < 0)
- continue;
- if (events->len > 0)
- strbuf_addf(events, ",");
- for (j = 0; j < idnum; j++) {
- pr_debug("found event %s\n", ids[j]);
- /*
- * Duration time maps to a software event and can make
- * groups not count. Always use it outside a
- * group.
- */
- if (!strcmp(ids[j], "duration_time")) {
- if (j > 0)
- strbuf_addf(events, "}:W,");
- strbuf_addf(events, "duration_time");
- no_group = true;
- continue;
- }
- strbuf_addf(events, "%s%s",
- j == 0 || no_group ? "{" : ",",
- ids[j]);
- no_group = false;
- }
- if (!no_group)
- strbuf_addf(events, "}:W");
+ if (!match_metric(pe->metric_group, metric) &&
+ !match_metric(pe->metric_name, metric))
+ continue;
- eg = malloc(sizeof(struct egroup));
- if (!eg) {
- ret = -ENOMEM;
- break;
- }
- eg->ids = ids;
- eg->idnum = idnum;
- eg->metric_name = pe->metric_name;
- eg->metric_expr = pe->metric_expr;
- eg->metric_unit = pe->unit;
- list_add_tail(&eg->nd, group_list);
- ret = 0;
- }
+ ret = add_metric(pe->metric_name,
+ pe->metric_expr,
+ pe->unit,
+ events, group_list);
+ if (ret)
+ break;
}
return ret;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] perf tools: Factor metric setup code into metricgroup__setup function
2019-12-11 22:47 [RFC 0/3] perf tools: Add support for used defined metric Jiri Olsa
2019-12-11 22:47 ` [PATCH 1/3] perf tools: Factor metric addition into add_metric function Jiri Olsa
@ 2019-12-11 22:47 ` Jiri Olsa
2019-12-11 22:48 ` [PATCH 3/3] perf stat: Add --metric option Jiri Olsa
2019-12-11 23:01 ` [RFC 0/3] perf tools: Add support for used defined metric Andi Kleen
3 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-12-11 22:47 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Joe Mario, Andi Kleen,
Kajol Jain
Factoring metric setup code into metricgroup__setup function,
so it can be used to add metric from different sources in
following patches.
Link: https://lkml.kernel.org/n/tip-7hjix4t30ls3qqd4l60dbr2n@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/metricgroup.c | 38 +++++++++++++++++++++++------------
1 file changed, 25 insertions(+), 13 deletions(-)
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 1d01958c148d..abcfa3c1b4d5 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -521,32 +521,44 @@ static void metricgroup__free_egroups(struct list_head *group_list)
}
}
+static int metricgroup__setup(struct evlist *evlist,
+ struct rblist *metric_events,
+ struct strbuf *extra_events,
+ struct list_head *group_list)
+{
+ struct parse_events_error parse_error;
+ int ret;
+
+ pr_debug("adding %s\n", extra_events->buf);
+ bzero(&parse_error, sizeof(parse_error));
+
+ ret = parse_events(evlist, extra_events->buf, &parse_error);
+ if (ret) {
+ parse_events_print_error(&parse_error, extra_events->buf);
+ return ret;
+ }
+
+ if (metric_events->nr_entries == 0)
+ metricgroup__rblist_init(metric_events);
+ return metricgroup__setup_events(group_list, evlist, metric_events);
+}
+
int metricgroup__parse_groups(const struct option *opt,
const char *str,
struct rblist *metric_events)
{
- struct parse_events_error parse_error;
struct evlist *perf_evlist = *(struct evlist **)opt->value;
struct strbuf extra_events;
LIST_HEAD(group_list);
int ret;
- if (metric_events->nr_entries == 0)
- metricgroup__rblist_init(metric_events);
ret = metricgroup__add_metric_list(str, &extra_events, &group_list);
if (ret)
return ret;
- pr_debug("adding %s\n", extra_events.buf);
- bzero(&parse_error, sizeof(parse_error));
- ret = parse_events(perf_evlist, extra_events.buf, &parse_error);
- if (ret) {
- parse_events_print_error(&parse_error, extra_events.buf);
- goto out;
- }
+
+ ret = metricgroup__setup(perf_evlist, metric_events, &extra_events,
+ &group_list);
strbuf_release(&extra_events);
- ret = metricgroup__setup_events(&group_list, perf_evlist,
- metric_events);
-out:
metricgroup__free_egroups(&group_list);
return ret;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] perf stat: Add --metric option
2019-12-11 22:47 [RFC 0/3] perf tools: Add support for used defined metric Jiri Olsa
2019-12-11 22:47 ` [PATCH 1/3] perf tools: Factor metric addition into add_metric function Jiri Olsa
2019-12-11 22:47 ` [PATCH 2/3] perf tools: Factor metric setup code into metricgroup__setup function Jiri Olsa
@ 2019-12-11 22:48 ` Jiri Olsa
2019-12-11 23:02 ` Andi Kleen
2019-12-11 23:01 ` [RFC 0/3] perf tools: Add support for used defined metric Andi Kleen
3 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2019-12-11 22:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: lkml, Ingo Molnar, Namhyung Kim, Alexander Shishkin,
Peter Zijlstra, Michael Petlan, Joe Mario, Andi Kleen,
Kajol Jain
Adding --metric option that allows to specify metric on the command
line, like:
# perf stat --metric 'DECODED_ICACHE_UOPS% = 100 * (idq.dsb_uops / \
(idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops))' ...
The syntax of the --metric option argument is:
[name[/unit]=]expression
where:
name - is string that will identify expression in results
(can't have = or /) default is "user metric"
unit - is conversion number that multiplies result
Examples:
ipc = instructions / cycles
ipc/1 = instructions / cycles
instructions / cycles
Currently only one metric can be passed to perf stat command.
The code facilitates the current metric code.
Link: https://lkml.kernel.org/n/tip-oe1ke93t9x9uc1hy0iueksqq@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/Documentation/perf-stat.txt | 16 ++++++++++
tools/perf/builtin-stat.c | 21 +++++++++++++
tools/perf/util/metricgroup.c | 43 ++++++++++++++++++++++++++
tools/perf/util/metricgroup.h | 2 ++
4 files changed, 82 insertions(+)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 9431b8066fb4..6a76faec91f1 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -264,6 +264,22 @@ For a group all metrics from the group are added.
The events from the metrics are automatically measured.
See perf list output for the possble metrics and metricgroups.
+--metric::
+Print metric specified by the argument, where argument is defined as:
+
+ [name[/unit]=]expression
+
+ where:
+ name - is string that will identify expression in results
+ (can't have = or /) default is "user metric"
+ unit - is conversion number that multiplies result
+ default is 1
+
+ Examples:
+ ipc = instructions / cycles
+ ipc/1 = instructions / cycles
+ instructions / cycles
+
-A::
--no-aggr::
Do not aggregate counts across all monitored CPUs.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index a098c2ebf4ea..206df6f1cc8a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -161,6 +161,7 @@ static bool append_file;
static bool interval_count;
static const char *output_name;
static int output_fd;
+static char *user_metric;
struct perf_stat {
bool record;
@@ -841,6 +842,22 @@ static int parse_metric_groups(const struct option *opt,
return metricgroup__parse_groups(opt, str, &stat_config.metric_events);
}
+static int parse_metric_expr(const struct option *opt,
+ const char *str,
+ int unset __maybe_unused)
+{
+ if (user_metric) {
+ pr_err("Only one user metric is currently supported.\n");
+ return -EINVAL;
+ }
+
+ user_metric = strdup(str);
+ if (!user_metric)
+ return -ENOMEM;
+
+ return metricgroup__parse_expr(opt, user_metric, &stat_config.metric_events);
+}
+
static struct option stat_options[] = {
OPT_BOOLEAN('T', "transaction", &transaction_run,
"hardware transaction statistics"),
@@ -923,6 +940,9 @@ static struct option stat_options[] = {
OPT_CALLBACK('M', "metrics", &evsel_list, "metric/metric group list",
"monitor specified metrics or metric groups (separated by ,)",
parse_metric_groups),
+ OPT_CALLBACK('m', "metric", &evsel_list, "metric expression",
+ "monitor specified metric expression ([name/unit=]expression)",
+ parse_metric_expr),
OPT_BOOLEAN_FLAG(0, "all-kernel", &stat_config.all_kernel,
"Configure all used events to run in kernel space.",
PARSE_OPT_EXCLUSIVE),
@@ -2184,6 +2204,7 @@ int cmd_stat(int argc, const char **argv)
perf_evlist__free_stats(evsel_list);
out:
zfree(&stat_config.walltime_run);
+ zfree(&user_metric);
if (smi_cost && smi_reset)
sysfs__write_int(FREEZE_ON_SMI_PATH, 0);
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index abcfa3c1b4d5..c85be0ad8227 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -563,6 +563,49 @@ int metricgroup__parse_groups(const struct option *opt,
return ret;
}
+int metricgroup__parse_expr(const struct option *opt, char *str,
+ struct rblist *metric_events)
+{
+ struct evlist *perf_evlist = *(struct evlist **)opt->value;
+ char *tok, *expr, *unit = NULL;
+ struct strbuf extra_events;
+ LIST_HEAD(group_list);
+ const char *name;
+ int ret;
+
+ /*
+ * user metric is passed as following argument:
+ * [name[/unit]=]expression
+ */
+ tok = strchr(str, '=');
+ if (tok) {
+ *tok++ = 0;
+ name = str;
+ expr = tok;
+
+ tok = strchr(name, '/');
+ if (tok) {
+ *tok++ = 0;
+ unit = tok;
+ }
+ } else {
+ expr = str;
+ name = "user metric";
+ }
+
+ strbuf_init(&extra_events, 100);
+
+ ret = add_metric(name, expr, unit, &extra_events, &group_list);
+ if (ret)
+ return ret;
+
+ ret = metricgroup__setup(perf_evlist, metric_events, &extra_events,
+ &group_list);
+ strbuf_release(&extra_events);
+ metricgroup__free_egroups(&group_list);
+ return ret;
+}
+
bool metricgroup__has_metric(const char *metric)
{
struct pmu_events_map *map = perf_pmu__find_map(NULL);
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 475c7f912864..b66546b3ce1c 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -30,6 +30,8 @@ struct metric_event *metricgroup__lookup(struct rblist *metric_events,
int metricgroup__parse_groups(const struct option *opt,
const char *str,
struct rblist *metric_events);
+int metricgroup__parse_expr(const struct option *opt, char *str,
+ struct rblist *metric_events);
void metricgroup__print(bool metrics, bool groups, char *filter,
bool raw, bool details);
--
2.21.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] perf stat: Add --metric option
2019-12-11 22:48 ` [PATCH 3/3] perf stat: Add --metric option Jiri Olsa
@ 2019-12-11 23:02 ` Andi Kleen
2019-12-12 9:41 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Andi Kleen @ 2019-12-11 23:02 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan, Joe Mario,
Kajol Jain
> Currently only one metric can be passed to perf stat command.
It's near certain that users will want more.
-Andi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] perf stat: Add --metric option
2019-12-11 23:02 ` Andi Kleen
@ 2019-12-12 9:41 ` Jiri Olsa
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-12-12 9:41 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar,
Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
Joe Mario, Kajol Jain
On Wed, Dec 11, 2019 at 03:02:23PM -0800, Andi Kleen wrote:
> > Currently only one metric can be passed to perf stat command.
>
> It's near certain that users will want more.
right, it was just convenient for the rfc ;-)
but I wonder we could keep it that way anyway and have:
current way of using pre-defined metrics:
# perf stat -M m1,m2,m3
simple/fast way of checking on metric:
# perf stat -m 'm4=....'
metric from user file:
# perf stat -M m5,m6 --metric-file=path
thanks,
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC 0/3] perf tools: Add support for used defined metric
2019-12-11 22:47 [RFC 0/3] perf tools: Add support for used defined metric Jiri Olsa
` (2 preceding siblings ...)
2019-12-11 22:48 ` [PATCH 3/3] perf stat: Add --metric option Jiri Olsa
@ 2019-12-11 23:01 ` Andi Kleen
2019-12-12 9:37 ` Jiri Olsa
3 siblings, 1 reply; 8+ messages in thread
From: Andi Kleen @ 2019-12-11 23:01 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, lkml, Ingo Molnar, Namhyung Kim,
Alexander Shishkin, Peter Zijlstra, Michael Petlan, Joe Mario,
Kajol Jain
On Wed, Dec 11, 2019 at 11:47:57PM +0100, Jiri Olsa wrote:
> hi,
> Joe asked for possibility to add user defined metrics. Given that
> we already have metrics support, I added --metric option that allows
> to specify metric on the command line, like:
>
> # perf stat --metric 'DECODED_ICACHE_UOPS% = 100 * (idq.dsb_uops / \
> (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops))' ...
>
> The code facilitates the current metric code, and I was surprised
> how easy it was, so I'm not sure I omitted something ;-)
There are some asserts you can hit, like for too many events.
Also some of the syntax (e.g. using @ instead of / and escapes) are
not super user friendly.
Other than that it should be ok.
Of course it would be better to put it into a file,
and then support comments etc.
I've been considering some extensions to perf to support @file
reading with comment support.
Right now there are some very odd bugs when you do that, lie
-e 'event,<newline>
event,<newline>
event...'
adds the newline to the event name, which breaks the output formats.
-Andi
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC 0/3] perf tools: Add support for used defined metric
2019-12-11 23:01 ` [RFC 0/3] perf tools: Add support for used defined metric Andi Kleen
@ 2019-12-12 9:37 ` Jiri Olsa
0 siblings, 0 replies; 8+ messages in thread
From: Jiri Olsa @ 2019-12-12 9:37 UTC (permalink / raw)
To: Andi Kleen
Cc: Jiri Olsa, Arnaldo Carvalho de Melo, lkml, Ingo Molnar,
Namhyung Kim, Alexander Shishkin, Peter Zijlstra, Michael Petlan,
Joe Mario, Kajol Jain
On Wed, Dec 11, 2019 at 03:01:26PM -0800, Andi Kleen wrote:
> On Wed, Dec 11, 2019 at 11:47:57PM +0100, Jiri Olsa wrote:
> > hi,
> > Joe asked for possibility to add user defined metrics. Given that
> > we already have metrics support, I added --metric option that allows
> > to specify metric on the command line, like:
> >
> > # perf stat --metric 'DECODED_ICACHE_UOPS% = 100 * (idq.dsb_uops / \
> > (idq.ms_uops + idq.mite_uops + idq.dsb_uops + lsd.uops))' ...
> >
> > The code facilitates the current metric code, and I was surprised
> > how easy it was, so I'm not sure I omitted something ;-)
>
> There are some asserts you can hit, like for too many events.
>
> Also some of the syntax (e.g. using @ instead of / and escapes) are
> not super user friendly.
>
> Other than that it should be ok.
>
> Of course it would be better to put it into a file,
> and then support comments etc.
should be also reasonable easy, redirecting the input for lex
>
> I've been considering some extensions to perf to support @file
> reading with comment support.
>
> Right now there are some very odd bugs when you do that, lie
>
> -e 'event,<newline>
> event,<newline>
> event...'
>
> adds the newline to the event name, which breaks the output formats.
should be possible to ignore new lines at some point no?
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread