From cb94a02e7494c001fa8b5a4c5e16693fafd98530 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 23 Sep 2021 00:46:04 -0700 Subject: perf metric: Restructure struct expr_parse_ctx. A later change to parsing the ids out (in expr__find_other) will potentially drop hashmaps and so it is more convenient to move expr_parse_ctx to have a hashmap pointer rather than a struct value. As this pointer must be freed, rather than just going out of scope, add expr__ctx_new and expr__ctx_free to manage expr_parse_ctx memory. Adjust use of struct expr_parse_ctx accordingly. Reviewed-by: Andi Kleen Signed-off-by: Ian Rogers Tested-by: John Garry Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Ingo Molnar Cc: Jin Yao Cc: Kajol Jain Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Sandeep Dasgupta Cc: Stephane Eranian Link: https://lore.kernel.org/r/20210923074616.674826-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 81 +++++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 39 deletions(-) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 4d01051951cd..b0a3b5fd0c00 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -22,67 +22,70 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) const char *p; double val; int ret; - struct expr_parse_ctx ctx; + struct expr_parse_ctx *ctx; - expr__ctx_init(&ctx); - expr__add_id_val(&ctx, strdup("FOO"), 1); - expr__add_id_val(&ctx, strdup("BAR"), 2); + ctx = expr__ctx_new(); + TEST_ASSERT_VAL("expr__ctx_new", ctx); + expr__add_id_val(ctx, strdup("FOO"), 1); + expr__add_id_val(ctx, strdup("BAR"), 2); - ret = test(&ctx, "1+1", 2); - ret |= test(&ctx, "FOO+BAR", 3); - ret |= test(&ctx, "(BAR/2)%2", 1); - ret |= test(&ctx, "1 - -4", 5); - ret |= test(&ctx, "(FOO-1)*2 + (BAR/2)%2 - -4", 5); - ret |= test(&ctx, "1-1 | 1", 1); - ret |= test(&ctx, "1-1 & 1", 0); - ret |= test(&ctx, "min(1,2) + 1", 2); - ret |= test(&ctx, "max(1,2) + 1", 3); - ret |= test(&ctx, "1+1 if 3*4 else 0", 2); - ret |= test(&ctx, "1.1 + 2.1", 3.2); - ret |= test(&ctx, ".1 + 2.", 2.1); - ret |= test(&ctx, "d_ratio(1, 2)", 0.5); - ret |= test(&ctx, "d_ratio(2.5, 0)", 0); - ret |= test(&ctx, "1.1 < 2.2", 1); - ret |= test(&ctx, "2.2 > 1.1", 1); - ret |= test(&ctx, "1.1 < 1.1", 0); - ret |= test(&ctx, "2.2 > 2.2", 0); - ret |= test(&ctx, "2.2 < 1.1", 0); - ret |= test(&ctx, "1.1 > 2.2", 0); + ret = test(ctx, "1+1", 2); + ret |= test(ctx, "FOO+BAR", 3); + ret |= test(ctx, "(BAR/2)%2", 1); + ret |= test(ctx, "1 - -4", 5); + ret |= test(ctx, "(FOO-1)*2 + (BAR/2)%2 - -4", 5); + ret |= test(ctx, "1-1 | 1", 1); + ret |= test(ctx, "1-1 & 1", 0); + ret |= test(ctx, "min(1,2) + 1", 2); + ret |= test(ctx, "max(1,2) + 1", 3); + ret |= test(ctx, "1+1 if 3*4 else 0", 2); + ret |= test(ctx, "1.1 + 2.1", 3.2); + ret |= test(ctx, ".1 + 2.", 2.1); + ret |= test(ctx, "d_ratio(1, 2)", 0.5); + ret |= test(ctx, "d_ratio(2.5, 0)", 0); + ret |= test(ctx, "1.1 < 2.2", 1); + ret |= test(ctx, "2.2 > 1.1", 1); + ret |= test(ctx, "1.1 < 1.1", 0); + ret |= test(ctx, "2.2 > 2.2", 0); + ret |= test(ctx, "2.2 < 1.1", 0); + ret |= test(ctx, "1.1 > 2.2", 0); - if (ret) + if (ret) { + expr__ctx_free(ctx); return ret; + } p = "FOO/0"; - ret = expr__parse(&val, &ctx, p, 1); + ret = expr__parse(&val, ctx, p, 1); TEST_ASSERT_VAL("division by zero", ret == -1); p = "BAR/"; - ret = expr__parse(&val, &ctx, p, 1); + ret = expr__parse(&val, ctx, p, 1); TEST_ASSERT_VAL("missing operand", ret == -1); - expr__ctx_clear(&ctx); + expr__ctx_clear(ctx); TEST_ASSERT_VAL("find other", expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", - &ctx, 1) == 0); - TEST_ASSERT_VAL("find other", hashmap__size(&ctx.ids) == 3); - TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "BAR", + ctx, 1) == 0); + TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 3); + TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BAR", (void **)&val_ptr)); - TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "BAZ", + TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BAZ", (void **)&val_ptr)); - TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "BOZO", + TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BOZO", (void **)&val_ptr)); - expr__ctx_clear(&ctx); + expr__ctx_clear(ctx); TEST_ASSERT_VAL("find other", expr__find_other("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@", - NULL, &ctx, 3) == 0); - TEST_ASSERT_VAL("find other", hashmap__size(&ctx.ids) == 2); - TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "EVENT1,param=3/", + NULL, ctx, 3) == 0); + TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 2); + TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT1,param=3/", (void **)&val_ptr)); - TEST_ASSERT_VAL("find other", hashmap__find(&ctx.ids, "EVENT2,param=3/", + TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT2,param=3/", (void **)&val_ptr)); - expr__ctx_clear(&ctx); + expr__ctx_free(ctx); return 0; } -- cgit From 7e06a5e30a0c5155291efab8cf866ffea052f829 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 23 Sep 2021 00:46:10 -0700 Subject: perf metric: Rename expr__find_other. A later change will remove the notion of other, rename the function to expr__find_ids as this is what it populates. Signed-off-by: Ian Rogers Tested-by: John Garry Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ingo Molnar Cc: Jin Yao Cc: Kajol Jain Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Sandeep Dasgupta Cc: Stephane Eranian Link: https://lore.kernel.org/r/20210923074616.674826-8-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index b0a3b5fd0c00..7ccb97c73347 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -64,25 +64,25 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) TEST_ASSERT_VAL("missing operand", ret == -1); expr__ctx_clear(ctx); - TEST_ASSERT_VAL("find other", - expr__find_other("FOO + BAR + BAZ + BOZO", "FOO", - ctx, 1) == 0); - TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 3); - TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BAR", + TEST_ASSERT_VAL("find ids", + expr__find_ids("FOO + BAR + BAZ + BOZO", "FOO", + ctx, 1) == 0); + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 3); + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "BAR", (void **)&val_ptr)); - TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BAZ", + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "BAZ", (void **)&val_ptr)); - TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "BOZO", + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "BOZO", (void **)&val_ptr)); expr__ctx_clear(ctx); - TEST_ASSERT_VAL("find other", - expr__find_other("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@", - NULL, ctx, 3) == 0); - TEST_ASSERT_VAL("find other", hashmap__size(ctx->ids) == 2); - TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT1,param=3/", + TEST_ASSERT_VAL("find ids", + expr__find_ids("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@", + NULL, ctx, 3) == 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)); - TEST_ASSERT_VAL("find other", hashmap__find(ctx->ids, "EVENT2,param=3/", + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT2,param=3/", (void **)&val_ptr)); expr__ctx_free(ctx); -- cgit From 114a9d6e396eeb061fa532803ff9a6fd3a966ad8 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 23 Sep 2021 00:46:11 -0700 Subject: perf metric: Add utilities to work on ids map. Add utilities to new/free an ids hashmap, as well as to union. Add testing of the union. Unioning hashmaps will be used when parsing the metric, if a value is known then the hashmap is unnecessary, otherwise we need to union together all the event ids to compute their values for reporting. Signed-off-by: Ian Rogers Tested-by: John Garry Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ingo Molnar Cc: Jin Yao Cc: Kajol Jain Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Sandeep Dasgupta Cc: Stephane Eranian Link: https://lore.kernel.org/r/20210923074616.674826-9-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 7ccb97c73347..1c881bea7fca 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -6,6 +6,51 @@ #include #include +static int test_ids_union(void) +{ + struct hashmap *ids1, *ids2; + + /* Empty union. */ + ids1 = ids__new(); + TEST_ASSERT_VAL("ids__new", ids1); + ids2 = ids__new(); + TEST_ASSERT_VAL("ids__new", ids2); + + ids1 = ids__union(ids1, ids2); + TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 0); + + /* Union {foo, bar} against {}. */ + ids2 = ids__new(); + TEST_ASSERT_VAL("ids__new", ids2); + + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("foo"), NULL), 0); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("bar"), NULL), 0); + + ids1 = ids__union(ids1, ids2); + TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 2); + + /* Union {foo, bar} against {foo}. */ + ids2 = ids__new(); + TEST_ASSERT_VAL("ids__new", ids2); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("foo"), NULL), 0); + + ids1 = ids__union(ids1, ids2); + TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 2); + + /* Union {foo, bar} against {bar,baz}. */ + ids2 = ids__new(); + TEST_ASSERT_VAL("ids__new", ids2); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("bar"), NULL), 0); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("baz"), NULL), 0); + + ids1 = ids__union(ids1, ids2); + TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 3); + + ids__free(ids1); + + return 0; +} + static int test(struct expr_parse_ctx *ctx, const char *e, double val2) { double val; @@ -24,6 +69,8 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) int ret; struct expr_parse_ctx *ctx; + TEST_ASSERT_EQUAL("ids_union", test_ids_union(), 0); + ctx = expr__ctx_new(); TEST_ASSERT_VAL("expr__ctx_new", ctx); expr__add_id_val(ctx, strdup("FOO"), 1); -- cgit From a8e4e880834b5dc53ff6b4cfc9f4268e61399976 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 23 Sep 2021 00:46:15 -0700 Subject: perf metric: Don't compute unused events For a metric like: EVENT1 if #smt_on else EVENT2 currently EVENT1 and EVENT2 will be measured and then when the metric is reported EVENT1 or EVENT2 will be printed depending on the value from smt_on() during the expr parsing. Computing both events is unnecessary and can lead to multiplexing as discussed in this thread: https://lore.kernel.org/lkml/20201110100346.2527031-1-irogers@google.com/ If the input is constant to certain operators like: IDS1 if CONST else IDS2 then the result will be either IDS1 or IDS2 depending on CONST (which may be evaluated from an entire expression), and so IDS1 or IDS2 may be discarded avoiding events from being programmed. Signed-off-by: Ian Rogers Tested-by: John Garry Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ingo Molnar Cc: Jin Yao Cc: Kajol Jain Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Sandeep Dasgupta Cc: Stephane Eranian Link: https://lore.kernel.org/r/20210923074616.674826-13-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 1c881bea7fca..287989321d2a 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 #include "util/debug.h" #include "util/expr.h" +#include "util/smt.h" #include "tests.h" #include #include @@ -132,6 +133,16 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT2,param=3/", (void **)&val_ptr)); + /* Only EVENT1 or EVENT2 need be measured depending on the value of smt_on. */ + expr__ctx_clear(ctx); + TEST_ASSERT_VAL("find ids", + expr__find_ids("EVENT1 if #smt_on else EVENT2", + NULL, ctx, 0) == 0); + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 1); + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, + smt_on() ? "EVENT1" : "EVENT2", + (void **)&val_ptr)); + expr__ctx_free(ctx); return 0; -- cgit From 94886961e324d5b6bae8e206b227c6eeb0f22c2c Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Thu, 23 Sep 2021 00:46:16 -0700 Subject: perf metric: Avoid events for an 'if' constant result For a metric like: CONST if expr else CONST if the values of CONST are identical then expr doesn't need evaluating, and events, in order to compute a result. Signed-off-by: Ian Rogers Tested-by: John Garry Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ingo Molnar Cc: Jin Yao Cc: Kajol Jain Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Sandeep Dasgupta Cc: Stephane Eranian Link: https://lore.kernel.org/r/20210923074616.674826-14-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 287989321d2a..f1d8411fce12 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -143,6 +143,13 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) smt_on() ? "EVENT1" : "EVENT2", (void **)&val_ptr)); + /* The expression is a constant 1.0 without needing to evaluate EVENT1. */ + 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); + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0); + expr__ctx_free(ctx); return 0; -- cgit From fa831fbb430853ad8c1abb18001dc87bed3cf52b Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Fri, 15 Oct 2021 10:21:16 -0700 Subject: 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. Signed-off-by: Ian Rogers Acked-by: Andi Kleen Cc: Adrian Hunter Cc: Alexander Antonov Cc: Alexander Shishkin Cc: Andrew Kilroy Cc: Andrew Morton Cc: Changbin Du Cc: Denys Zagorui Cc: Fabian Hemmer Cc: Felix Fietkau Cc: Heiko Carstens Cc: Ingo Molnar Cc: Jacob Keller Cc: Jiapeng Chong Cc: Jin Yao Cc: Jiri Olsa Cc: Joakim Zhang Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kees Kook Cc: Mark Rutland Cc: Namhyung Kim Cc: Nicholas Fraser Cc: Nick Desaulniers Cc: Paul Clarke Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Sami Tolvanen Cc: ShihCheng Tu Cc: Song Liu Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Thomas Richter Cc: Wan Jiabing Cc: Zhen Lei Link: https://lore.kernel.org/r/20211015172132.1162559-6-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'tools/perf/tests/expr.c') 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); -- cgit From 80be6434c36f40d82c26035b949d78d845fec044 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Fri, 15 Oct 2021 10:21:20 -0700 Subject: perf metric: Modify resolution and recursion check Modify resolution. Rather than resolving a list of metrics, resolve a metric immediately after it is added. This simplifies knowing the root of the metric's tree so that IDs may be associated with it. A bug in the current implementation is that all the IDs were placed on the first metric in a metric group. Rather than maintain data on IDs' parents to detect cycles, maintain a list of visited metrics and detect cycles if the same metric is visited twice. Only place the root metric onto the list of metrics. Signed-off-by: Ian Rogers Acked-by: Andi Kleen Cc: Adrian Hunter Cc: Alexander Antonov Cc: Alexander Shishkin Cc: Andrew Kilroy Cc: Andrew Morton Cc: Changbin Du Cc: Denys Zagorui Cc: Fabian Hemmer Cc: Felix Fietkau Cc: Heiko Carstens Cc: Ingo Molnar Cc: Jacob Keller Cc: Jiapeng Chong Cc: Jin Yao Cc: Jiri Olsa Cc: Joakim Zhang Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kees Kook Cc: Mark Rutland Cc: Namhyung Kim Cc: Nicholas Fraser Cc: Nick Desaulniers Cc: Paul Clarke Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Sami Tolvanen Cc: ShihCheng Tu Cc: Song Liu Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Thomas Richter Cc: Wan Jiabing Cc: Zhen Lei Link: https://lore.kernel.org/r/20211015172132.1162559-10-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 3c16f3df1980..718c13e5a0f4 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -24,8 +24,8 @@ static int test_ids_union(void) ids2 = ids__new(); TEST_ASSERT_VAL("ids__new", ids2); - TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("foo"), NULL), 0); - TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("bar"), NULL), 0); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("foo")), 0); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids1, strdup("bar")), 0); ids1 = ids__union(ids1, ids2); TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 2); @@ -33,7 +33,7 @@ static int test_ids_union(void) /* Union {foo, bar} against {foo}. */ ids2 = ids__new(); TEST_ASSERT_VAL("ids__new", ids2); - TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("foo"), NULL), 0); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("foo")), 0); ids1 = ids__union(ids1, ids2); TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 2); @@ -41,8 +41,8 @@ static int test_ids_union(void) /* Union {foo, bar} against {bar,baz}. */ ids2 = ids__new(); TEST_ASSERT_VAL("ids__new", ids2); - TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("bar"), NULL), 0); - TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("baz"), NULL), 0); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("bar")), 0); + TEST_ASSERT_EQUAL("ids__insert", ids__insert(ids2, strdup("baz")), 0); ids1 = ids__union(ids1, ids2); TEST_ASSERT_EQUAL("union", (int)hashmap__size(ids1), 3); -- cgit From ec5c5b3d2c21b3f332fdc9c026c42723fb8a0ce6 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Fri, 15 Oct 2021 10:21:27 -0700 Subject: perf metric: Encode and use metric-id as qualifier For a metric like IPC a group of events like {instructions,cycles}:W would be formed. If the events names were changed in parsing then the metric expression parser would fail to find them. This change makes the event encoding be something like: {instructions/metric-id=instructions/, cycles/metric-id=cycles/} and then uses the evsel's stable metric-id value to locate the events. This fixes the case that an event is restricted to user because of the paranoia setting: $ echo 2 > /proc/sys/kernel/perf_event_paranoid $ perf stat -M IPC /bin/true Performance counter stats for '/bin/true': 150,298 inst_retired.any:u # 0.77 IPC 187,095 cpu_clk_unhalted.thread:u 0.002042731 seconds time elapsed 0.000000000 seconds user 0.002377000 seconds sys Adding the metric-id as a qualifier has a complication in that qualifiers will become embedded in qualifiers. For example, msr/tsc/ could become msr/tsc,metric-id=msr/tsc// which will fail parse-events. To solve this problem the metric is encoded and decoded for the metric-id with ! standing in for an encoded value. Previously ! wasn't parsed. With this msr/tsc/ becomes msr/tsc,metric-id=msr!3tsc!3/ The metric expression parser is changed so that @ isn't changed to /, instead this is done when the ID is encoded for parse events. metricgroup__add_metric_non_group() and metricgroup__add_metric_weak_group() need to inject the metric-id qualifier, so to avoid repetition they are merged into a single metricgroup__build_event_string with error codes more rigorously checked. stat-shadow's prepare_metric() uses the metric-id to match the metricgroup code. As "metric-id=..." is added to all events, it is adding during testing with the fake PMU. This complicates pmu_str_check code as PE_PMU_EVENT_FAKE won't match as part of a configuration. The testing fake PMU case is fixed so that if a known qualifier with an ! is parsed then it isn't reported as a fake PMU. This is sufficient to pass all testing but it and the original mechanism are somewhat brittle. Signed-off-by: Ian Rogers Acked-by: Andi Kleen Cc: Adrian Hunter Cc: Alexander Antonov Cc: Alexander Shishkin Cc: Andrew Kilroy Cc: Andrew Morton Cc: Changbin Du Cc: Denys Zagorui Cc: Fabian Hemmer Cc: Felix Fietkau Cc: Heiko Carstens Cc: Ingo Molnar Cc: Jacob Keller Cc: Jiapeng Chong Cc: Jin Yao Cc: Jiri Olsa Cc: Joakim Zhang Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Kees Kook Cc: Mark Rutland Cc: Namhyung Kim Cc: Nicholas Fraser Cc: Nick Desaulniers Cc: Paul Clarke Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Sami Tolvanen Cc: ShihCheng Tu Cc: Song Liu Cc: Stephane Eranian Cc: Sumanth Korikkar Cc: Thomas Richter Cc: Wan Jiabing Cc: Zhen Lei Link: https://lore.kernel.org/r/20211015172132.1162559-17-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 718c13e5a0f4..077783223ce0 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -129,9 +129,9 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) expr__find_ids("EVENT1\\,param\\=?@ + EVENT2\\,param\\=?@", 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/", + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT1,param=3@", (void **)&val_ptr)); - TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT2,param=3/", + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT2,param=3@", (void **)&val_ptr)); /* Only EVENT1 or EVENT2 need be measured depending on the value of smt_on. */ -- cgit From d68f0365087395fe232e39ac9c8ee53627522c3c Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 3 Nov 2021 23:41:50 -0700 Subject: perf test: Move each test suite struct to its test Rather than export test functions, export the test struct. Rename with a suite__ prefix to avoid name collisions. Committer notes: Its '&suite__vectors_page', not '&suite__vectors_pages', noticed when cross building to arm (32-bit). Signed-off-by: Ian Rogers Tested-by: Sohaib Mohamed Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Brendan Higgins Cc: Daniel Latypov Cc: David Gow Cc: Ingo Molnar Cc: Jin Yao Cc: John Garry Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20211104064208.3156807-5-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 077783223ce0..8c6397d0c381 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -62,7 +62,7 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2) return 0; } -int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) +static int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) { struct expr_id_data *val_ptr; const char *p; @@ -155,3 +155,5 @@ int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) return 0; } + +DEFINE_SUITE("Simple expression parser", expr); -- cgit From 33f44bfd3c04e3559633c24b797044e849144fea Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 3 Nov 2021 23:41:51 -0700 Subject: perf test: Rename struct test to test_suite This is to align with kunit's terminology. Signed-off-by: Ian Rogers Tested-by: Sohaib Mohamed Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Brendan Higgins Cc: Daniel Latypov Cc: David Gow Cc: Ingo Molnar Cc: Jin Yao Cc: John Garry Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: https://lore.kernel.org/r/20211104064208.3156807-6-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 8c6397d0c381..0c041ac707e6 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -62,7 +62,7 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2) return 0; } -static int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) +static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_unused) { struct expr_id_data *val_ptr; const char *p; -- cgit From 604ce2f00465efdf6efa9708c0b9b1fc302f46f0 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 10 Nov 2021 16:21:02 -0800 Subject: perf test: Add expr test for events with hyphens An example of such an event is topdown-fe-bound. Signed-off-by: Ian Rogers Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ingo Molnar Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Madhavan Srinivasan Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul A . Clarke Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Song Liu Cc: Wan Jiabing Cc: Yury Norov Link: https://lore.kernel.org/r/20211111002109.194172-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 0c041ac707e6..48c62d0789c5 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -134,6 +134,16 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "EVENT2,param=3@", (void **)&val_ptr)); + expr__ctx_clear(ctx); + TEST_ASSERT_VAL("find ids", + expr__find_ids("dash\\-event1 - dash\\-event2", + NULL, ctx) == 0); + TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 2); + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "dash-event1", + (void **)&val_ptr)); + TEST_ASSERT_VAL("find ids", hashmap__find(ctx->ids, "dash-event2", + (void **)&val_ptr)); + /* Only EVENT1 or EVENT2 need be measured depending on the value of smt_on. */ expr__ctx_clear(ctx); TEST_ASSERT_VAL("find ids", -- cgit From fdf1e29b6118c18fbf2003321fcf474d4bb778ec Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 10 Nov 2021 16:21:07 -0800 Subject: perf expr: Add metric literals for topology. Allow the number of cpus, cores, dies and packages to be queried by a metric expression. Signed-off-by: Ian Rogers Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ingo Molnar Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Madhavan Srinivasan Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul A . Clarke Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Song Liu Cc: Wan Jiabing Cc: Yury Norov Link: https://lore.kernel.org/r/20211111002109.194172-7-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index 48c62d0789c5..ed95ebda49a7 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -66,7 +66,7 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u { struct expr_id_data *val_ptr; const char *p; - double val; + double val, num_cpus, num_cores, num_dies, num_packages; int ret; struct expr_parse_ctx *ctx; @@ -161,6 +161,16 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u NULL, ctx) == 0); TEST_ASSERT_VAL("find ids", hashmap__size(ctx->ids) == 0); + /* Test toplogy constants appear well ordered. */ + expr__ctx_clear(ctx); + TEST_ASSERT_VAL("#num_cpus", expr__parse(&num_cpus, ctx, "#num_cpus") == 0); + TEST_ASSERT_VAL("#num_cores", expr__parse(&num_cores, ctx, "#num_cores") == 0); + TEST_ASSERT_VAL("#num_cpus >= #num_cores", num_cpus >= num_cores); + TEST_ASSERT_VAL("#num_dies", expr__parse(&num_dies, ctx, "#num_dies") == 0); + TEST_ASSERT_VAL("#num_cores >= #num_dies", num_cores >= num_dies); + TEST_ASSERT_VAL("#num_packages", expr__parse(&num_packages, ctx, "#num_packages") == 0); + TEST_ASSERT_VAL("#num_dies >= #num_packages", num_dies >= num_packages); + expr__ctx_free(ctx); return 0; -- cgit From 9aba0adae8c773ba0c0adc7c4d97768e044166cb Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 10 Nov 2021 16:21:09 -0800 Subject: perf expr: Add source_count for aggregating events Events like uncore_imc/cas_count_read/ on Skylake open multiple events and then aggregate in the metric leader. To determine the average value per event the number of these events is needed. Add a source_count function that returns this value by counting the number of events with the given metric leader. For most events the value is 1 but for uncore_imc/cas_count_read/ it can yield values like 6. Add a generic test, but manually tested with a test metric that uses the function. Signed-off-by: Ian Rogers Acked-by: Jiri Olsa Cc: Alexander Shishkin Cc: Andi Kleen Cc: Ingo Molnar Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Madhavan Srinivasan Cc: Mark Rutland Cc: Namhyung Kim Cc: Paul A . Clarke Cc: Peter Zijlstra Cc: Riccardo Mancini Cc: Song Liu Cc: Wan Jiabing Cc: Yury Norov Link: https://lore.kernel.org/r/20211111002109.194172-9-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'tools/perf/tests/expr.c') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index ed95ebda49a7..c895de481fe1 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -171,6 +171,18 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u TEST_ASSERT_VAL("#num_packages", expr__parse(&num_packages, ctx, "#num_packages") == 0); TEST_ASSERT_VAL("#num_dies >= #num_packages", num_dies >= num_packages); + /* + * Source count returns the number of events aggregating in a leader + * event including the leader. Check parsing yields an id. + */ + expr__ctx_clear(ctx); + TEST_ASSERT_VAL("source count", + expr__find_ids("source_count(EVENT1)", + NULL, ctx) == 0); + TEST_ASSERT_VAL("source count", hashmap__size(ctx->ids) == 1); + TEST_ASSERT_VAL("source count", hashmap__find(ctx->ids, "EVENT1", + (void **)&val_ptr)); + expr__ctx_free(ctx); return 0; -- cgit