From 2c46f54249c6e86d8d81b5ea2452319f197caf17 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 12 Jul 2020 15:26:17 +0200 Subject: perf metric: Rename expr__add_id() to expr__add_val() Rename expr__add_id() to expr__add_val() so we can use expr__add_id() to actually add just the id without any value in following changes. There's no functional change. Signed-off-by: Jiri Olsa Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Kajol Jain Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200712132634.138901-2-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/expr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index 8a2c1074f90f..bb6bac836b48 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -22,7 +22,7 @@ struct expr_scanner_ctx { void expr__ctx_init(struct expr_parse_ctx *ctx); void expr__ctx_clear(struct expr_parse_ctx *ctx); -int expr__add_id(struct expr_parse_ctx *ctx, const char *id, double val); +int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val); int expr__get_id(struct expr_parse_ctx *ctx, const char *id, double *val_ptr); int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int runtime); -- cgit v1.2.3 From 070b3b5ad7bd077e673cad2c591a2ecf49c0b58a Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 12 Jul 2020 15:26:18 +0200 Subject: perf metric: Add 'struct expr_id_data' to keep expr value Add 'struct expr_id_data' to keep an expr value instead of just a simple double pointer, so we can store more data for ID in the following changes. Signed-off-by: Jiri Olsa Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Kajol Jain Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200712132634.138901-3-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/expr.c | 3 ++- tools/perf/util/expr.c | 22 +++++++++++----------- tools/perf/util/expr.h | 4 ++++ tools/perf/util/metricgroup.c | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c index d13fc1dfd5ef..4d01051951cd 100644 --- a/tools/perf/tests/expr.c +++ b/tools/perf/tests/expr.c @@ -18,8 +18,9 @@ static int test(struct expr_parse_ctx *ctx, const char *e, double val2) int test__expr(struct test *t __maybe_unused, int subtest __maybe_unused) { + struct expr_id_data *val_ptr; const char *p; - double val, *val_ptr; + double val; int ret; struct expr_parse_ctx ctx; diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index 9116a3a01eea..5d05f9765ed8 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -33,32 +33,32 @@ static bool key_equal(const void *key1, const void *key2, } /* Caller must make sure id is allocated */ -int expr__add_id_val(struct expr_parse_ctx *ctx, const char *name, double val) +int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val) { - double *val_ptr = NULL, *old_val = NULL; + struct expr_id_data *data_ptr = NULL, *old_data = NULL; char *old_key = NULL; int ret; if (val != 0.0) { - val_ptr = malloc(sizeof(double)); - if (!val_ptr) + data_ptr = malloc(sizeof(*data_ptr)); + if (!data_ptr) return -ENOMEM; - *val_ptr = val; + data_ptr->val = val; } - ret = hashmap__set(&ctx->ids, name, val_ptr, - (const void **)&old_key, (void **)&old_val); + ret = hashmap__set(&ctx->ids, id, data_ptr, + (const void **)&old_key, (void **)&old_data); free(old_key); - free(old_val); + free(old_data); return ret; } int expr__get_id(struct expr_parse_ctx *ctx, const char *id, double *val_ptr) { - double *data; + struct expr_id_data *data; if (!hashmap__find(&ctx->ids, id, (void **)&data)) return -1; - *val_ptr = (data == NULL) ? 0.0 : *data; + *val_ptr = (data == NULL) ? 0.0 : data->val; return 0; } @@ -119,7 +119,7 @@ int expr__parse(double *final_val, struct expr_parse_ctx *ctx, int expr__find_other(const char *expr, const char *one, struct expr_parse_ctx *ctx, int runtime) { - double *old_val = NULL; + struct expr_id_data *old_val = NULL; char *old_key = NULL; int ret = __expr__parse(NULL, ctx, expr, EXPR_OTHER, runtime); diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index bb6bac836b48..21fe5bd85718 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -15,6 +15,10 @@ struct expr_parse_ctx { struct hashmap ids; }; +struct expr_id_data { + double val; +}; + struct expr_scanner_ctx { int start_token; int runtime; diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index 82fecb5a302d..df0356ec120d 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -138,7 +138,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist, unsigned long *evlist_used) { struct evsel *ev, *current_leader = NULL; - double *val_ptr; + struct expr_id_data *val_ptr; int i = 0, matched_events = 0, events_to_match; const int idnum = (int)hashmap__size(&pctx->ids); -- cgit v1.2.3 From 332603c2aa1a3be92720343269e670f8118a8831 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 19 Jul 2020 20:13:03 +0200 Subject: perf metric: Add expr__add_id function Add the expr__add_id() function to data for ID with zero value, which is used when scanning the expression for IDs. Signed-off-by: Jiri Olsa Reviewed-by: Kajol Jain Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200719181320.785305-3-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/expr.c | 31 +++++++++++++++++++++++++------ tools/perf/util/expr.h | 1 + tools/perf/util/expr.y | 2 +- 3 files changed, 27 insertions(+), 7 deletions(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index 578a173d4873..9228f60e2a20 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -32,6 +32,26 @@ static bool key_equal(const void *key1, const void *key2, return !strcmp((const char *)key1, (const char *)key2); } +/* Caller must make sure id is allocated */ +int expr__add_id(struct expr_parse_ctx *ctx, const char *id) +{ + struct expr_id_data *data_ptr = NULL, *old_data = NULL; + char *old_key = NULL; + int ret; + + data_ptr = malloc(sizeof(*data_ptr)); + if (!data_ptr) + return -ENOMEM; + + ret = hashmap__set(&ctx->ids, id, data_ptr, + (const void **)&old_key, (void **)&old_data); + if (ret) + free(data_ptr); + free(old_key); + free(old_data); + return ret; +} + /* Caller must make sure id is allocated */ int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val) { @@ -39,12 +59,11 @@ int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val) char *old_key = NULL; int ret; - if (val != 0.0) { - data_ptr = malloc(sizeof(*data_ptr)); - if (!data_ptr) - return -ENOMEM; - data_ptr->val = val; - } + data_ptr = malloc(sizeof(*data_ptr)); + if (!data_ptr) + return -ENOMEM; + data_ptr->val = val; + ret = hashmap__set(&ctx->ids, id, data_ptr, (const void **)&old_key, (void **)&old_data); if (ret) diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index 21fe5bd85718..ac32cda42006 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -26,6 +26,7 @@ struct expr_scanner_ctx { void expr__ctx_init(struct expr_parse_ctx *ctx); void expr__ctx_clear(struct expr_parse_ctx *ctx); +int expr__add_id(struct expr_parse_ctx *ctx, const char *id); int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val); int expr__get_id(struct expr_parse_ctx *ctx, const char *id, double *val_ptr); int expr__parse(double *final_val, struct expr_parse_ctx *ctx, diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y index b2b3420ea6ec..8befe4a46f87 100644 --- a/tools/perf/util/expr.y +++ b/tools/perf/util/expr.y @@ -69,7 +69,7 @@ all_other: all_other other other: ID { - expr__add_id_val(ctx, $1, 0.0); + expr__add_id(ctx, $1); } | MIN | MAX | IF | ELSE | SMT_ON | NUMBER | '|' | '^' | '&' | '-' | '+' | '*' | '/' | '%' | '(' | ')' | ',' -- cgit v1.2.3 From 5c5f5e835f7e40e74f4a8370abb32647b11c3366 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 19 Jul 2020 20:13:04 +0200 Subject: perf metric: Change expr__get_id to return struct expr_id_data Changing expr__get_id to use and return struct expr_id_data pointer as value for the ID. This way we can access data other than value for given ID in following changes. Signed-off-by: Jiri Olsa Reviewed-by: Kajol Jain Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200719181320.785305-4-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/expr.c | 10 +++------- tools/perf/util/expr.h | 3 ++- tools/perf/util/expr.y | 14 +++++++++----- 3 files changed, 14 insertions(+), 13 deletions(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index 9228f60e2a20..4e5a6533dfce 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -73,14 +73,10 @@ int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val) return ret; } -int expr__get_id(struct expr_parse_ctx *ctx, const char *id, double *val_ptr) +int expr__get_id(struct expr_parse_ctx *ctx, const char *id, + struct expr_id_data **data) { - struct expr_id_data *data; - - if (!hashmap__find(&ctx->ids, id, (void **)&data)) - return -1; - *val_ptr = (data == NULL) ? 0.0 : data->val; - return 0; + return hashmap__find(&ctx->ids, id, (void **)data) ? 0 : -1; } void expr__ctx_init(struct expr_parse_ctx *ctx) diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index ac32cda42006..f38292fdab19 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -28,7 +28,8 @@ void expr__ctx_init(struct expr_parse_ctx *ctx); void expr__ctx_clear(struct expr_parse_ctx *ctx); int expr__add_id(struct expr_parse_ctx *ctx, const char *id); int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val); -int expr__get_id(struct expr_parse_ctx *ctx, const char *id, double *val_ptr); +int expr__get_id(struct expr_parse_ctx *ctx, const char *id, + struct expr_id_data **data); int expr__parse(double *final_val, struct expr_parse_ctx *ctx, const char *expr, int runtime); int expr__find_other(const char *expr, const char *one, diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y index 8befe4a46f87..0d4f5d324be7 100644 --- a/tools/perf/util/expr.y +++ b/tools/perf/util/expr.y @@ -85,12 +85,16 @@ if_expr: ; expr: NUMBER - | ID { if (expr__get_id(ctx, $1, &$$)) { - pr_debug("%s not found\n", $1); + | ID { + struct expr_id_data *data; + + if (expr__get_id(ctx, $1, &data) || !data) { + pr_debug("%s not found\n", $1); + free($1); + YYABORT; + } + $$ = data->val; free($1); - YYABORT; - } - free($1); } | expr '|' expr { $$ = (long)$1 | (long)$3; } | expr '&' expr { $$ = (long)$1 & (long)$3; } -- cgit v1.2.3 From 3fd29fa6c1644026ec7adbe017cacdf03724e6bc Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 19 Jul 2020 20:13:05 +0200 Subject: perf metric: Add expr__del_id function Adding expr__del_id function to remove ID from hashmap. It will save us few lines in following changes. Signed-off-by: Jiri Olsa Reviewed-by: Kajol Jain Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200719181320.785305-5-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/expr.c | 21 +++++++++++++-------- tools/perf/util/expr.h | 1 + 2 files changed, 14 insertions(+), 8 deletions(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index 4e5a6533dfce..f726211f49d4 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -79,6 +79,17 @@ int expr__get_id(struct expr_parse_ctx *ctx, const char *id, return hashmap__find(&ctx->ids, id, (void **)data) ? 0 : -1; } +void expr__del_id(struct expr_parse_ctx *ctx, const char *id) +{ + struct expr_id_data *old_val = NULL; + char *old_key = NULL; + + hashmap__delete(&ctx->ids, id, + (const void **)&old_key, (void **)&old_val); + free(old_key); + free(old_val); +} + void expr__ctx_init(struct expr_parse_ctx *ctx) { hashmap__init(&ctx->ids, key_hash, key_equal, NULL); @@ -136,16 +147,10 @@ int expr__parse(double *final_val, struct expr_parse_ctx *ctx, int expr__find_other(const char *expr, const char *one, struct expr_parse_ctx *ctx, int runtime) { - struct expr_id_data *old_val = NULL; - char *old_key = NULL; int ret = __expr__parse(NULL, ctx, expr, EXPR_OTHER, runtime); - if (one) { - hashmap__delete(&ctx->ids, one, - (const void **)&old_key, (void **)&old_val); - free(old_key); - free(old_val); - } + if (one) + expr__del_id(ctx, one); return ret; } diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index f38292fdab19..2462abd0ac65 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -26,6 +26,7 @@ struct expr_scanner_ctx { void expr__ctx_init(struct expr_parse_ctx *ctx); void expr__ctx_clear(struct expr_parse_ctx *ctx); +void expr__del_id(struct expr_parse_ctx *ctx, const char *id); int expr__add_id(struct expr_parse_ctx *ctx, const char *id); int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val); int expr__get_id(struct expr_parse_ctx *ctx, const char *id, -- cgit v1.2.3 From fc393839c11bbe2c7f1a44ab34e5f2a219d8366e Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 19 Jul 2020 20:13:11 +0200 Subject: perf metric: Add referenced metrics to hash data Adding referenced metrics to the parsing context so they can be resolved during the metric processing. Adding expr__add_ref function to store referenced metrics into parse context. Signed-off-by: Jiri Olsa Reviewed-by: Kajol Jain Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200719181320.785305-11-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/expr.c | 54 +++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/expr.h | 13 ++++++++++- tools/perf/util/stat-shadow.c | 20 +++++++++++----- 3 files changed, 80 insertions(+), 7 deletions(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index f726211f49d4..d3997c2b4a90 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -4,10 +4,14 @@ #include #include #include +#include "metricgroup.h" +#include "debug.h" #include "expr.h" #include "expr-bison.h" #include "expr-flex.h" #include +#include +#include #ifdef PARSER_DEBUG extern int expr_debug; @@ -63,6 +67,7 @@ int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val) if (!data_ptr) return -ENOMEM; data_ptr->val = val; + data_ptr->is_ref = false; ret = hashmap__set(&ctx->ids, id, data_ptr, (const void **)&old_key, (void **)&old_data); @@ -73,6 +78,55 @@ int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val) return ret; } +int expr__add_ref(struct expr_parse_ctx *ctx, struct metric_ref *ref) +{ + struct expr_id_data *data_ptr = NULL, *old_data = NULL; + char *old_key = NULL; + char *name, *p; + int ret; + + data_ptr = zalloc(sizeof(*data_ptr)); + if (!data_ptr) + return -ENOMEM; + + name = strdup(ref->metric_name); + if (!name) { + free(data_ptr); + return -ENOMEM; + } + + /* + * The jevents tool converts all metric expressions + * to lowercase, including metric references, hence + * we need to add lowercase name for metric, so it's + * properly found. + */ + for (p = name; *p; p++) + *p = tolower(*p); + + /* + * Intentionally passing just const char pointers, + * originally from 'struct pmu_event' object. + * We don't need to change them, so there's no + * need to create our own copy. + */ + data_ptr->ref.metric_name = ref->metric_name; + data_ptr->ref.metric_expr = ref->metric_expr; + data_ptr->is_ref = true; + + ret = hashmap__set(&ctx->ids, name, data_ptr, + (const void **)&old_key, (void **)&old_data); + if (ret) + free(data_ptr); + + pr_debug2("adding ref metric %s: %s\n", + ref->metric_name, ref->metric_expr); + + free(old_key); + free(old_data); + return ret; +} + int expr__get_id(struct expr_parse_ctx *ctx, const char *id, struct expr_id_data **data) { diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index 2462abd0ac65..81d04ff7f857 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -11,12 +11,22 @@ #include "util/hashmap.h" //#endif +struct metric_ref; + struct expr_parse_ctx { struct hashmap ids; }; struct expr_id_data { - double val; + union { + double val; + struct { + const char *metric_name; + const char *metric_expr; + } ref; + }; + + bool is_ref; }; struct expr_scanner_ctx { @@ -29,6 +39,7 @@ void expr__ctx_clear(struct expr_parse_ctx *ctx); void expr__del_id(struct expr_parse_ctx *ctx, const char *id); int expr__add_id(struct expr_parse_ctx *ctx, const char *id); int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val); +int expr__add_ref(struct expr_parse_ctx *ctx, struct metric_ref *ref); int expr__get_id(struct expr_parse_ctx *ctx, const char *id, struct expr_id_data **data); int expr__parse(double *final_val, struct expr_parse_ctx *ctx, diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c index fc9ac4b4218e..e1ba6c1b916a 100644 --- a/tools/perf/util/stat-shadow.c +++ b/tools/perf/util/stat-shadow.c @@ -731,13 +731,14 @@ static void print_smi_cost(struct perf_stat_config *config, } static int prepare_metric(struct evsel **metric_events, + struct metric_ref *metric_refs, struct expr_parse_ctx *pctx, int cpu, struct runtime_stat *st) { double scale; char *n, *pn; - int i; + int i, j, ret; expr__ctx_init(pctx); for (i = 0; metric_events[i]; i++) { @@ -778,12 +779,19 @@ static int prepare_metric(struct evsel **metric_events, expr__add_id_val(pctx, n, avg_stats(stats)*scale); } + for (j = 0; metric_refs && metric_refs[j].metric_name; j++) { + ret = expr__add_ref(pctx, &metric_refs[j]); + if (ret) + return ret; + } + return i; } static void generic_metric(struct perf_stat_config *config, const char *metric_expr, struct evsel **metric_events, + struct metric_ref *metric_refs, char *name, const char *metric_name, const char *metric_unit, @@ -798,7 +806,7 @@ static void generic_metric(struct perf_stat_config *config, int i; void *ctxp = out->ctx; - i = prepare_metric(metric_events, &pctx, cpu, st); + i = prepare_metric(metric_events, metric_refs, &pctx, cpu, st); if (i < 0) return; @@ -847,7 +855,7 @@ double test_generic_metric(struct metric_expr *mexp, int cpu, struct runtime_sta struct expr_parse_ctx pctx; double ratio; - if (prepare_metric(mexp->metric_events, &pctx, cpu, st) < 0) + if (prepare_metric(mexp->metric_events, mexp->metric_refs, &pctx, cpu, st) < 0) return 0.; if (expr__parse(&ratio, &pctx, mexp->metric_expr, 1)) @@ -1064,8 +1072,8 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config, else print_metric(config, ctxp, NULL, NULL, name, 0); } else if (evsel->metric_expr) { - generic_metric(config, evsel->metric_expr, evsel->metric_events, evsel->name, - evsel->metric_name, NULL, 1, cpu, out, st); + generic_metric(config, evsel->metric_expr, evsel->metric_events, NULL, + evsel->name, evsel->metric_name, NULL, 1, cpu, out, st); } else if (runtime_stat_n(st, STAT_NSECS, 0, cpu) != 0) { char unit = 'M'; char unit_buf[10]; @@ -1093,7 +1101,7 @@ void perf_stat__print_shadow_stats(struct perf_stat_config *config, if (num++ > 0) out->new_line(config, ctxp); generic_metric(config, mexp->metric_expr, mexp->metric_events, - evsel->name, mexp->metric_name, + mexp->metric_refs, evsel->name, mexp->metric_name, mexp->metric_unit, mexp->runtime, cpu, out, st); } } -- cgit v1.2.3 From acf71b05d1a19726594a8436ba9d8af871941e6c Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 19 Jul 2020 20:13:12 +0200 Subject: perf metric: Compute referenced metrics Adding computation (expr__parse call) of referenced metric at the point when it needs to be resolved during the parent metric computation. Once the inner metric is computed, the result is stored and used if there's another usage of that metric. Signed-off-by: Jiri Olsa Reviewed-by: Kajol Jain Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200719181320.785305-12-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/expr.c | 31 +++++++++++++++++++++++++++++++ tools/perf/util/expr.h | 3 +++ tools/perf/util/expr.y | 4 ++-- 3 files changed, 36 insertions(+), 2 deletions(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index d3997c2b4a90..a346ca590513 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -112,6 +112,7 @@ int expr__add_ref(struct expr_parse_ctx *ctx, struct metric_ref *ref) */ data_ptr->ref.metric_name = ref->metric_name; data_ptr->ref.metric_expr = ref->metric_expr; + data_ptr->ref.counted = false; data_ptr->is_ref = true; ret = hashmap__set(&ctx->ids, name, data_ptr, @@ -133,6 +134,34 @@ int expr__get_id(struct expr_parse_ctx *ctx, const char *id, return hashmap__find(&ctx->ids, id, (void **)data) ? 0 : -1; } +int expr__resolve_id(struct expr_parse_ctx *ctx, const char *id, + struct expr_id_data **datap) +{ + struct expr_id_data *data; + + if (expr__get_id(ctx, id, datap) || !*datap) { + pr_debug("%s not found\n", id); + return -1; + } + + data = *datap; + + pr_debug2("lookup: is_ref %d, counted %d, val %f: %s\n", + data->is_ref, data->ref.counted, data->val, id); + + if (data->is_ref && !data->ref.counted) { + data->ref.counted = true; + pr_debug("processing metric: %s ENTRY\n", id); + if (expr__parse(&data->val, ctx, data->ref.metric_expr, 1)) { + pr_debug("%s failed to count\n", id); + return -1; + } + pr_debug("processing metric: %s EXIT: %f\n", id, data->val); + } + + return 0; +} + void expr__del_id(struct expr_parse_ctx *ctx, const char *id) { struct expr_id_data *old_val = NULL; @@ -173,6 +202,8 @@ __expr__parse(double *val, struct expr_parse_ctx *ctx, const char *expr, void *scanner; int ret; + pr_debug2("parsing metric: %s\n", expr); + ret = expr_lex_init_extra(&scanner_ctx, &scanner); if (ret) return ret; diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index 81d04ff7f857..9ed208d93418 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -23,6 +23,7 @@ struct expr_id_data { struct { const char *metric_name; const char *metric_expr; + bool counted; } ref; }; @@ -42,6 +43,8 @@ int expr__add_id_val(struct expr_parse_ctx *ctx, const char *id, double val); int expr__add_ref(struct expr_parse_ctx *ctx, struct metric_ref *ref); int expr__get_id(struct expr_parse_ctx *ctx, const char *id, struct expr_id_data **data); +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); int expr__find_other(const char *expr, const char *one, diff --git a/tools/perf/util/expr.y b/tools/perf/util/expr.y index 0d4f5d324be7..d34b370391c6 100644 --- a/tools/perf/util/expr.y +++ b/tools/perf/util/expr.y @@ -88,11 +88,11 @@ expr: NUMBER | ID { struct expr_id_data *data; - if (expr__get_id(ctx, $1, &data) || !data) { - pr_debug("%s not found\n", $1); + if (expr__resolve_id(ctx, $1, &data)) { free($1); YYABORT; } + $$ = data->val; free($1); } -- cgit v1.2.3 From f6fb0960f920e3040088992f32bbceded7a74322 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Sun, 19 Jul 2020 20:13:16 +0200 Subject: perf metric: Add recursion check when processing nested metrics Keeping the stack of nested metrics via 'struct expr_id' objects and checking if we are in recursion via already processed metric. The stack is implemented as static array within the struct egroup with 100 entries, which should be enough nesting depth for any metric we have or plan to have at the moment. Adding test that simulates the recursion and checks we can detect it. Committer notes: Bumped RECURSION_ID_MAX to 1000 as per Jiri's reply to Paul Clark on the patch series e-mail discussion. Fixed these: tests/parse-metric.c:308:7: error: missing field 'val' initializer [-Werror,-Wmissing-field-initializers] { 0 }, ^ util/metricgroup.c:924:28: error: missing field 'parent' initializer [-Werror,-Wmissing-field-initializers] struct expr_ids ids = { 0 }; ^ util/metricgroup.c:924:26: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces] struct expr_ids ids = { 0 }; ^ {} util/metricgroup.c:924:26: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces] struct expr_ids ids = { 0 }; ^ {} util/metricgroup.c:924:28: error: missing field 'cnt' initializer [-Werror,-Wmissing-field-initializers] struct expr_ids ids = { 0 }; ^ Signed-off-by: Jiri Olsa Reviewed-by: Kajol Jain Acked-by: Ian Rogers Cc: Alexander Shishkin Cc: Andi Kleen Cc: John Garry Cc: Michael Petlan Cc: Namhyung Kim Cc: Paul Clarke Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lore.kernel.org/lkml/20200719181320.785305-16-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/parse-metric.c | 34 ++++++++++- tools/perf/util/expr.c | 2 + tools/perf/util/expr.h | 9 ++- tools/perf/util/metricgroup.c | 122 ++++++++++++++++++++++++++++++++++++---- 4 files changed, 152 insertions(+), 15 deletions(-) (limited to 'tools/perf/util/expr.h') diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c index 7a7dea833a6e..d7cc3bf64854 100644 --- a/tools/perf/tests/parse-metric.c +++ b/tools/perf/tests/parse-metric.c @@ -57,6 +57,18 @@ static struct pmu_event pme_test[] = { .metric_expr = "d_ratio(dcache_l2_all_miss, dcache_l2_all)", .metric_name = "DCache_L2_Misses", }, +{ + .metric_expr = "ipc + m2", + .metric_name = "M1", +}, +{ + .metric_expr = "ipc + m1", + .metric_name = "M2", +}, +{ + .metric_expr = "1/m3", + .metric_name = "M3", +} }; static struct pmu_events_map map = { @@ -139,8 +151,8 @@ static int compute_metric(const char *name, struct value *vals, double *ratio) err = metricgroup__parse_groups_test(evlist, &map, name, false, false, &metric_events); - - TEST_ASSERT_VAL("failed to parse metric", err == 0); + if (err) + return err; if (perf_evlist__alloc_stats(evlist, false)) return -1; @@ -264,11 +276,29 @@ static int test_dcache_l2(void) return 0; } +static int test_recursion_fail(void) +{ + double ratio; + struct value vals[] = { + { .event = "inst_retired.any", .val = 300 }, + { .event = "cpu_clk_unhalted.thread", .val = 200 }, + { .event = NULL, }, + }; + + TEST_ASSERT_VAL("failed to find recursion", + compute_metric("M1", vals, &ratio) == -1); + + TEST_ASSERT_VAL("failed to find recursion", + compute_metric("M3", vals, &ratio) == -1); + return 0; +} + int test__parse_metric(struct test *test __maybe_unused, int subtest __maybe_unused) { TEST_ASSERT_VAL("IPC failed", test_ipc() == 0); TEST_ASSERT_VAL("frontend failed", test_frontend() == 0); TEST_ASSERT_VAL("cache_miss_cycles failed", test_cache_miss_cycles() == 0); TEST_ASSERT_VAL("DCache_L2 failed", test_dcache_l2() == 0); + TEST_ASSERT_VAL("recursion fail failed", test_recursion_fail() == 0); return 0; } diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index a346ca590513..53482ef53c41 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -47,6 +47,8 @@ int expr__add_id(struct expr_parse_ctx *ctx, const char *id) if (!data_ptr) return -ENOMEM; + data_ptr->parent = ctx->parent; + ret = hashmap__set(&ctx->ids, id, data_ptr, (const void **)&old_key, (void **)&old_data); if (ret) diff --git a/tools/perf/util/expr.h b/tools/perf/util/expr.h index 9ed208d93418..fc2b5e824a66 100644 --- a/tools/perf/util/expr.h +++ b/tools/perf/util/expr.h @@ -13,8 +13,14 @@ struct metric_ref; +struct expr_id { + char *id; + struct expr_id *parent; +}; + struct expr_parse_ctx { - struct hashmap ids; + struct hashmap ids; + struct expr_id *parent; }; struct expr_id_data { @@ -25,6 +31,7 @@ struct expr_id_data { const char *metric_expr; bool counted; } ref; + struct expr_id *parent; }; bool is_ref; diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index 86665c502443..bf28d0c37da8 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -24,6 +24,7 @@ #include #include #include "util.h" +#include struct metric_event *metricgroup__lookup(struct rblist *metric_events, struct evsel *evsel, @@ -126,6 +127,28 @@ struct egroup { bool has_constraint; }; +#define RECURSION_ID_MAX 1000 + +struct expr_ids { + struct expr_id id[RECURSION_ID_MAX]; + int cnt; +}; + +static struct expr_id *expr_ids__alloc(struct expr_ids *ids) +{ + if (ids->cnt >= RECURSION_ID_MAX) + return NULL; + return &ids->id[ids->cnt++]; +} + +static void expr_ids__exit(struct expr_ids *ids) +{ + int i; + + for (i = 0; i < ids->cnt; i++) + free(ids->id[i].id); +} + /** * Find a group of events in perf_evlist that correpond to those from a parsed * metric expression. Note, as find_evsel_group is called in the same order as @@ -620,7 +643,9 @@ static int __add_metric(struct list_head *group_list, struct pmu_event *pe, bool metric_no_group, int runtime, - struct egroup **egp) + struct egroup **egp, + struct expr_id *parent, + struct expr_ids *ids) { struct metric_ref_node *ref; struct egroup *eg; @@ -630,7 +655,7 @@ static int __add_metric(struct list_head *group_list, * We got in here for the parent group, * allocate it and put it on the list. */ - eg = malloc(sizeof(*eg)); + eg = zalloc(sizeof(*eg)); if (!eg) return -ENOMEM; @@ -643,6 +668,18 @@ static int __add_metric(struct list_head *group_list, INIT_LIST_HEAD(&eg->metric_refs); eg->metric_refs_cnt = 0; *egp = eg; + + parent = expr_ids__alloc(ids); + if (!parent) { + free(eg); + return -EINVAL; + } + + parent->id = strdup(pe->metric_name); + if (!parent->id) { + free(eg); + return -ENOMEM; + } } else { /* * We got here for the referenced metric, via the @@ -668,6 +705,10 @@ static int __add_metric(struct list_head *group_list, eg->metric_refs_cnt++; } + /* Force all found IDs in metric to have us as parent ID. */ + WARN_ON_ONCE(!parent); + eg->pctx.parent = parent; + /* * For both the parent and referenced metrics, we parse * all the metric's IDs and add it to the parent context. @@ -728,15 +769,62 @@ static struct pmu_event *find_metric(const char *metric, struct pmu_events_map * return NULL; } +static int recursion_check(struct egroup *eg, const char *id, struct expr_id **parent, + struct expr_ids *ids) +{ + struct expr_id_data *data; + struct expr_id *p; + int ret; + + /* + * We get the parent referenced by 'id' argument and + * traverse through all the parent object IDs to check + * if we already processed 'id', if we did, it's recursion + * and we fail. + */ + ret = expr__get_id(&eg->pctx, id, &data); + if (ret) + return ret; + + p = data->parent; + + while (p->parent) { + if (!strcmp(p->id, id)) { + pr_err("failed: recursion detected for %s\n", id); + return -1; + } + p = p->parent; + } + + /* + * If we are over the limit of static entris, the metric + * is too difficult/nested to process, fail as well. + */ + p = expr_ids__alloc(ids); + if (!p) { + pr_err("failed: too many nested metrics\n"); + return -EINVAL; + } + + p->id = strdup(id); + p->parent = data->parent; + *parent = p; + + return p->id ? 0 : -ENOMEM; +} + static int add_metric(struct list_head *group_list, struct pmu_event *pe, bool metric_no_group, - struct egroup **egp); + struct egroup **egp, + struct expr_id *parent, + struct expr_ids *ids); static int __resolve_metric(struct egroup *eg, bool metric_no_group, struct list_head *group_list, - struct pmu_events_map *map) + struct pmu_events_map *map, + struct expr_ids *ids) { struct hashmap_entry *cur; size_t bkt; @@ -750,18 +838,23 @@ static int __resolve_metric(struct egroup *eg, do { all = true; hashmap__for_each_entry((&eg->pctx.ids), cur, bkt) { + struct expr_id *parent; struct pmu_event *pe; pe = find_metric(cur->key, map); if (!pe) continue; + ret = recursion_check(eg, cur->key, &parent, ids); + if (ret) + return ret; + all = false; /* The metric key itself needs to go out.. */ expr__del_id(&eg->pctx, cur->key); /* ... and it gets resolved to the parent context. */ - ret = add_metric(group_list, pe, metric_no_group, &eg); + ret = add_metric(group_list, pe, metric_no_group, &eg, parent, ids); if (ret) return ret; @@ -778,13 +871,14 @@ static int __resolve_metric(struct egroup *eg, static int resolve_metric(bool metric_no_group, struct list_head *metric_list, - struct pmu_events_map *map) + struct pmu_events_map *map, + struct expr_ids *ids) { struct egroup *eg; int err; list_for_each_entry(eg, metric_list, nd) { - err = __resolve_metric(eg, metric_no_group, metric_list, map); + err = __resolve_metric(eg, metric_no_group, metric_list, map, ids); if (err) return err; } @@ -794,7 +888,9 @@ static int resolve_metric(bool metric_no_group, static int add_metric(struct list_head *group_list, struct pmu_event *pe, bool metric_no_group, - struct egroup **egp) + struct egroup **egp, + struct expr_id *parent, + struct expr_ids *ids) { struct egroup *orig = *egp; int ret = 0; @@ -802,7 +898,7 @@ static int add_metric(struct list_head *group_list, pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name); if (!strstr(pe->metric_expr, "?")) { - ret = __add_metric(group_list, pe, metric_no_group, 1, egp); + ret = __add_metric(group_list, pe, metric_no_group, 1, egp, parent, ids); } else { int j, count; @@ -814,7 +910,7 @@ static int add_metric(struct list_head *group_list, */ for (j = 0; j < count && !ret; j++, *egp = orig) - ret = __add_metric(group_list, pe, metric_no_group, j, egp); + ret = __add_metric(group_list, pe, metric_no_group, j, egp, parent, ids); } return ret; @@ -825,6 +921,7 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group, struct list_head *group_list, struct pmu_events_map *map) { + struct expr_ids ids = { .cnt = 0, }; struct pmu_event *pe; struct egroup *eg; LIST_HEAD(list); @@ -835,7 +932,7 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group, has_match = true; eg = NULL; - ret = add_metric(&list, pe, metric_no_group, &eg); + ret = add_metric(&list, pe, metric_no_group, &eg, NULL, &ids); if (ret) return ret; @@ -844,7 +941,7 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group, * included in the expression. */ ret = resolve_metric(metric_no_group, - &list, map); + &list, map, &ids); if (ret) return ret; } @@ -867,6 +964,7 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group, } list_splice(&list, group_list); + expr_ids__exit(&ids); return 0; } -- cgit v1.2.3