[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20211015172132.1162559-6-irogers@google.com>
Date: Fri, 15 Oct 2021 10:21:16 -0700
From: Ian Rogers <irogers@...gle.com>
To: Andi Kleen <ak@...ux.intel.com>, Jiri Olsa <jolsa@...hat.com>,
Jin Yao <yao.jin@...ux.intel.com>,
Namhyung Kim <namhyung@...nel.org>,
John Garry <john.garry@...wei.com>,
Kajol Jain <kjain@...ux.ibm.com>,
"Paul A . Clarke" <pc@...ibm.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Riccardo Mancini <rickyman7@...il.com>,
Kan Liang <kan.liang@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Kees Cook <keescook@...omium.org>,
Sami Tolvanen <samitolvanen@...gle.com>,
Nick Desaulniers <ndesaulniers@...gle.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Jacob Keller <jacob.e.keller@...el.com>,
Zhen Lei <thunder.leizhen@...wei.com>,
ToastC <mrtoastcheng@...il.com>,
Joakim Zhang <qiangqing.zhang@....com>,
Felix Fietkau <nbd@....name>,
Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>,
Song Liu <songliubraving@...com>, Fabian Hemmer <copy@...y.sh>,
Alexander Antonov <alexander.antonov@...ux.intel.com>,
Nicholas Fraser <nfraser@...eweavers.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Denys Zagorui <dzagorui@...co.com>,
Wan Jiabing <wanjiabing@...o.com>,
Thomas Richter <tmricht@...ux.ibm.com>,
Sumanth Korikkar <sumanthk@...ux.ibm.com>,
Heiko Carstens <hca@...ux.ibm.com>,
Changbin Du <changbin.du@...el.com>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
Andrew Kilroy <andrew.kilroy@....com>
Cc: Stephane Eranian <eranian@...gle.com>,
Ian Rogers <irogers@...gle.com>
Subject: [PATCH v2 05/21] perf metric: Move runtime value to the expr context
The runtime value is needed when recursively parsing metrics, currently
a value of 1 is passed which is incorrect. Rather than add more
arguments to the bison parser, add runtime to the context. Fix call
sites not to pass a value. The runtime value is defaulted to 0, which is
arbitrary. In some places this replaces a value of 1, which was also
arbitrary. This shouldn't affect anything other than PPC. The use of
0 or 1 shouldn't matter as a proper runtime value would be needed in a
case that it did matter.
Acked-by: Andi Kleen <ak@...ux.intel.com>
Signed-off-by: Ian Rogers <irogers@...gle.com>
---
tools/perf/tests/expr.c | 15 ++++++++-------
tools/perf/tests/pmu-events.c | 10 +++++-----
tools/perf/util/expr.c | 15 ++++++++-------
tools/perf/util/expr.h | 5 +++--
tools/perf/util/metricgroup.c | 7 +++----
tools/perf/util/stat-shadow.c | 7 ++++---
6 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index f1d8411fce12..3c16f3df1980 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -56,7 +56,7 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2)
{
double val;
- if (expr__parse(&val, ctx, e, 1))
+ if (expr__parse(&val, ctx, e))
TEST_ASSERT_VAL("parse test failed", 0);
TEST_ASSERT_VAL("unexpected value", val == val2);
return 0;
@@ -104,17 +104,17 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
}
p = "FOO/0";
- ret = expr__parse(&val, ctx, p, 1);
+ ret = expr__parse(&val, ctx, p);
TEST_ASSERT_VAL("division by zero", ret == -1);
p = "BAR/";
- ret = expr__parse(&val, ctx, p, 1);
+ ret = expr__parse(&val, ctx, p);
TEST_ASSERT_VAL("missing operand", ret == -1);
expr__ctx_clear(ctx);
TEST_ASSERT_VAL("find ids",
expr__find_ids("FOO + BAR + BAZ + BOZO", "FOO",
- ctx, 1) == 0);
+ ctx) == 0);
TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 3);
TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "BAR",
(void **)&val_ptr));
@@ -124,9 +124,10 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
(void **)&val_ptr));
expr__ctx_clear(ctx);
+ ctx->runtime = 3;
TEST_ASSERT_VAL("find ids",
expr__find_ids("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@",
- NULL, ctx, 3) == 0);
+ NULL, ctx) == 0);
TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 2);
TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1,param=3/",
(void **)&val_ptr));
@@ -137,7 +138,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
expr__ctx_clear(ctx);
TEST_ASSERT_VAL("find ids",
expr__find_ids("EVENT1 if #smt_on else EVENT2",
- NULL, ctx, 0) == 0);
+ NULL, ctx) == 0);
TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1);
TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids,
smt_on() ? "EVENT1" : "EVENT2",
@@ -147,7 +148,7 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused)
expr__ctx_clear(ctx);
TEST_ASSERT_VAL("find ids",
expr__find_ids("1.0 if EVENT1 > 100.0 else 1.0",
- NULL, ctx, 0) == 0);
+ NULL, ctx) == 0);
TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0);
expr__ctx_free(ctx);
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index cc5cea141beb..71b08c296410 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -866,7 +866,7 @@ static int resolve_metric_simple(struct expr_parse_ctx *pctx,
ref->metric_expr = pe->metric_expr;
list_add_tail(&metric->list, compound_list);
- rc = expr__find_ids(pe->metric_expr, NULL, pctx, 0);
+ rc = expr__find_ids(pe->metric_expr, NULL, pctx);
if (rc)
goto out_err;
break; /* The hashmap has been modified, so restart */
@@ -916,7 +916,7 @@ static int test_parsing(void)
if (!pe->metric_expr)
continue;
expr__ctx_clear(ctx);
- if (expr__find_ids(pe->metric_expr, NULL, ctx, 0) < 0) {
+ if (expr__find_ids(pe->metric_expr, NULL, ctx) < 0) {
expr_failure("Parse find ids failed", map, pe);
ret++;
continue;
@@ -949,7 +949,7 @@ static int test_parsing(void)
free(metric);
}
- if (expr__parse(&result, ctx, pe->metric_expr, 0)) {
+ if (expr__parse(&result, ctx, pe->metric_expr)) {
expr_failure("Parse failed", map, pe);
ret++;
}
@@ -989,7 +989,7 @@ static int metric_parse_fake(const char *str)
pr_debug("expr__ctx_new failed");
return TEST_FAIL;
}
- if (expr__find_ids(str, NULL, ctx, 0) < 0) {
+ if (expr__find_ids(str, NULL, ctx) < 0) {
pr_err("expr__find_ids failed\n");
return -1;
}
@@ -1010,7 +1010,7 @@ static int metric_parse_fake(const char *str)
}
}
- if (expr__parse(&result, ctx, str, 0))
+ if (expr__parse(&result, ctx, str))
pr_err("expr__parse failed\n");
else
ret = 0;
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index db2445677c8c..62fb39fd4d9d 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -246,7 +246,7 @@ int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id,
data->ref.metric_name);
pr_debug("processing metric: %s ENTRY\n", id);
data->kind = EXPR_ID_DATA__REF_VALUE;
- if (expr__parse(&data->ref.val, ctx, data->ref.metric_expr, 1)) {
+ if (expr__parse(&data->ref.val, ctx, data->ref.metric_expr)) {
pr_debug("%s failed to count\n", id);
return -1;
}
@@ -284,6 +284,7 @@ struct expr_parse_ctx *expr__ctx_new(void)
ctx->ids = hashmap__new(key_hash, key_equal, NULL);
ctx->parent = NULL;
+ ctx->runtime = 0;
return ctx;
}
@@ -314,10 +315,10 @@ void expr__ctx_free(struct expr_parse_ctx *ctx)
static int
__expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
- bool compute_ids, int runtime)
+ bool compute_ids)
{
struct expr_scanner_ctx scanner_ctx = {
- .runtime = runtime,
+ .runtime = ctx->runtime,
};
YY_BUFFER_STATE buffer;
void *scanner;
@@ -345,15 +346,15 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr,
}
int expr__parse(double *final_val, struct expr_parse_ctx *ctx,
- const char *expr, int runtime)
+ const char *expr)
{
- return __expr__parse(final_val, ctx, expr, /*compute_ids=*/false, runtime) ? -1 : 0;
+ return __expr__parse(final_val, ctx, expr, /*compute_ids=*/false) ? -1 : 0;
}
int expr__find_ids(const char *expr, const char *one,
- struct expr_parse_ctx *ctx, int runtime)
+ struct expr_parse_ctx *ctx)
{
- int ret = __expr__parse(NULL, ctx, expr, /*compute_ids=*/true, runtime);
+ int ret = __expr__parse(NULL, ctx, expr, /*compute_ids=*/true);
if (one)
expr__del_id(ctx, one);
diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h
index b20513f0ae59..124475a4f245 100644
--- a/tools/perf/util/expr.h
+++ b/tools/perf/util/expr.h
@@ -21,6 +21,7 @@ struct expr_id {
struct expr_parse_ctx {
struct hashmap *ids;
struct expr_id *parent;
+ int runtime;
};
struct expr_id_data;
@@ -52,10 +53,10 @@ int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id,
struct expr_id_data **datap);
int expr__parse(double *final_val, struct expr_parse_ctx *ctx,
- const char *expr, int runtime);
+ const char *expr);
int expr__find_ids(const char *expr, const char *one,
- struct expr_parse_ctx *ids, int runtime);
+ struct expr_parse_ctx *ids);
double expr_id_data__value(const struct expr_id_data *data);
struct expr_id *expr_id_data__parent(struct expr_id_data *data);
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index b60ccbbf0829..139f4a793f92 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -124,7 +124,6 @@ struct metric {
const char *metric_unit;
struct list_head metric_refs;
int metric_refs_cnt;
- int runtime;
bool has_constraint;
};
@@ -391,7 +390,7 @@ static int metricgroup__setup_events(struct list_head *groups,
expr->metric_name = m->metric_name;
expr->metric_unit = m->metric_unit;
expr->metric_events = metric_events;
- expr->runtime = m->runtime;
+ expr->runtime = m->pctx->runtime;
list_add(&expr->nd, &me->head);
}
@@ -812,7 +811,7 @@ static int __add_metric(struct list_head *metric_list,
m->metric_name = pe->metric_name;
m->metric_expr = pe->metric_expr;
m->metric_unit = pe->unit;
- m->runtime = runtime;
+ m->pctx->runtime = runtime;
m->has_constraint = metric_no_group || metricgroup__has_constraint(pe);
INIT_LIST_HEAD(&m->metric_refs);
m->metric_refs_cnt = 0;
@@ -862,7 +861,7 @@ static int __add_metric(struct list_head *metric_list,
* For both the parent and referenced metrics, we parse
* all the metric's IDs and add it to the parent context.
*/
- if (expr__find_ids(pe->metric_expr, NULL, m->pctx, runtime) < 0) {
+ if (expr__find_ids(pe->metric_expr, NULL, m->pctx) < 0) {
if (m->metric_refs_cnt == 0) {
expr__ctx_free(m->pctx);
free(m);
diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 9bc841e09a0c..20f1b9d0f272 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -394,7 +394,7 @@ void perf_stat__collect_metric_expr(struct evlist *evsel_list)
if (!metric_events) {
if (expr__find_ids(counter->metric_expr,
counter->name,
- ctx, 1) < 0)
+ ctx) < 0)
continue;
metric_events = calloc(sizeof(struct evsel *),
@@ -894,13 +894,14 @@ static void generic_metric(struct perf_stat_config *config,
if (!pctx)
return;
+ pctx->runtime = runtime;
i = prepare_metric(metric_events, metric_refs, pctx, cpu, st);
if (i < 0) {
expr__ctx_free(pctx);
return;
}
if (!metric_events[i]) {
- if (expr__parse(&ratio, pctx, metric_expr, runtime) == 0) {
+ if (expr__parse(&ratio, pctx, metric_expr) == 0) {
char *unit;
char metric_bf[64];
@@ -951,7 +952,7 @@ double test_generic_metric(struct metric_expr *mexp, int cpu, struct runtime_sta
if (prepare_metric(mexp->metric_events, mexp->metric_refs, pctx, cpu, st) < 0)
goto out;
- if (expr__parse(&ratio, pctx, mexp->metric_expr, 1))
+ if (expr__parse(&ratio, pctx, mexp->metric_expr))
ratio = 0.0;
out:
--
2.33.0.1079.g6e70778dc9-goog
Powered by blists - more mailing lists