[PATCH v2 24/30] perf stat: Share events between metrics and command line events

[Ian Rogers <irogers@google.com>]

The metrics added by -T, --smi-cost, --topdown and the Default metric
groups were parsed into a separate evlist and then added to the events
from the command line, so they didn't share events with them or, for
the Default metric groups, with each other.

Parse them directly into evsel_list so that the evsel matcher of
metricgroup__parse_groups() shares events counting the same thing. Only
mark the newly added events as skippable and keep the evsel indices in
list order after sorting the Default metric events, as required when
writing the group descriptions of a perf.data file. This removes the
copying of the metric events from the separate evlist with
metricgroup__copy_metric_events(), which crashed for "perf stat -T" on
an Alder Lake system.

For example, "perf stat -e duration_time -T" now uses the duration_time
event from the command line for the transaction metrics.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-stat.c | 72 +++++++++++++++++++++++----------------
 1 file changed, 43 insertions(+), 29 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f81192cec481..2b563382cf43 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1975,6 +1975,15 @@ static int default_evlist_evsel_cmp(void *priv __maybe_unused,
 		      evsel__name((struct evsel *)rhs_leader));
 }
 
+static bool evsel_in_array(const struct evsel *evsel, struct evsel * const *evsels, int nr)
+{
+	for (int i = 0; i < nr; i++) {
+		if (evsels[i] == evsel)
+			return true;
+	}
+	return false;
+}
+
 /*
  * Add default events, if there were no attributes specified or
  * if -d/--detailed, -d -d or -d -d -d is used:
@@ -1984,18 +1993,31 @@ static int add_default_events(void)
 	const char *pmu = parse_events_option_args.pmu_filter ?: "all";
 	struct perf_pmus *pmus = perf_env__pmus(host_env());
 	struct parse_events_error err;
-	struct evlist *evlist = evlist__new();
+	/*
+	 * Add the metrics directly to evsel_list so that they share events
+	 * with the events and metrics already there.
+	 */
+	struct evlist *evlist = evsel_list;
+	int nr_orig_entries = evlist__nr_entries(evlist);
+	struct evsel **orig_evsels = NULL;
 	struct evsel *evsel;
-	int ret = 0;
+	int idx = 0, ret = 0;
 
-	if (!evlist)
-		return -ENOMEM;
+	/* Set attrs if no event is selected and !null_run: */
+	if (stat_config.null_run)
+		return 0;
 
 	parse_events_error__init(&err);
 
-	/* Set attrs if no event is selected and !null_run: */
-	if (stat_config.null_run)
-		goto out;
+	if (nr_orig_entries > 0) {
+		orig_evsels = calloc(nr_orig_entries, sizeof(*orig_evsels));
+		if (!orig_evsels) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		evlist__for_each_entry(evlist, evsel)
+			orig_evsels[idx++] = evsel;
+	}
 
 	if (transaction_run) {
 		/* Handle -T as -M transaction. Once platform specific metrics
@@ -2101,7 +2123,7 @@ static int add_default_events(void)
 	if (!stat_config.topdown_level)
 		stat_config.topdown_level = 1;
 
-	if (!evlist__nr_entries(evlist) && !evlist__nr_entries(evsel_list)) {
+	if (!evlist__nr_entries(evlist)) {
 		/*
 		 * Add Default metrics. To minimize multiplexing, don't request
 		 * threshold computation, but it will be computed if the events
@@ -2112,8 +2134,6 @@ static int add_default_events(void)
 		};
 
 		for (size_t i = 0; i < ARRAY_SIZE(default_metricgroup_names); i++) {
-			struct evlist *metric_evlist;
-
 			if (!metricgroup__has_metric_or_groups(pmus, pmu,
 							       default_metricgroup_names[i]))
 				continue;
@@ -2121,12 +2141,7 @@ static int add_default_events(void)
 			if ((int)i > detailed_run)
 				break;
 
-			metric_evlist = evlist__new();
-			if (!metric_evlist) {
-				ret = -ENOMEM;
-				break;
-			}
-			if (metricgroup__parse_groups(metric_evlist, pmu,
+			if (metricgroup__parse_groups(evlist, pmu,
 						      parse_events_option_args.cputype_filter,
 						      default_metricgroup_names[i],
 							/*metric_no_group=*/false,
@@ -2135,26 +2150,29 @@ static int add_default_events(void)
 							stat_config.user_requested_cpu_list,
 							stat_config.system_wide,
 							stat_config.hardware_aware_grouping) < 0) {
-				evlist__put(metric_evlist);
 				ret = -1;
 				break;
 			}
 
-			evlist__for_each_entry(metric_evlist, evsel)
+			/* evsel_list was empty so all the events are for the Default metrics. */
+			evlist__for_each_entry(evlist, evsel)
 				evsel->default_metricgroup = true;
-
-			evlist__splice_list_tail(evlist, &evlist__core(metric_evlist)->entries);
-			metricgroup__copy_metric_events(evlist, /*cgrp=*/NULL,
-							evlist__metric_events(evlist),
-							evlist__metric_events(metric_evlist));
-			evlist__put(metric_evlist);
 		}
 		list_sort(/*priv=*/NULL, &evlist__core(evlist)->entries, default_evlist_evsel_cmp);
+		/* Keep the indices in list order, as required by the perf.data header. */
+		idx = 0;
+		evlist__for_each_entry(evlist, evsel) {
+			evsel->core.idx = idx++;
+			evsel->tracking = !evsel->core.idx;
+		}
 
 	}
 out:
 	if (!ret) {
 		evlist__for_each_entry(evlist, evsel) {
+			/* Only events added here, not those from the command line. */
+			if (evsel_in_array(evsel, orig_evsels, nr_orig_entries))
+				continue;
 			/*
 			 * Make at least one event non-skippable so fatal errors are visible.
 			 * 'cycles' always used to be default and non-skippable, so use that.
@@ -2163,12 +2181,8 @@ static int add_default_events(void)
 				evsel->skippable = true;
 		}
 	}
+	free(orig_evsels);
 	parse_events_error__exit(&err);
-	evlist__splice_list_tail(evsel_list, &evlist__core(evlist)->entries);
-	metricgroup__copy_metric_events(evsel_list, /*cgrp=*/NULL,
-					evlist__metric_events(evsel_list),
-					evlist__metric_events(evlist));
-	evlist__put(evlist);
 	return ret;
 }
 
-- 
2.56.1.703.gf848ed807c-goog