Messages in this thread | | | From | Ian Rogers <> | Date | Tue, 16 Apr 2024 21:56:38 -0700 | Subject | Re: [RFC PATCH v5 02/16] perf stat: Add basic functions for the hardware aware grouping |
| |
On Fri, Apr 12, 2024 at 2:08 PM <weilin.wang@intel.com> wrote: > > From: Weilin Wang <weilin.wang@intel.com> > > Add the first set of functions for the hardware aware grouping method. Function > hw_aware_parse_groups() is the entry point of this metric grouping method It > does metric grouping on a combined list of events and will create a list of > grouping strings as final results of the grouping method. These grouping strings > will be used in the same manner as existing metric grouping process. > > This method will fall back to normal grouping when hardware aware grouping > return with err so that perf stat still executes and returns with correct > result. > > Signed-off-by: Weilin Wang <weilin.wang@intel.com>
Reviewed-by: Ian Rogers <irogers@google.com>
> --- > tools/perf/util/metricgroup.c | 217 +++++++++++++++++++++++++++++++++- > 1 file changed, 216 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c > index 11613450725a..8047f03b2b1f 100644 > --- a/tools/perf/util/metricgroup.c > +++ b/tools/perf/util/metricgroup.c > @@ -159,6 +159,14 @@ struct metric { > struct evlist *evlist; > }; > > +/** > + * Each group is one node in the group string list. > + */ > +struct metricgroup__group_strs { > + struct list_head nd; > + struct strbuf grouping_str; > +}; > + > static void metric__watchdog_constraint_hint(const char *name, bool foot) > { > static bool violate_nmi_constraint; > @@ -1432,6 +1440,101 @@ static int build_combined_expr_ctx(const struct list_head *metric_list, > return ret; > } > > +/** > + * hw_aware_build_grouping - Build event groupings by reading counter > + * requirement of the events and counter available on the system from > + * pmu-events. > + * @ctx: the event identifiers parsed from metrics. > + * @groupings: header to the list of final event grouping. > + * @modifier: any modifiers added to the events. > + */ > +static int hw_aware_build_grouping(struct expr_parse_ctx *ctx __maybe_unused, > + struct list_head *groupings __maybe_unused, > + const char *modifier __maybe_unused) > +{ > + int ret = 0; > + > + pr_debug("This is a placeholder\n"); > + return ret; > +} > + > +static void group_str_free(struct metricgroup__group_strs *g) > +{ > + if (!g) > + return; > + > + strbuf_release(&g->grouping_str); > + free(g); > +} > + > +static void metricgroup__free_grouping_strs(struct list_head > + *grouping_strs) > +{ > + struct metricgroup__group_strs *g, *tmp; > + > + list_for_each_entry_safe(g, tmp, grouping_strs, nd) { > + list_del_init(&g->nd); > + group_str_free(g); > + } > +} > + > +/** > + * hw_aware_parse_ids - Build the event string for the ids and parse them > + * creating an evlist. The encoded metric_ids are decoded. Events are placed > + * into groups based on event counter requirements and counter availabilities of > + * the system. > + * @metric_no_merge: is metric sharing explicitly disabled. > + * @fake_pmu: used when testing metrics not supported by the current CPU. > + * @ids: the event identifiers parsed from a metric. > + * @modifier: any modifiers added to the events. > + * @out_evlist: the created list of events. > + */ > +static int hw_aware_parse_ids(struct perf_pmu *fake_pmu, > + struct expr_parse_ctx *ids, const char *modifier, > + struct evlist **out_evlist) > +{ > + struct parse_events_error parse_error; > + struct evlist *parsed_evlist; > + LIST_HEAD(groupings); > + struct metricgroup__group_strs *group; > + int ret; > + > + *out_evlist = NULL; > + ret = hw_aware_build_grouping(ids, &groupings, modifier); > + if (ret) { > + metricgroup__free_grouping_strs(&groupings); > + return ret; > + } > + > + parsed_evlist = evlist__new(); > + if (!parsed_evlist) { > + ret = -ENOMEM; > + goto err_out; > + } > + list_for_each_entry(group, &groupings, nd) { > + struct strbuf *events = &group->grouping_str; > + > + pr_debug("Parsing metric events '%s'\n", events->buf); > + parse_events_error__init(&parse_error); > + ret = __parse_events(parsed_evlist, events->buf, /*pmu_filter=*/NULL, > + &parse_error, fake_pmu, /*warn_if_reordered=*/false); > + if (ret) { > + parse_events_error__print(&parse_error, events->buf); > + goto err_out; > + } > + ret = decode_all_metric_ids(parsed_evlist, modifier); > + if (ret) > + goto err_out; > + } > + *out_evlist = parsed_evlist; > + parsed_evlist = NULL; > +err_out: > + parse_events_error__exit(&parse_error); > + evlist__delete(parsed_evlist); > + metricgroup__free_grouping_strs(&groupings); > + return ret; > +} > + > /** > * parse_ids - Build the event string for the ids and parse them creating an > * evlist. The encoded metric_ids are decoded. > @@ -1520,6 +1623,113 @@ static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu, > return ret; > } > > +static int hw_aware_parse_groups(struct evlist *perf_evlist, > + const char *pmu, const char *str, > + bool metric_no_threshold, > + const char *user_requested_cpu_list, > + bool system_wide, > + struct perf_pmu *fake_pmu, > + struct rblist *metric_events_list, > + const struct pmu_metrics_table *table) > +{ > + struct evlist *combined_evlist = NULL; > + LIST_HEAD(metric_list); > + struct metric *m; > + int ret; > + bool metric_no_group = false; > + bool metric_no_merge = false; > + > + if (metric_events_list->nr_entries == 0) > + metricgroup__rblist_init(metric_events_list); > + ret = metricgroup__add_metric_list(pmu, str, metric_no_group, metric_no_threshold, > + user_requested_cpu_list, > + system_wide, &metric_list, table); > + if (ret) > + goto out; > + > + /* Sort metrics from largest to smallest. */ > + list_sort(NULL, &metric_list, metric_list_cmp); > + > + if (!metric_no_merge) { > + struct expr_parse_ctx *combined = NULL; > + > + ret = build_combined_expr_ctx(&metric_list, &combined); > + > + if (!ret && combined && hashmap__size(combined->ids)) { > + ret = hw_aware_parse_ids(fake_pmu, combined, > + /*modifier=*/NULL, > + &combined_evlist); > + } > + > + if (combined) > + expr__ctx_free(combined); > + if (ret) > + goto out; > + } > + > + list_for_each_entry(m, &metric_list, nd) { > + struct metric_expr *expr; > + struct metric_event *me; > + struct evsel **metric_events; > + > + ret = setup_metric_events(fake_pmu ? "all" : m->pmu, m->pctx->ids, > + combined_evlist, &metric_events); > + if (ret) { > + pr_debug("Cannot resolve IDs for %s: %s\n", > + m->metric_name, m->metric_expr); > + goto out; > + } > + > + me = metricgroup__lookup(metric_events_list, metric_events[0], true); > + > + expr = malloc(sizeof(struct metric_expr)); > + if (!expr) { > + ret = -ENOMEM; > + free(metric_events); > + goto out; > + } > + > + expr->metric_refs = m->metric_refs; > + m->metric_refs = NULL; > + expr->metric_expr = m->metric_expr; > + if (m->modifier) { > + char *tmp; > + > + if (asprintf(&tmp, "%s:%s", m->metric_name, m->modifier) < 0) > + expr->metric_name = NULL; > + else > + expr->metric_name = tmp; > + } else { > + expr->metric_name = strdup(m->metric_name); > + } > + > + if (!expr->metric_name) { > + ret = -ENOMEM; > + free(metric_events); > + goto out; > + } > + expr->metric_threshold = m->metric_threshold; > + expr->metric_unit = m->metric_unit; > + expr->metric_events = metric_events; > + expr->runtime = m->pctx->sctx.runtime; > + list_add(&expr->nd, &me->head); > + } > + > + if (combined_evlist) { > + evlist__splice_list_tail(perf_evlist, &combined_evlist->core.entries); > + evlist__delete(combined_evlist); > + } > + > + list_for_each_entry(m, &metric_list, nd) { > + if (m->evlist) > + evlist__splice_list_tail(perf_evlist, &m->evlist->core.entries); > + } > + > +out: > + metricgroup__free_metrics(&metric_list); > + return ret; > +} > + > static int parse_groups(struct evlist *perf_evlist, > const char *pmu, const char *str, > bool metric_no_group, > @@ -1698,10 +1908,15 @@ int metricgroup__parse_groups(struct evlist *perf_evlist, > if (!table) > return -EINVAL; > if (hardware_aware_grouping) { > + int ret;
nit: there should be a '\n' between variables and code, scripts/checkpatch.pl sould catch this.
> pr_debug("Use hardware aware grouping instead of traditional metric grouping method\n"); > + ret = hw_aware_parse_groups(perf_evlist, pmu, str, > + metric_no_threshold, user_requested_cpu_list, system_wide, > + /*fake_pmu=*/NULL, metric_events, table); > + if (!ret) > + return 0; > } > > -
nit: unneeded whitespace change.
Thanks, Ian
> return parse_groups(perf_evlist, pmu, str, metric_no_group, metric_no_merge, > metric_no_threshold, user_requested_cpu_list, system_wide, > /*fake_pmu=*/NULL, metric_events, table); > -- > 2.42.0 >
| |