perf metric: Move runtime value to the expr context
authorIan Rogers <irogers@google.com>
Fri, 15 Oct 2021 17:21:16 +0000 (10:21 -0700)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Wed, 20 Oct 2021 13:33:02 +0000 (10:33 -0300)
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.

Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Andi Kleen <ak@linux.intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Antonov <alexander.antonov@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andrew Kilroy <andrew.kilroy@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Changbin Du <changbin.du@intel.com>
Cc: Denys Zagorui <dzagorui@cisco.com>
Cc: Fabian Hemmer <copy@copy.sh>
Cc: Felix Fietkau <nbd@nbd.name>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Joakim Zhang <qiangqing.zhang@nxp.com>
Cc: John Garry <john.garry@huawei.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Kees Kook <keescook@chromium.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Nicholas Fraser <nfraser@codeweavers.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Paul Clarke <pc@us.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Riccardo Mancini <rickyman7@gmail.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: ShihCheng Tu <mrtoastcheng@gmail.com>
Cc: Song Liu <songliubraving@fb.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Sumanth Korikkar <sumanthk@linux.ibm.com>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: Wan Jiabing <wanjiabing@vivo.com>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Link: https://lore.kernel.org/r/20211015172132.1162559-6-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/tests/expr.c
tools/perf/tests/pmu-events.c
tools/perf/util/expr.c
tools/perf/util/expr.h
tools/perf/util/metricgroup.c
tools/perf/util/stat-shadow.c

index f1d8411fce12065e492515348bf33dba537cdfa2..3c16f3df19807d46e9c6ad2575a3b1bfc7bf21f9 100644 (file)
@@ -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);
index cc5cea141bebe5865bada74b83fd1ab0627a6df6..71b08c2964103ace66526bbd78e5fbc97c7daf9b 100644 (file)
@@ -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;
index db2445677c8cf13c3307fb308f4768963e2dab54..62fb39fd4d9d243bba7810b88f07b075b0afad2d 100644 (file)
@@ -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);
index b20513f0ae59c46131e13ab6dc85d7f8e4ff038b..124475a4f24558d35fd9d48946a6d30c7ddf63e5 100644 (file)
@@ -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);
index b60ccbbf08297d069246a160491b3c883b1ff73b..139f4a793f92fb02b60d8413849007fe363dc7de 100644 (file)
@@ -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);
index 9bc841e09a0c7dd2724d482891052e501363b8ea..20f1b9d0f2722fd1c24b291ae0687293a7feadb8 100644 (file)
@@ -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: