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/util/metricgroup.c | 44 +++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) (limited to 'tools/perf/util/metricgroup.c') diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c index 29b747ac31c1..b7924a2f1f45 100644 --- a/tools/perf/util/metricgroup.c +++ b/tools/perf/util/metricgroup.c @@ -118,7 +118,7 @@ struct metric_ref_node { struct metric { struct list_head nd; - struct expr_parse_ctx pctx; + struct expr_parse_ctx *pctx; const char *metric_name; const char *metric_expr; const char *metric_unit; @@ -198,7 +198,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist, struct evsel *ev, *current_leader = NULL; struct expr_id_data *val_ptr; int i = 0, matched_events = 0, events_to_match; - const int idnum = (int)hashmap__size(&pctx->ids); + const int idnum = (int)hashmap__size(pctx->ids); /* * duration_time is always grouped separately, when events are grouped @@ -206,7 +206,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist, * add it to metric_events at the end. */ if (!has_constraint && - hashmap__find(&pctx->ids, "duration_time", (void **)&val_ptr)) + hashmap__find(pctx->ids, "duration_time", (void **)&val_ptr)) events_to_match = idnum - 1; else events_to_match = idnum; @@ -242,7 +242,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist, if (contains_event(metric_events, matched_events, ev->name)) continue; /* Does this event belong to the parse context? */ - if (hashmap__find(&pctx->ids, ev->name, (void **)&val_ptr)) + if (hashmap__find(pctx->ids, ev->name, (void **)&val_ptr)) metric_events[matched_events++] = ev; if (matched_events == events_to_match) @@ -322,12 +322,12 @@ static int metricgroup__setup_events(struct list_head *groups, struct metric_ref *metric_refs = NULL; metric_events = calloc(sizeof(void *), - hashmap__size(&m->pctx.ids) + 1); + hashmap__size(m->pctx->ids) + 1); if (!metric_events) { ret = -ENOMEM; break; } - evsel = find_evsel_group(perf_evlist, &m->pctx, + evsel = find_evsel_group(perf_evlist, m->pctx, metric_no_merge, m->has_constraint, metric_events, evlist_used); @@ -693,7 +693,7 @@ static void metricgroup__add_metric_weak_group(struct strbuf *events, size_t bkt; bool no_group = true, has_duration = false; - hashmap__for_each_entry((&ctx->ids), cur, bkt) { + hashmap__for_each_entry(ctx->ids, cur, bkt) { pr_debug("found event %s\n", (const char *)cur->key); /* * Duration time maps to a software event and can make @@ -724,7 +724,7 @@ static void metricgroup__add_metric_non_group(struct strbuf *events, size_t bkt; bool first = true; - hashmap__for_each_entry((&ctx->ids), cur, bkt) { + hashmap__for_each_entry(ctx->ids, cur, bkt) { if (!first) strbuf_addf(events, ","); strbuf_addf(events, "%s", (const char *)cur->key); @@ -799,7 +799,11 @@ static int __add_metric(struct list_head *metric_list, if (!m) return -ENOMEM; - expr__ctx_init(&m->pctx); + m->pctx = expr__ctx_new(); + if (!m->pctx) { + free(m); + return -ENOMEM; + } m->metric_name = pe->metric_name; m->metric_expr = pe->metric_expr; m->metric_unit = pe->unit; @@ -847,15 +851,15 @@ static int __add_metric(struct list_head *metric_list, /* Force all found IDs in metric to have us as parent ID. */ WARN_ON_ONCE(!parent); - m->pctx.parent = parent; + m->pctx->parent = parent; /* * For both the parent and referenced metrics, we parse * all the metric's IDs and add it to the parent context. */ - if (expr__find_other(pe->metric_expr, NULL, &m->pctx, runtime) < 0) { + if (expr__find_other(pe->metric_expr, NULL, m->pctx, runtime) < 0) { if (m->metric_refs_cnt == 0) { - expr__ctx_clear(&m->pctx); + expr__ctx_free(m->pctx); free(m); *mp = NULL; } @@ -878,8 +882,8 @@ static int __add_metric(struct list_head *metric_list, list_for_each_prev(pos, metric_list) { struct metric *old = list_entry(pos, struct metric, nd); - if (hashmap__size(&m->pctx.ids) <= - hashmap__size(&old->pctx.ids)) + if (hashmap__size(m->pctx->ids) <= + hashmap__size(old->pctx->ids)) break; } list_add(&m->nd, pos); @@ -927,7 +931,7 @@ static int recursion_check(struct metric *m, const char *id, struct expr_id **pa * if we already processed 'id', if we did, it's recursion * and we fail. */ - ret = expr__get_id(&m->pctx, id, &data); + ret = expr__get_id(m->pctx, id, &data); if (ret) return ret; @@ -982,7 +986,7 @@ static int __resolve_metric(struct metric *m, */ do { all = true; - hashmap__for_each_entry((&m->pctx.ids), cur, bkt) { + hashmap__for_each_entry(m->pctx->ids, cur, bkt) { struct expr_id *parent; struct pmu_event *pe; @@ -996,7 +1000,7 @@ static int __resolve_metric(struct metric *m, all = false; /* The metric key itself needs to go out.. */ - expr__del_id(&m->pctx, cur->key); + expr__del_id(m->pctx, cur->key); /* ... and it gets resolved to the parent context. */ ret = add_metric(metric_list, pe, metric_no_group, &m, parent, ids); @@ -1144,10 +1148,10 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group, if (m->has_constraint) { metricgroup__add_metric_non_group(events, - &m->pctx); + m->pctx); } else { metricgroup__add_metric_weak_group(events, - &m->pctx); + m->pctx); } } @@ -1210,7 +1214,7 @@ static void metricgroup__free_metrics(struct list_head *metric_list) list_for_each_entry_safe (m, tmp, metric_list, nd) { metric__free_refs(m); - expr__ctx_clear(&m->pctx); + expr__ctx_free(m->pctx); list_del_init(&m->nd); free(m); } -- cgit v1.2.3