[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP-5=fUTHPu_vaNyeaq9uDwf7Hq79khFZymFJK2w39k-9RdutQ@mail.gmail.com>
Date: Tue, 16 Apr 2024 21:56:38 -0700
From: Ian Rogers <irogers@...gle.com>
To: weilin.wang@...el.com
Cc: Kan Liang <kan.liang@...ux.intel.com>, Namhyung Kim <namhyung@...nel.org>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, Perry Taylor <perry.taylor@...el.com>,
Samantha Alt <samantha.alt@...el.com>, Caleb Biggers <caleb.biggers@...el.com>
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@...el.com> wrote:
>
> From: Weilin Wang <weilin.wang@...el.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@...el.com>
Reviewed-by: Ian Rogers <irogers@...gle.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
>
Powered by blists - more mailing lists