[PATCH v2 7/9] perf pmu: Merge PMU instances that match the same comma-separated json Unit

[Besar Wicaksono <bwicaksono@nvidia.com>]

Since commit 730670b1d108 ("perf pmu: Support more complex PMU event
aliasing") a json event "Unit" with comma-separated parts matches PMU
names with an optional number after each part. For example, the NVIDIA
Tegra410 PCIe PMUs are named nvidia_pcie_pmu_<socket>_rc_<rc>, and the
Unit "nvidia_pcie_pmu_,rc_" of their json events matches all of them.
perf uses this match to attach json events and metrics to these PMUs.
But perf uses a different match to decide which PMUs to combine in the
counting result, and it only ignores the number at the end of the name.
So the RCs of the same socket are combined, but the two sockets are not:
nvidia_pcie_pmu_0_rc_<rc> and nvidia_pcie_pmu_1_rc_<rc> give two
results.

On a dual-socket system, this gives one line per socket under the same
name in the default output, and with --metric-only a metric column per
socket, a "none" value for the other socket and a duplicate metric key:

  $ perf stat -a --per-socket -j --metric-only -M pcie_avg_wr_bandwidth_to_all_dst -- sleep 1
  {"socket" : "S268435456", "counters" : 6, "MB/s  pcie_avg_wr_bandwidth_to_all_dst" : "0.0", "MB/s  pcie_avg_wr_bandwidth_to_all_dst" : "none"}
  {"socket" : "S285212672", "counters" : 0, "MB/s  pcie_avg_wr_bandwidth_to_all_dst" : "none", "MB/s  pcie_avg_wr_bandwidth_to_all_dst" : "0.0"}

Keep the existing match, which ignores the number at the end of the
name, and add a match for uncore PMUs with a comma-separated Unit: two
such PMUs are instances of one PMU if both match the Unit of the first
PMU's json events.

After:

  $ perf stat -a --per-socket -j --metric-only -M pcie_avg_wr_bandwidth_to_all_dst -- sleep 1
  {"socket" : "S268435456", "counters" : 6, "MB/s  pcie_avg_wr_bandwidth_to_all_dst" : "0.0"}
  {"socket" : "S285212672", "counters" : 6, "MB/s  pcie_avg_wr_bandwidth_to_all_dst" : "0.0"}

Signed-off-by: Besar Wicaksono <bwicaksono@nvidia.com>
---
 tools/perf/tests/pmu.c         | 72 ++++++++++++++++++++++++++++++++++
 tools/perf/util/parse-events.c |  2 +-
 tools/perf/util/pmu.c          | 65 ++++++++++++++++++++++++++++++
 tools/perf/util/pmu.h          |  9 +++++
 4 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 13e8d7fa80af..e722cd08e30a 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -629,6 +629,77 @@ static int test__pmu_match(struct test_suite *test __maybe_unused, int subtest _
 	return TEST_OK;
 }
 
+static int test__pmu_instance_match(struct test_suite *test __maybe_unused,
+				    int subtest __maybe_unused)
+{
+/*
+ * Check that perf_pmu__is_instance_of() returns expect for uncore PMU _name,
+ * whose json events have the comma-separated Unit _unit (NULL if none), and
+ * uncore PMU _other.
+ */
+#define TEST_INSTANCE(msg, _name, _unit, _other, expect)			\
+	TEST_ASSERT_EQUAL(msg, perf_pmu__is_instance_of(			\
+		&(struct perf_pmu){ .name = _name, .is_uncore = true,		\
+				    .json_unit = _unit,				\
+				    .json_unit_checked = true },		\
+		&(struct perf_pmu){ .name = _other, .is_uncore = true,		\
+				    .json_unit_checked = true }),		\
+		expect)
+
+	/* No comma-separated Unit: only the suffix number is ignored. */
+	TEST_INSTANCE("Same PMU type: same name",
+		      "nvidia_ucf_pmu_0", NULL, "nvidia_ucf_pmu_0", true);
+	TEST_INSTANCE("Same PMU type: differ only in suffix number",
+		      "nvidia_ucf_pmu_0", NULL, "nvidia_ucf_pmu_1", true);
+	TEST_INSTANCE("Same PMU type: differ only in suffix number, with a number inside the name",
+		      "nvidia_nvlink_c2c_pmu_0", NULL, "nvidia_nvlink_c2c_pmu_1", true);
+	TEST_INSTANCE("Same PMU type: differ only in suffix number, with a number in the middle",
+		      "uncore_type_0_1", NULL, "uncore_type_0_2", true);
+	TEST_INSTANCE("Same PMU type: differ only in hex suffix digits",
+		      "smmuv3_pmcg_fe0004030", NULL, "smmuv3_pmcg_fe0004620", true);
+	TEST_INSTANCE("Same PMU type: differ only in hex suffix, with letters",
+		      "smmuv3_pmcg_fe0004030", NULL, "smmuv3_pmcg_fe00053a0", true);
+	TEST_INSTANCE("Different PMU types: differ before suffix number",
+		      "nvidia_ucf_pmu_0", NULL, "nvidia_nvlink_c2c_pmu_0", false);
+	TEST_INSTANCE("Different PMU types: differ in the middle number",
+		      "uncore_type_0_1", NULL, "uncore_type_1_1", false);
+	TEST_INSTANCE("Different PMU types: differ before hex suffix",
+		      "smmuv3_pmcg_fe0004030", NULL, "arm_dmc620_fe0004030", false);
+
+	/* Comma-separated Unit: an optional number after each part. */
+	TEST_INSTANCE("Same PMU type: both match comma Unit, numbers after _",
+		      "nvidia_pcie_pmu_0_rc_0", "nvidia_pcie_pmu_,rc_",
+		      "nvidia_pcie_pmu_1_rc_5", true);
+	TEST_INSTANCE("Same PMU type: both match comma Unit, numbers after _, reversed",
+		      "nvidia_pcie_pmu_1_rc_5", "nvidia_pcie_pmu_,rc_",
+		      "nvidia_pcie_pmu_0_rc_0", true);
+	TEST_INSTANCE("Same PMU type: both match comma Unit, numbers without _",
+		      "hisi_sccl1_ddrc0", "hisi_sccl,ddrc", "hisi_sccl3_ddrc2", true);
+	TEST_INSTANCE("Same PMU type: comma Unit, part without number",
+		      "hisi_sccl1_ddrc", "hisi_sccl,ddrc", "hisi_sccl3_ddrc", true);
+	TEST_INSTANCE("Same PMU type: comma Unit, one vs two numbers",
+		      "hisi_sccl1_ddrc", "hisi_sccl,ddrc", "hisi_sccl3_ddrc2", true);
+
+	/* Comma-separated Unit that doesn't match the other PMU. */
+	TEST_INSTANCE("Different PMU types: comma Unit of other type",
+		      "nvidia_pcie_pmu_0_rc_0", "nvidia_pcie_pmu_,rc_",
+		      "nvidia_pcie_tgt_pmu_1_rc_0", false);
+	TEST_INSTANCE("Different PMU types: comma Unit of other type, reversed",
+		      "nvidia_pcie_tgt_pmu_1_rc_0", "nvidia_pcie_tgt_pmu_,rc_",
+		      "nvidia_pcie_pmu_1_rc_5", false);
+	TEST_INSTANCE("Different PMU types: comma Unit part rc_ missing",
+		      "nvidia_pcie_pmu_0_rc_0", "nvidia_pcie_pmu_,rc_",
+		      "nvidia_pcie_pmu_0", false);
+	TEST_INSTANCE("Different PMU types: comma Unit part rc_ missing, reversed",
+		      "nvidia_pcie_pmu_0", NULL, "nvidia_pcie_pmu_0_rc_0", false);
+	TEST_INSTANCE("Different PMU types: comma Unit vs suffix-only name",
+		      "nvidia_pcie_pmu_0_rc_0", "nvidia_pcie_pmu_,rc_",
+		      "nvidia_ucf_pmu_0", false);
+
+#undef TEST_INSTANCE
+	return TEST_OK;
+}
+
 static struct test_case tests__pmu[] = {
 	TEST_CASE("Parsing with PMU format directory", pmu_format),
 	TEST_CASE("Parsing with PMU event", pmu_events),
@@ -636,6 +707,7 @@ static struct test_case tests__pmu[] = {
 	TEST_CASE("PMU name combining", name_len),
 	TEST_CASE("PMU name comparison", name_cmp),
 	TEST_CASE("PMU cmdline match", pmu_match),
+	TEST_CASE("PMU instance match", pmu_instance_match),
 	TEST_CASE("PMU config helpers", pmu_config_helpers),
 	{	.name = NULL, }
 };
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index f3ba70d4f76b..9455a7679468 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -193,7 +193,7 @@ __add_event(struct list_head *list, int *idx,
 
 		first_wildcard_match = NULL;
 		list_for_each_entry_continue(pos, list, core.node) {
-			if (perf_pmu__name_no_suffix_match(pos->pmu, pmu->name)) {
+			if (perf_pmu__is_instance_of(pos->pmu, pmu)) {
 				first_wildcard_match = pos;
 				break;
 			}
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 01753d5c69d1..df2abdf23697 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1120,6 +1120,13 @@ static int pmu_add_sys_aliases_iter_fn(const struct pmu_event *pe,
 				/*val_fd=*/ -1,
 				pe,
 				EVENT_SRC_SYS_JSON);
+
+		/*
+		 * Sys json events are not in pmu->events_table, so store their
+		 * comma-separated Unit here for perf_pmu__json_unit().
+		 */
+		if (!pmu->json_unit && strchr(pe->pmu, ','))
+			pmu->json_unit = pe->pmu;
 	}
 	return 0;
 }
@@ -2435,6 +2442,64 @@ bool perf_pmu__name_no_suffix_match(const struct perf_pmu *pmu, const char *to_m
 	return perf_pmu___name_match(pmu, to_match, /*wildcard=*/false);
 }
 
+/* Look for a json Unit with comma-separated parts among the PMU's events. */
+static int pmu_json_unit_callback(const struct pmu_event *pe,
+				  const struct pmu_events_table *table __maybe_unused,
+				  void *vdata)
+{
+	const char **unit = vdata;
+
+	if (!pe->pmu || !strchr(pe->pmu, ','))
+		return 0;
+
+	*unit = pe->pmu;
+	return 1;
+}
+
+/*
+ * The json Unit with comma-separated parts matching the PMU, if any. Such a
+ * Unit, like "nvidia_pcie_pmu_,rc_", matches PMU names with an optional
+ * number after each part, like "nvidia_pcie_pmu_1_rc_3".
+ */
+static const char *perf_pmu__json_unit(struct perf_pmu *pmu)
+{
+	if (!pmu->json_unit_checked) {
+		pmu->json_unit_checked = true;
+		/* Not found in the sys json events: look in the CPU json events. */
+		if (!pmu->json_unit)
+			pmu_events_table__for_each_event(pmu->events_table, pmu,
+							 pmu_json_unit_callback,
+							 &pmu->json_unit);
+	}
+	return pmu->json_unit;
+}
+
+/**
+ * perf_pmu__is_instance_of - Check whether pmu and other are instances of
+ *                            the same PMU: either their names match
+ *                            ignoring any suffix, or both are uncore PMUs
+ *                            matching the json Unit of pmu with
+ *                            comma-separated parts.
+ * @pmu: The PMU whose name and json Unit to match.
+ * @other: The possible other instance.
+ *
+ * Only the json Unit of pmu is used. Instances of one PMU have the same
+ * json events, so the result doesn't depend on the order of the PMUs.
+ */
+bool perf_pmu__is_instance_of(struct perf_pmu *pmu, struct perf_pmu *other)
+{
+	const char *unit;
+
+	if (perf_pmu__name_no_suffix_match(pmu, other->name))
+		return true;
+
+	if (!pmu->is_uncore || !other->is_uncore)
+		return false;
+
+	unit = perf_pmu__json_unit(pmu);
+	return unit && perf_pmu__name_wildcard_match(other, unit);
+}
+
 bool perf_pmu__is_software(const struct perf_pmu *pmu)
 {
 	const char *known_sw_pmus[] = {
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 2c8916ad073e..5263d97b8f20 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -148,6 +148,12 @@ struct perf_pmu {
 	 * @events_table: The events table for json events in pmu-events.c.
 	 */
 	const struct pmu_events_table *events_table;
+	/**
+	 * @json_unit: The json Unit with comma-separated parts of the events
+	 * matching the PMU, NULL if none. Units without a comma are not
+	 * stored. Found by perf_pmu__is_instance_of().
+	 */
+	const char *json_unit;
 	/** @sysfs_aliases: Number of sysfs aliases loaded. */
 	uint32_t sysfs_aliases;
 	/** @cpu_json_aliases: Number of json event aliases loaded specific to the CPUID. */
@@ -166,6 +172,8 @@ struct perf_pmu {
 	 * been added?
 	 */
 	bool cpu_aliases_added;
+	/** @json_unit_checked: Has json_unit been looked up? */
+	bool json_unit_checked;
 	/** @caps_initialized: Has the list caps been initialized? */
 	bool caps_initialized;
 	/** @nr_caps: The length of the list caps. */
@@ -300,6 +308,7 @@ int perf_pmu__for_each_event(struct perf_pmu *pmu, bool skip_duplicate_pmus,
 			     void *state, pmu_event_callback cb);
 bool perf_pmu__name_wildcard_match(const struct perf_pmu *pmu, const char *to_match);
 bool perf_pmu__name_no_suffix_match(const struct perf_pmu *pmu, const char *to_match);
+bool perf_pmu__is_instance_of(struct perf_pmu *pmu, struct perf_pmu *other);
 
 /**
  * perf_pmu_is_software - is the PMU a software PMU as in it uses the
-- 
2.43.0