* [RFC PATCH] perf pmu-events: Don't lower case MetricExpr
@ 2021-11-26 7:13 Ian Rogers
2022-01-12 17:22 ` Ian Rogers
0 siblings, 1 reply; 6+ messages in thread
From: Ian Rogers @ 2021-11-26 7:13 UTC (permalink / raw)
To: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Arnaldo Carvalho de Melo, Kan Liang,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
linux-perf-users, linux-kernel
Cc: eranian, Ian Rogers
This patch changes MetricExpr to be written out in the same case. This
enables events in metrics to use modifiers like 'G' which currently
yield parse errors when made lower case. To keep tests passing the
literal #smt_on is compared in a non-case sensitive way - #SMT_on is
present in at least SkylakeX metrics.
This patch is on top of:
https://lore.kernel.org/lkml/20211124001231.3277836-1-irogers@google.com/
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/pmu-events/jevents.c | 2 --
tools/perf/util/expr.c | 2 +-
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 2e7c4153875b..1a57c3f81dd4 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -672,8 +672,6 @@ static int json_events(const char *fn,
addfield(map, &je.metric_constraint, "", "", val);
} else if (json_streq(map, field, "MetricExpr")) {
addfield(map, &je.metric_expr, "", "", val);
- for (s = je.metric_expr; *s; s++)
- *s = tolower(*s);
} else if (json_streq(map, field, "ArchStdEvent")) {
addfield(map, &arch_std, "", "", val);
for (s = arch_std; *s; s++)
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index cdbab4f959fe..5cd6b9ff2489 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -397,7 +397,7 @@ double expr__get_literal(const char *literal)
static struct cpu_topology *topology;
double result = NAN;
- if (!strcmp("#smt_on", literal)) {
+ if (!strcasecmp("#smt_on", literal)) {
result = smt_on() > 0 ? 1.0 : 0.0;
goto out;
}
--
2.34.0.rc2.393.gf8c9666880-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] perf pmu-events: Don't lower case MetricExpr
2021-11-26 7:13 [RFC PATCH] perf pmu-events: Don't lower case MetricExpr Ian Rogers
@ 2022-01-12 17:22 ` Ian Rogers
2022-01-12 17:45 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Ian Rogers @ 2022-01-12 17:22 UTC (permalink / raw)
To: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Arnaldo Carvalho de Melo, Kan Liang,
Peter Zijlstra, Ingo Molnar, Mark Rutland, Alexander Shishkin,
linux-perf-users, linux-kernel
Cc: eranian
On Thu, Nov 25, 2021 at 11:13 PM Ian Rogers <irogers@google.com> wrote:
>
> This patch changes MetricExpr to be written out in the same case. This
> enables events in metrics to use modifiers like 'G' which currently
> yield parse errors when made lower case. To keep tests passing the
> literal #smt_on is compared in a non-case sensitive way - #SMT_on is
> present in at least SkylakeX metrics.
Ping.
Thanks,
Ian
> This patch is on top of:
> https://lore.kernel.org/lkml/20211124001231.3277836-1-irogers@google.com/
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/pmu-events/jevents.c | 2 --
> tools/perf/util/expr.c | 2 +-
> 2 files changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 2e7c4153875b..1a57c3f81dd4 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -672,8 +672,6 @@ static int json_events(const char *fn,
> addfield(map, &je.metric_constraint, "", "", val);
> } else if (json_streq(map, field, "MetricExpr")) {
> addfield(map, &je.metric_expr, "", "", val);
> - for (s = je.metric_expr; *s; s++)
> - *s = tolower(*s);
> } else if (json_streq(map, field, "ArchStdEvent")) {
> addfield(map, &arch_std, "", "", val);
> for (s = arch_std; *s; s++)
> diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
> index cdbab4f959fe..5cd6b9ff2489 100644
> --- a/tools/perf/util/expr.c
> +++ b/tools/perf/util/expr.c
> @@ -397,7 +397,7 @@ double expr__get_literal(const char *literal)
> static struct cpu_topology *topology;
> double result = NAN;
>
> - if (!strcmp("#smt_on", literal)) {
> + if (!strcasecmp("#smt_on", literal)) {
> result = smt_on() > 0 ? 1.0 : 0.0;
> goto out;
> }
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] perf pmu-events: Don't lower case MetricExpr
2022-01-12 17:22 ` Ian Rogers
@ 2022-01-12 17:45 ` Arnaldo Carvalho de Melo
2022-01-12 17:50 ` Arnaldo Carvalho de Melo
2022-01-12 17:53 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-12 17:45 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Kan Liang, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, linux-perf-users, linux-kernel,
eranian
Em Wed, Jan 12, 2022 at 09:22:51AM -0800, Ian Rogers escreveu:
> On Thu, Nov 25, 2021 at 11:13 PM Ian Rogers <irogers@google.com> wrote:
> >
> > This patch changes MetricExpr to be written out in the same case. This
> > enables events in metrics to use modifiers like 'G' which currently
> > yield parse errors when made lower case. To keep tests passing the
> > literal #smt_on is compared in a non-case sensitive way - #SMT_on is
> > present in at least SkylakeX metrics.
>
> Ping.
I tried applying 20211124001231.3277836-1-irogers@google.com on top of
your perf_cpu series, it failed, will check.
BTW, I got the two other patches in that series:
⬢[acme@toolbox perf]$ git log --oneline -2
6dd8646939a770e4 (HEAD -> perf/core) perf tools: Probe non-deprecated sysfs path 1st
0ce05781f4905fcf perf tools: Fix SMT fallback with large core counts
⬢[acme@toolbox perf]$
- Arnaldo
> Thanks,
> Ian
>
> > This patch is on top of:
> > https://lore.kernel.org/lkml/20211124001231.3277836-1-irogers@google.com/
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> > tools/perf/pmu-events/jevents.c | 2 --
> > tools/perf/util/expr.c | 2 +-
> > 2 files changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> > index 2e7c4153875b..1a57c3f81dd4 100644
> > --- a/tools/perf/pmu-events/jevents.c
> > +++ b/tools/perf/pmu-events/jevents.c
> > @@ -672,8 +672,6 @@ static int json_events(const char *fn,
> > addfield(map, &je.metric_constraint, "", "", val);
> > } else if (json_streq(map, field, "MetricExpr")) {
> > addfield(map, &je.metric_expr, "", "", val);
> > - for (s = je.metric_expr; *s; s++)
> > - *s = tolower(*s);
> > } else if (json_streq(map, field, "ArchStdEvent")) {
> > addfield(map, &arch_std, "", "", val);
> > for (s = arch_std; *s; s++)
> > diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
> > index cdbab4f959fe..5cd6b9ff2489 100644
> > --- a/tools/perf/util/expr.c
> > +++ b/tools/perf/util/expr.c
> > @@ -397,7 +397,7 @@ double expr__get_literal(const char *literal)
> > static struct cpu_topology *topology;
> > double result = NAN;
> >
> > - if (!strcmp("#smt_on", literal)) {
> > + if (!strcasecmp("#smt_on", literal)) {
> > result = smt_on() > 0 ? 1.0 : 0.0;
> > goto out;
> > }
> > --
> > 2.34.0.rc2.393.gf8c9666880-goog
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] perf pmu-events: Don't lower case MetricExpr
2022-01-12 17:45 ` Arnaldo Carvalho de Melo
@ 2022-01-12 17:50 ` Arnaldo Carvalho de Melo
2022-01-12 17:53 ` Arnaldo Carvalho de Melo
1 sibling, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-12 17:50 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Kan Liang, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, linux-perf-users, linux-kernel,
eranian
Em Wed, Jan 12, 2022 at 02:45:07PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Jan 12, 2022 at 09:22:51AM -0800, Ian Rogers escreveu:
> > On Thu, Nov 25, 2021 at 11:13 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > This patch changes MetricExpr to be written out in the same case. This
> > > enables events in metrics to use modifiers like 'G' which currently
> > > yield parse errors when made lower case. To keep tests passing the
> > > literal #smt_on is compared in a non-case sensitive way - #SMT_on is
> > > present in at least SkylakeX metrics.
> >
> > Ping.
>
> I tried applying 20211124001231.3277836-1-irogers@google.com on top of
> your perf_cpu series, it failed, will check.
>
> BTW, I got the two other patches in that series:
>
> ⬢[acme@toolbox perf]$ git log --oneline -2
> 6dd8646939a770e4 (HEAD -> perf/core) perf tools: Probe non-deprecated sysfs path 1st
> 0ce05781f4905fcf perf tools: Fix SMT fallback with large core counts
> ⬢[acme@toolbox perf]$
It was due to that cpu__max_present_cpu().cpu;
Fixed, applied.
- Arnaldo
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index e808738493e219fd..c94fb9bef919f5cb 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -405,12 +405,17 @@ double expr_id_data__source_count(const struct expr_id_data *data)
double expr__get_literal(const char *literal)
{
static struct cpu_topology *topology;
+ double result = NAN;
- if (!strcmp("#smt_on", literal))
- return smt_on() > 0 ? 1.0 : 0.0;
+ if (!strcmp("#smt_on", literal)) {
+ result = smt_on() > 0 ? 1.0 : 0.0;
+ goto out;
+ }
- if (!strcmp("#num_cpus", literal))
- return cpu__max_present_cpu().cpu;
+ if (!strcmp("#num_cpus", literal)) {
+ result = cpu__max_present_cpu().cpu;
+ goto out;
+ }
/*
* Assume that topology strings are consistent, such as CPUs "0-1"
@@ -422,16 +427,24 @@ double expr__get_literal(const char *literal)
topology = cpu_topology__new();
if (!topology) {
pr_err("Error creating CPU topology");
- return NAN;
+ goto out;
}
}
- if (!strcmp("#num_packages", literal))
- return topology->package_cpus_lists;
- if (!strcmp("#num_dies", literal))
- return topology->die_cpus_lists;
- if (!strcmp("#num_cores", literal))
- return topology->core_cpus_lists;
+ if (!strcmp("#num_packages", literal)) {
+ result = topology->package_cpus_lists;
+ goto out;
+ }
+ if (!strcmp("#num_dies", literal)) {
+ result = topology->die_cpus_lists;
+ goto out;
+ }
+ if (!strcmp("#num_cores", literal)) {
+ result = topology->core_cpus_lists;
+ goto out;
+ }
pr_err("Unrecognized literal '%s'", literal);
- return NAN;
+out:
+ pr_debug2("literal: %s = %f\n", literal, result);
+ return result;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] perf pmu-events: Don't lower case MetricExpr
2022-01-12 17:45 ` Arnaldo Carvalho de Melo
2022-01-12 17:50 ` Arnaldo Carvalho de Melo
@ 2022-01-12 17:53 ` Arnaldo Carvalho de Melo
2022-01-12 17:58 ` Arnaldo Carvalho de Melo
1 sibling, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-12 17:53 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Kan Liang, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, linux-perf-users, linux-kernel,
eranian
Em Wed, Jan 12, 2022 at 02:45:07PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Jan 12, 2022 at 09:22:51AM -0800, Ian Rogers escreveu:
> > On Thu, Nov 25, 2021 at 11:13 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > This patch changes MetricExpr to be written out in the same case. This
> > > enables events in metrics to use modifiers like 'G' which currently
> > > yield parse errors when made lower case. To keep tests passing the
> > > literal #smt_on is compared in a non-case sensitive way - #SMT_on is
> > > present in at least SkylakeX metrics.
> >
> > Ping.
>
> I tried applying 20211124001231.3277836-1-irogers@google.com on top of
> your perf_cpu series, it failed, will check.
>
> BTW, I got the two other patches in that series:
>
> ⬢[acme@toolbox perf]$ git log --oneline -2
> 6dd8646939a770e4 (HEAD -> perf/core) perf tools: Probe non-deprecated sysfs path 1st
> 0ce05781f4905fcf perf tools: Fix SMT fallback with large core counts
> ⬢[acme@toolbox perf]$
Ok, I have that one now on, but could Andi or somebody else that works
more frequently with that code provide an Acked-by or Reviewed-by?
- Arnaldo
> - Arnaldo
>
> > Thanks,
> > Ian
> >
> > > This patch is on top of:
> > > https://lore.kernel.org/lkml/20211124001231.3277836-1-irogers@google.com/
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > ---
> > > tools/perf/pmu-events/jevents.c | 2 --
> > > tools/perf/util/expr.c | 2 +-
> > > 2 files changed, 1 insertion(+), 3 deletions(-)
> > >
> > > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> > > index 2e7c4153875b..1a57c3f81dd4 100644
> > > --- a/tools/perf/pmu-events/jevents.c
> > > +++ b/tools/perf/pmu-events/jevents.c
> > > @@ -672,8 +672,6 @@ static int json_events(const char *fn,
> > > addfield(map, &je.metric_constraint, "", "", val);
> > > } else if (json_streq(map, field, "MetricExpr")) {
> > > addfield(map, &je.metric_expr, "", "", val);
> > > - for (s = je.metric_expr; *s; s++)
> > > - *s = tolower(*s);
> > > } else if (json_streq(map, field, "ArchStdEvent")) {
> > > addfield(map, &arch_std, "", "", val);
> > > for (s = arch_std; *s; s++)
> > > diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
> > > index cdbab4f959fe..5cd6b9ff2489 100644
> > > --- a/tools/perf/util/expr.c
> > > +++ b/tools/perf/util/expr.c
> > > @@ -397,7 +397,7 @@ double expr__get_literal(const char *literal)
> > > static struct cpu_topology *topology;
> > > double result = NAN;
> > >
> > > - if (!strcmp("#smt_on", literal)) {
> > > + if (!strcasecmp("#smt_on", literal)) {
> > > result = smt_on() > 0 ? 1.0 : 0.0;
> > > goto out;
> > > }
> > > --
> > > 2.34.0.rc2.393.gf8c9666880-goog
> > >
>
> --
>
> - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] perf pmu-events: Don't lower case MetricExpr
2022-01-12 17:53 ` Arnaldo Carvalho de Melo
@ 2022-01-12 17:58 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-01-12 17:58 UTC (permalink / raw)
To: Ian Rogers
Cc: Andi Kleen, Jiri Olsa, Namhyung Kim, John Garry, Kajol Jain,
Paul A . Clarke, Kan Liang, Peter Zijlstra, Ingo Molnar,
Mark Rutland, Alexander Shishkin, linux-perf-users, linux-kernel,
eranian
Em Wed, Jan 12, 2022 at 02:53:42PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Jan 12, 2022 at 02:45:07PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Jan 12, 2022 at 09:22:51AM -0800, Ian Rogers escreveu:
> > > On Thu, Nov 25, 2021 at 11:13 PM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > This patch changes MetricExpr to be written out in the same case. This
> > > > enables events in metrics to use modifiers like 'G' which currently
> > > > yield parse errors when made lower case. To keep tests passing the
> > > > literal #smt_on is compared in a non-case sensitive way - #SMT_on is
> > > > present in at least SkylakeX metrics.
> > >
> > > Ping.
> >
> > I tried applying 20211124001231.3277836-1-irogers@google.com on top of
> > your perf_cpu series, it failed, will check.
> >
> > BTW, I got the two other patches in that series:
> >
> > ⬢[acme@toolbox perf]$ git log --oneline -2
> > 6dd8646939a770e4 (HEAD -> perf/core) perf tools: Probe non-deprecated sysfs path 1st
> > 0ce05781f4905fcf perf tools: Fix SMT fallback with large core counts
> > ⬢[acme@toolbox perf]$
>
> Ok, I have that one now on, but could Andi or somebody else that works
> more frequently with that code provide an Acked-by or Reviewed-by?
Its there since since MetricExpr was introduced:
commit 00636c3b48e8acac2acd2601274c6eab4ecf8201
Author: Andi Kleen <ak@linux.intel.com>
Date: Mon Mar 20 13:17:07 2017 -0700
perf pmu: Support MetricExpr header in JSON event list
Add support for parsing the MetricExpr header in the JSON event lists
and storing them in the alias structure.
Used in the next patch.
v2: Change DividedBy to MetricExpr
v3: Really catch all uses of DividedBy
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/20170320201711.14142-10-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>o
-----------------------------
There is no explanation as to why it should be lowercased, and Ian's
reason not to lowercase it is valid, not to lose information, so I'll
apply the patch, please holler if there is some other subtlety I'm
missing...
- Arnaldo
> > > > This patch is on top of:
> > > > https://lore.kernel.org/lkml/20211124001231.3277836-1-irogers@google.com/
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > ---
> > > > tools/perf/pmu-events/jevents.c | 2 --
> > > > tools/perf/util/expr.c | 2 +-
> > > > 2 files changed, 1 insertion(+), 3 deletions(-)
> > > >
> > > > diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> > > > index 2e7c4153875b..1a57c3f81dd4 100644
> > > > --- a/tools/perf/pmu-events/jevents.c
> > > > +++ b/tools/perf/pmu-events/jevents.c
> > > > @@ -672,8 +672,6 @@ static int json_events(const char *fn,
> > > > addfield(map, &je.metric_constraint, "", "", val);
> > > > } else if (json_streq(map, field, "MetricExpr")) {
> > > > addfield(map, &je.metric_expr, "", "", val);
> > > > - for (s = je.metric_expr; *s; s++)
> > > > - *s = tolower(*s);
> > > > } else if (json_streq(map, field, "ArchStdEvent")) {
> > > > addfield(map, &arch_std, "", "", val);
> > > > for (s = arch_std; *s; s++)
> > > > diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
> > > > index cdbab4f959fe..5cd6b9ff2489 100644
> > > > --- a/tools/perf/util/expr.c
> > > > +++ b/tools/perf/util/expr.c
> > > > @@ -397,7 +397,7 @@ double expr__get_literal(const char *literal)
> > > > static struct cpu_topology *topology;
> > > > double result = NAN;
> > > >
> > > > - if (!strcmp("#smt_on", literal)) {
> > > > + if (!strcasecmp("#smt_on", literal)) {
> > > > result = smt_on() > 0 ? 1.0 : 0.0;
> > > > goto out;
> > > > }
> > > > --
> > > > 2.34.0.rc2.393.gf8c9666880-goog
> > > >
> >
> > --
> >
> > - Arnaldo
>
> --
>
> - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-12 17:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-26 7:13 [RFC PATCH] perf pmu-events: Don't lower case MetricExpr Ian Rogers
2022-01-12 17:22 ` Ian Rogers
2022-01-12 17:45 ` Arnaldo Carvalho de Melo
2022-01-12 17:50 ` Arnaldo Carvalho de Melo
2022-01-12 17:53 ` Arnaldo Carvalho de Melo
2022-01-12 17:58 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).