[PATCH v6 00/10] perf pmu-events: Support event aliasing for system PMUs

[PATCH v6 00/10] perf pmu-events: Support event aliasing for system PMUs

[John Garry]

Currently event aliasing and metrics is supported for only CPU and uncore
PMUs. More specifically, only uncore PMUs aliasing is supported for when
the uncore PMUs are fixed for a CPU, which may not always be the case for
certain architectures.

This series adds support for PMU event aliasing and metrics for system and
other uncore PMUs which are not tied to a specific CPU.

For this, we introduce system event tables in generated pmu-events.c,
which contain a per-SoC table of events of all its system PMUs. Each
per-PMU event is matched by a "COMPAT" property.

When creating aliases and metrics for PMUs, we treat core/uncore and
system PMUs differently:

- For CPU PMUs, we always match for the event mapfile based on the CPUID.
  This has not changed.

- For an system PMUs, we iterate through all the events in all the system
  PMU tables.

  Matches are based on the "COMPAT" property matching the PMU sysfs
  identifier contents, in /sys/bus/event_source/devices/<PMU>/identifier

  Uncore PMUs, may be matched via CPUID or same as system PMU, depending
  on whether the uncore PMU is tied to a specific CPUID.

Initial reference support is also added for for imx8mm DDR PMU, and it
looks like this for the user:

$ ./perf list metric

List of pre-defined events (to be used in -e):
Metric Groups:

No_group:
  imx8mm_ddr_read.all
       [bytes all masters read from ddr based on read-cycles event. Unit: imx8_ddr ]
  imx8mm_ddr_write.all                                                            
       [bytes all masters write to ddr based on write-cycles event. Unit: imx8_ddr ]

$ sudo ./perf stat -v -M imx8mm_ddr_read.all sleep 1
metric expr imx8mm_ddr.read_cycles * 4 * 4 for imx8mm_ddr_read.all
found event imx8mm_ddr.read_cycles
adding {imx8mm_ddr.read_cycles}:W
imx8mm_ddr.read_cycles -> imx8_ddr0/event=0x2a/
imx8mm_ddr.read_cycles -> imx8_ddr1/event=0x2a/
Control descriptor is not initialized
imx8mm_ddr.read_cycles: 2 1001455480 1001455480
imx8mm_ddr.read_cycles: 3 1001454940 1001454940

Performance counter stats for 'system wide':

	5	imx8mm_ddr.read_cycles	# 0.1 KB  imx8mm_ddr_read.all

		1.001493170 seconds time elapsed

I have not included HiSilicon hip09 events from earlier RFC since it is a
new platform and not all event codes are available yet, so they can come
later.

Series is here:
https://github.com/hisilicon/kernel-dev/tree/private-topic-perf-5.10-sys-pmu-events-v6

Kernel part accepted / pending in the following:
- https://lore.kernel.org/linux-arm-kernel/160631703729.2332128.13220150013299384201.b4-ty@kernel.org/
- https://lore.kernel.org/linux-devicetree/9468d155-f285-0d03-181b-fe378042f858@huawei.com/

Differences to v5:
- Add tags from Kajol Jain (thanks)
- For now, don't include HiSilicon hip09 events until all event codes
  available
- Rebase

Differences to v4:
- Drop hack for fixing metrics containing aliases which match multiple
  PMUs, and add a proper fix attempt
- Rebase to acme perf/core from 30 Oct
- Fix up imx8 event names according to request from Joakim

Joakim Zhang (1):
  perf vendor events: Add JSON metrics for imx8mm DDR Perf

John Garry (9):
  perf jevents: Add support for an extra directory level
  perf jevents: Add support for system events tables
  perf pmu: Add pmu_id()
  perf pmu: Add pmu_add_sys_aliases()
  perf evlist: Change perf_evlist__splice_list_tail() ordering
  perf metricgroup: Fix metrics using aliases covering multiple PMUs
  perf metricgroup: Split up metricgroup__print()
  perf metricgroup: Support printing metric groups for system PMUs
  perf metricgroup: Support adding metrics for system PMUs

 .../arch/arm64/freescale/imx8mm/sys/ddrc.json |  39 +++
 .../arm64/freescale/imx8mm/sys/metrics.json   |  18 ++
 tools/perf/pmu-events/jevents.c               |  87 +++++-
 tools/perf/pmu-events/pmu-events.h            |   6 +
 tools/perf/util/evlist.c                      |  19 +-
 tools/perf/util/metricgroup.c                 | 254 +++++++++++++-----
 tools/perf/util/pmu.c                         |  96 +++++++
 tools/perf/util/pmu.h                         |   3 +
 8 files changed, 451 insertions(+), 71 deletions(-)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json

-- 
2.26.2

[PATCH v6 01/10] perf jevents: Add support for an extra directory level

[John Garry]

Currently only upto a level 2 directory is supported, in form
vendor/platform.

Add support for a further level, to support vendor/platform
sub-directories in future, which will be vendor/platform/cpu
and vendor/platform/sys.

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/pmu-events/jevents.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 72cfa3b5046d..9022216b1253 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -978,15 +978,20 @@ static int process_one_file(const char *fpath, const struct stat *sb,
 	int level   = ftwbuf->level;
 	int err = 0;
 
-	if (level == 2 && is_dir) {
+	if (level >= 2 && is_dir) {
+		int count = 0;
 		/*
 		 * For level 2 directory, bname will include parent name,
 		 * like vendor/platform. So search back from platform dir
 		 * to find this.
+		 * Something similar for level 3 directory, but we're a PMU
+		 * category folder, like vendor/platform/cpu.
 		 */
 		bname = (char *) fpath + ftwbuf->base - 2;
 		for (;;) {
 			if (*bname == '/')
+				count++;
+			if (count == level - 1)
 				break;
 			bname--;
 		}
@@ -999,13 +1004,13 @@ static int process_one_file(const char *fpath, const struct stat *sb,
 		 level, sb->st_size, bname, fpath);
 
 	/* base dir or too deep */
-	if (level == 0 || level > 3)
+	if (level == 0 || level > 4)
 		return 0;
 
 
 	/* model directory, reset topic */
 	if ((level == 1 && is_dir && is_leaf_dir(fpath)) ||
-	    (level == 2 && is_dir)) {
+	    (level >= 2 && is_dir && is_leaf_dir(fpath))) {
 		if (close_table)
 			print_events_table_suffix(eventsfp);
 
-- 
2.26.2

[PATCH v6 02/10] perf jevents: Add support for system events tables

[John Garry]

Process the JSONs to find support for "system" events, which are not tied
to a specific CPUID.

A "COMPAT" property is now used to match against the namespace ID from the
kernel PMU driver.

The generated pmu-events.c will now have 2 tables:
a. CPU events, as before.
b. New pmu_sys_event_tables[] table, which will have events matched to
   specific SoCs.

It will look like this:

struct pmu_event pme_hisilicon_hip09_sys[] = {
{
	.name = "cycles",
	.compat = "0x00030736",
	.event = "event=0",
	.desc = "Clock cycles",
	.topic = "smmu v3 pmcg",
	.long_desc = "Clock cycles",
},
{
	.name = "smmuv3_pmcg.l1_tlb",
	.compat = "0x00030736",
	.event = "event=0x8a",
	.desc = "SMMUv3 PMCG l1_tlb. Unit: smmuv3_pmcg ",
	.topic = "smmu v3 pmcg",
	.long_desc = "SMMUv3 PMCG l1_tlb",
	.pmu = "smmuv3_pmcg",
},
...
};

struct pmu_event pme_arm_cortex_a53[] = {
{
	.name = "ext_mem_req",
	.event = "event=0xc0",
	.desc = "External memory request",
	.topic = "memory",
},
{
	.name = "ext_mem_req_nc",
	.event = "event=0xc1",
	.desc = "Non-cacheable external memory request",
	.topic = "memory",
},
...
};

struct pmu_event pme_hisilicon_hip09_cpu[] = {
{
	.name = "l2d_cache_refill_wr",
	.event = "event=0x53",
	.desc = "L2D cache refill, write",
	.topic = "core imp def",
	.long_desc = "Attributable Level 2 data cache refill, write",
},
...
};

struct pmu_events_map pmu_events_map[] = {
{
	.cpuid = "0x00000000410fd030",
	.version = "v1",
	.type = "core",
	.table = pme_arm_cortex_a53
},
{
	.cpuid = "0x00000000480fd010",
	.version = "v1",
	.type = "core",
	.table = pme_hisilicon_hip09_cpu
},
	{
		.table = 0
	},
};

struct pmu_event pme_hisilicon_hip09_cpu[] = {
{
	.name = "uncore_hisi_l3c.rd_cpipe",
	.event = "event=0",
	.desc = "Total read accesses. Unit: hisi_sccl,l3c ",
	.topic = "uncore l3c",
	.long_desc = "Total read accesses",
	.pmu = "hisi_sccl,l3c",
},
{
	.name = "uncore_hisi_l3c.wr_cpipe",
	.event = "event=0x1",
	.desc = "Total write accesses. Unit: hisi_sccl,l3c ",
	.topic = "uncore l3c",
	.long_desc = "Total write accesses",
	.pmu = "hisi_sccl,l3c",
},
...
};


struct pmu_sys_events pmu_sys_event_tables[] = {
{
	.table = pme_hisilicon_hip09_sys,
},
...
};

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/pmu-events/jevents.c    | 74 +++++++++++++++++++++++++++++-
 tools/perf/pmu-events/pmu-events.h |  6 +++
 2 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 9022216b1253..7a65fd2d25bb 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -55,6 +55,7 @@ char *prog;
 
 struct json_event {
 	char *name;
+	char *compat;
 	char *event;
 	char *desc;
 	char *long_desc;
@@ -82,6 +83,23 @@ enum aggr_mode_class convert(const char *aggr_mode)
 
 typedef int (*func)(void *data, struct json_event *je);
 
+static LIST_HEAD(sys_event_tables);
+
+struct sys_event_table {
+	struct list_head list;
+	char *soc_id;
+};
+
+static void free_sys_event_tables(void)
+{
+	struct sys_event_table *et, *next;
+
+	list_for_each_entry_safe(et, next, &sys_event_tables, list) {
+		free(et->soc_id);
+		free(et);
+	}
+}
+
 int eprintf(int level, int var, const char *fmt, ...)
 {
 
@@ -360,6 +378,8 @@ static int print_events_table_entry(void *data, struct json_event *je)
 	if (je->event)
 		fprintf(outfp, "\t.event = \"%s\",\n", je->event);
 	fprintf(outfp, "\t.desc = \"%s\",\n", je->desc);
+	if (je->compat)
+		fprintf(outfp, "\t.compat = \"%s\",\n", je->compat);
 	fprintf(outfp, "\t.topic = \"%s\",\n", topic);
 	if (je->long_desc && je->long_desc[0])
 		fprintf(outfp, "\t.long_desc = \"%s\",\n", je->long_desc);
@@ -390,6 +410,7 @@ struct event_struct {
 	struct list_head list;
 	char *name;
 	char *event;
+	char *compat;
 	char *desc;
 	char *long_desc;
 	char *pmu;
@@ -583,6 +604,8 @@ static int json_events(const char *fn,
 				free(code);
 			} else if (json_streq(map, field, "EventName")) {
 				addfield(map, &je.name, "", "", val);
+			} else if (json_streq(map, field, "Compat")) {
+				addfield(map, &je.compat, "", "", val);
 			} else if (json_streq(map, field, "BriefDescription")) {
 				addfield(map, &je.desc, "", "", val);
 				fixdesc(je.desc);
@@ -683,6 +706,7 @@ static int json_events(const char *fn,
 		free(event);
 		free(je.desc);
 		free(je.name);
+		free(je.compat);
 		free(je.long_desc);
 		free(extra_desc);
 		free(je.pmu);
@@ -747,6 +771,15 @@ static char *file_name_to_table_name(char *fname)
 	return tblname;
 }
 
+static bool is_sys_dir(char *fname)
+{
+	size_t len = strlen(fname), len2 = strlen("/sys");
+
+	if (len2 > len)
+		return false;
+	return !strcmp(fname+len-len2, "/sys");
+}
+
 static void print_mapping_table_prefix(FILE *outfp)
 {
 	fprintf(outfp, "struct pmu_events_map pmu_events_map[] = {\n");
@@ -781,6 +814,23 @@ static void print_mapping_test_table(FILE *outfp)
 	fprintf(outfp, "},\n");
 }
 
+static int process_system_event_tables(FILE *outfp)
+{
+	struct sys_event_table *sys_event_table;
+
+	fprintf(outfp, "\nstruct pmu_sys_events pmu_sys_event_tables[] = {");
+
+	list_for_each_entry(sys_event_table, &sys_event_tables, list) {
+		fprintf(outfp, "\n\t{\n\t\t.table = %s,\n\t},",
+			sys_event_table->soc_id);
+	}
+	fprintf(outfp, "\n\t{\n\t\t.table = 0\n\t},");
+
+	fprintf(outfp, "\n};\n");
+
+	return 0;
+}
+
 static int process_mapfile(FILE *outfp, char *fpath)
 {
 	int n = 16384;
@@ -1026,6 +1076,22 @@ static int process_one_file(const char *fpath, const struct stat *sb,
 			return -1;
 		}
 
+		if (is_sys_dir(bname)) {
+			struct sys_event_table *sys_event_table;
+
+			sys_event_table = malloc(sizeof(*sys_event_table));
+			if (!sys_event_table)
+				return -1;
+
+			sys_event_table->soc_id = strdup(tblname);
+			if (!sys_event_table->soc_id) {
+				free(sys_event_table);
+				return -1;
+			}
+			list_add_tail(&sys_event_table->list,
+				      &sys_event_tables);
+		}
+
 		print_events_table_prefix(eventsfp, tblname);
 		return 0;
 	}
@@ -1185,10 +1251,16 @@ int main(int argc, char *argv[])
 	}
 
 	rc = process_mapfile(eventsfp, mapfile);
-	fclose(eventsfp);
 	if (rc) {
 		pr_info("%s: Error processing mapfile %s\n", prog, mapfile);
 		/* Make build fail */
+		ret = 1;
+		goto err_close_eventsfp;
+	}
+
+	rc = process_system_event_tables(eventsfp);
+	fclose(eventsfp);
+	if (rc) {
 		ret = 1;
 		goto err_out;
 	}
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index 7da1a3743b77..d1172f6aebf1 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -12,6 +12,7 @@ enum aggr_mode_class {
  */
 struct pmu_event {
 	const char *name;
+	const char *compat;
 	const char *event;
 	const char *desc;
 	const char *topic;
@@ -43,10 +44,15 @@ struct pmu_events_map {
 	struct pmu_event *table;
 };
 
+struct pmu_sys_events {
+	struct pmu_event *table;
+};
+
 /*
  * Global table mapping each known CPU for the architecture to its
  * table of PMU events.
  */
 extern struct pmu_events_map pmu_events_map[];
+extern struct pmu_sys_events pmu_sys_event_tables[];
 
 #endif
-- 
2.26.2

[PATCH v6 03/10] perf pmu: Add pmu_id()

[John Garry]

Add a function to read the PMU id sysfs entry. This is only done for uncore
PMUs where this would possibly be relevant.

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/util/pmu.c | 18 ++++++++++++++++++
 tools/perf/util/pmu.h |  1 +
 2 files changed, 19 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index d41caeb35cf6..cbeda45ce578 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -597,6 +597,7 @@ static struct perf_cpu_map *__pmu_cpumask(const char *path)
  * Uncore PMUs have a "cpumask" file under sysfs. CPU PMUs (e.g. on arm/arm64)
  * may have a "cpus" file.
  */
+#define SYS_TEMPLATE_ID	"./bus/event_source/devices/%s/identifier"
 #define CPUS_TEMPLATE_UNCORE	"%s/bus/event_source/devices/%s/cpumask"
 #define CPUS_TEMPLATE_CPU	"%s/bus/event_source/devices/%s/cpus"
 
@@ -635,6 +636,21 @@ static bool pmu_is_uncore(const char *name)
 	return file_available(path);
 }
 
+static char *pmu_id(const char *name)
+{
+	char path[PATH_MAX], *str;
+	size_t len;
+
+	snprintf(path, PATH_MAX, SYS_TEMPLATE_ID, name);
+
+	if (sysfs__read_str(path, &str, &len) < 0)
+		return NULL;
+
+	str[len - 1] = 0; /* remove line feed */
+
+	return str;
+}
+
 /*
  *  PMU CORE devices have different name other than cpu in sysfs on some
  *  platforms.
@@ -847,6 +863,8 @@ static struct perf_pmu *pmu_lookup(const char *name)
 	pmu->name = strdup(name);
 	pmu->type = type;
 	pmu->is_uncore = pmu_is_uncore(name);
+	if (pmu->is_uncore)
+		pmu->id = pmu_id(name);
 	pmu->max_precise = pmu_max_precise(name);
 	pmu_add_cpu_aliases(&aliases, pmu);
 
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index a64e9c9ce731..d4366e8e79df 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -30,6 +30,7 @@ struct perf_pmu_caps {
 
 struct perf_pmu {
 	char *name;
+	char *id;
 	__u32 type;
 	bool selectable;
 	bool is_uncore;
-- 
2.26.2

[PATCH v6 04/10] perf pmu: Add pmu_add_sys_aliases()

[John Garry]

Add pmu_add_sys_aliases() to add system PMU events aliases.

For adding system PMU events, iterate through all the events for all SoC
event tables in pmu_sys_event_tables[].

Matches must satisfy both:
- PMU identifier matches event "compat" value
- event "Unit" member must match, same as uncore event aliases matched by
  CPUID

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/util/pmu.c | 78 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/pmu.h |  2 ++
 2 files changed, 80 insertions(+)

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index cbeda45ce578..44ef28302fc7 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -812,6 +812,83 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
 	pmu_add_cpu_aliases_map(head, pmu, map);
 }
 
+void pmu_for_each_sys_event(pmu_sys_event_iter_fn fn, void *data)
+{
+	int i = 0;
+
+	while (1) {
+		struct pmu_sys_events *event_table;
+		int j = 0;
+
+		event_table = &pmu_sys_event_tables[i++];
+
+		if (!event_table->table)
+			break;
+
+		while (1) {
+			struct pmu_event *pe = &event_table->table[j++];
+			int ret;
+
+			if (!pe->name && !pe->metric_group && !pe->metric_name)
+				break;
+
+			ret = fn(pe, data);
+			if (ret)
+				break;
+		}
+	}
+}
+
+struct pmu_sys_event_iter_data {
+	struct list_head *head;
+	struct perf_pmu *pmu;
+};
+
+static int pmu_add_sys_aliases_iter_fn(struct pmu_event *pe, void *data)
+{
+	struct pmu_sys_event_iter_data *idata = data;
+	struct perf_pmu *pmu = idata->pmu;
+
+	if (!pe->name) {
+		if (pe->metric_group || pe->metric_name)
+			return 0;
+		return -EINVAL;
+	}
+
+	if (!pe->compat || !pe->pmu)
+		return 0;
+
+	if (!strcmp(pmu->id, pe->compat) &&
+	    pmu_uncore_alias_match(pe->pmu, pmu->name)) {
+		__perf_pmu__new_alias(idata->head, NULL,
+				      (char *)pe->name,
+				      (char *)pe->desc,
+				      (char *)pe->event,
+				      (char *)pe->long_desc,
+				      (char *)pe->topic,
+				      (char *)pe->unit,
+				      (char *)pe->perpkg,
+				      (char *)pe->metric_expr,
+				      (char *)pe->metric_name,
+				      (char *)pe->deprecated);
+	}
+
+	return 0;
+}
+
+static void pmu_add_sys_aliases(struct list_head *head, struct perf_pmu *pmu)
+{
+	struct pmu_sys_event_iter_data idata = {
+		.head = head,
+		.pmu = pmu,
+	};
+
+	if (!pmu->id)
+		return;
+
+	pmu_for_each_sys_event(pmu_add_sys_aliases_iter_fn, &idata);
+}
+
 struct perf_event_attr * __weak
 perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
 {
@@ -867,6 +944,7 @@ static struct perf_pmu *pmu_lookup(const char *name)
 		pmu->id = pmu_id(name);
 	pmu->max_precise = pmu_max_precise(name);
 	pmu_add_cpu_aliases(&aliases, pmu);
+	pmu_add_sys_aliases(&aliases, pmu);
 
 	INIT_LIST_HEAD(&pmu->format);
 	INIT_LIST_HEAD(&pmu->aliases);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index d4366e8e79df..8164388478c6 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -117,6 +117,8 @@ struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
 void perf_pmu_free_alias(struct perf_pmu_alias *alias);
 
+typedef int (*pmu_sys_event_iter_fn)(struct pmu_event *pe, void *data);
+void pmu_for_each_sys_event(pmu_sys_event_iter_fn fn, void *data);
 int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
 
 int perf_pmu__caps_parse(struct perf_pmu *pmu);
-- 
2.26.2

[PATCH v6 05/10] perf evlist: Change perf_evlist__splice_list_tail() ordering

[John Garry]

Function find_evsel_group() expects events to be ordered such that they
are grouped after their leader.

Modify perf_evlist__splice_list_tail() to guarantee this (ordering).

[Should prob also change the function name]

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/util/evlist.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 8bdf3d2c907c..b7e4e6d85d59 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -180,11 +180,22 @@ void evlist__remove(struct evlist *evlist, struct evsel *evsel)
 void perf_evlist__splice_list_tail(struct evlist *evlist,
 				   struct list_head *list)
 {
-	struct evsel *evsel, *temp;
+	while (!list_empty(list)) {
+		struct evsel *evsel, *temp, *leader = NULL;
 
-	__evlist__for_each_entry_safe(list, temp, evsel) {
-		list_del_init(&evsel->core.node);
-		evlist__add(evlist, evsel);
+		__evlist__for_each_entry_safe(list, temp, evsel) {
+			list_del_init(&evsel->core.node);
+			evlist__add(evlist, evsel);
+			leader = evsel;
+			break;
+		}
+
+		__evlist__for_each_entry_safe(list, temp, evsel) {
+			if (evsel->leader == leader) {
+				list_del_init(&evsel->core.node);
+				evlist__add(evlist, evsel);
+			}
+		}
 	}
 }
 
-- 
2.26.2

[PATCH v6 06/10] perf metricgroup: Fix metrics using aliases covering multiple PMUs

[John Garry]

Support for metric expressions using aliases which cover multiple PMUs is
broken. Consider the following test metric expression:

"MetricExpr": "UNC_CBO_XSNP_RESPONSE.MISS_XCORE * UNC_CBO_XSNP_RESPONSE.MISS_EVICTION"

When used on my broadwell, "perf stat" gives:

unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_1/umask=0x81,event=0x22/
unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_0/umask=0x81,event=0x22/
unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_1/umask=0x41,event=0x22/
unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_0/umask=0x41,event=0x22/
Control descriptor is not initialized
unc_cbo_xsnp_response.miss_eviction: 3645925 1000850523 1000850523
unc_cbo_xsnp_response.miss_xcore: 106850 1000850523 1000850523

 Performance counter stats for 'system wide':

         3,645,925      unc_cbo_xsnp_response.miss_eviction # 389567086250.00 test_metric_inc
           106,850      unc_cbo_xsnp_response.miss_xcore

       1.000883096 seconds time elapsed


Notice that only the results from one PMU are included. Fix the logic of
find_evsel_group() to enable events which apply to multiple PMUs, by
checking if the event pmu_name matches that of the metric event.

With that, "perf stat" now gives:

unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_1/umask=0x81,event=0x22/
unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_0/umask=0x81,event=0x22/
unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_1/umask=0x41,event=0x22/
unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_0/umask=0x41,event=0x22/
Control descriptor is not initialized
unc_cbo_xsnp_response.miss_eviction: 4237983 1000904100 1000904100
unc_cbo_xsnp_response.miss_xcore: 218643 1000904100 1000904100
unc_cbo_xsnp_response.miss_eviction: 4254148 1000902629 1000902629
unc_cbo_xsnp_response.miss_xcore: 213352 1000902629 1000902629

 Performance counter stats for 'system wide':

         4,237,983      unc_cbo_xsnp_response.miss_eviction # 3668558131345.00 test_metric_inc
           218,643      unc_cbo_xsnp_response.miss_xcore
         4,254,148      unc_cbo_xsnp_response.miss_eviction
           213,352      unc_cbo_xsnp_response.miss_xcore

       1.000938151 seconds time elapsed


Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/util/metricgroup.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 81d201c8b833..b89160718c04 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -279,7 +279,9 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 			 * when then group is left.
 			 */
 			if (!has_constraint &&
-			    ev->leader != metric_events[i]->leader)
+			    ev->leader != metric_events[i]->leader &&
+			    !strcmp(ev->leader->pmu_name,
+				    metric_events[i]->leader->pmu_name))
 				break;
 			if (!strcmp(metric_events[i]->name, ev->name)) {
 				set_bit(ev->idx, evlist_used);
-- 
2.26.2

[PATCH v6 07/10] perf metricgroup: Split up metricgroup__print()

[John Garry]

To aid supporting system event metric groups, break up the function
metricgroup__print() into a part which iterates metrics and a part which
actually "prints" the metric.

No functional change intended.

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/util/metricgroup.c | 124 +++++++++++++++++++---------------
 1 file changed, 70 insertions(+), 54 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index b89160718c04..4c6a686b08eb 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -493,6 +493,72 @@ static void metricgroup__print_strlist(struct strlist *metrics, bool raw)
 		putchar('\n');
 }
 
+static int metricgroup__print_pmu_event(struct pmu_event *pe,
+					bool metricgroups, char *filter,
+					bool raw, bool details,
+					struct rblist *groups,
+					struct strlist *metriclist)
+{
+	const char *g;
+	char *omg, *mg;
+
+	g = pe->metric_group;
+	if (!g && pe->metric_name) {
+		if (pe->name)
+			return 0;
+		g = "No_group";
+	}
+
+	if (!g)
+		return 0;
+
+	mg = strdup(g);
+
+	if (!mg)
+		return -ENOMEM;
+	omg = mg;
+	while ((g = strsep(&mg, ";")) != NULL) {
+		struct mep *me;
+		char *s;
+
+		g = skip_spaces(g);
+		if (*g == 0)
+			g = "No_group";
+		if (filter && !strstr(g, filter))
+			continue;
+		if (raw)
+			s = (char *)pe->metric_name;
+		else {
+			if (asprintf(&s, "%s\n%*s%s]",
+				     pe->metric_name, 8, "[", pe->desc) < 0)
+				return -1;
+			if (details) {
+				if (asprintf(&s, "%s\n%*s%s]",
+					     s, 8, "[", pe->metric_expr) < 0)
+					return -1;
+			}
+		}
+
+		if (!s)
+			continue;
+
+		if (!metricgroups) {
+			strlist__add(metriclist, s);
+		} else {
+			me = mep_lookup(groups, g);
+			if (!me)
+				continue;
+			strlist__add(me->metrics, s);
+		}
+
+		if (!raw)
+			free(s);
+	}
+	free(omg);
+
+	return 0;
+}
+
 void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 			bool raw, bool details)
 {
@@ -517,66 +583,16 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	groups.node_cmp = mep_cmp;
 	groups.node_delete = mep_delete;
 	for (i = 0; ; i++) {
-		const char *g;
 		pe = &map->table[i];
 
 		if (!pe->name && !pe->metric_group && !pe->metric_name)
 			break;
 		if (!pe->metric_expr)
 			continue;
-		g = pe->metric_group;
-		if (!g && pe->metric_name) {
-			if (pe->name)
-				continue;
-			g = "No_group";
-		}
-		if (g) {
-			char *omg;
-			char *mg = strdup(g);
-
-			if (!mg)
-				return;
-			omg = mg;
-			while ((g = strsep(&mg, ";")) != NULL) {
-				struct mep *me;
-				char *s;
-
-				g = skip_spaces(g);
-				if (*g == 0)
-					g = "No_group";
-				if (filter && !strstr(g, filter))
-					continue;
-				if (raw)
-					s = (char *)pe->metric_name;
-				else {
-					if (asprintf(&s, "%s\n%*s%s]",
-						     pe->metric_name, 8, "[", pe->desc) < 0)
-						return;
-
-					if (details) {
-						if (asprintf(&s, "%s\n%*s%s]",
-							     s, 8, "[", pe->metric_expr) < 0)
-							return;
-					}
-				}
-
-				if (!s)
-					continue;
-
-				if (!metricgroups) {
-					strlist__add(metriclist, s);
-				} else {
-					me = mep_lookup(&groups, g);
-					if (!me)
-						continue;
-					strlist__add(me->metrics, s);
-				}
-
-				if (!raw)
-					free(s);
-			}
-			free(omg);
-		}
+		if (metricgroup__print_pmu_event(pe, metricgroups, filter,
+						 raw, details, &groups,
+						 metriclist) < 0)
+			return;
 	}
 
 	if (!filter || !rblist__empty(&groups)) {
-- 
2.26.2

[PATCH v6 08/10] perf metricgroup: Support printing metric groups for system PMUs

[John Garry]

Currently printing metricgroups for core- or uncore-based events matched
by CPUID is supported.

Extend this for system events.

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/util/metricgroup.c | 64 ++++++++++++++++++++++++++++++++---
 1 file changed, 60 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 4c6a686b08eb..abc5d0e28d0f 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -559,6 +559,49 @@ static int metricgroup__print_pmu_event(struct pmu_event *pe,
 	return 0;
 }
 
+struct metricgroup_print_sys_idata {
+	struct strlist *metriclist;
+	bool metricgroups;
+	char *filter;
+	bool raw;
+	bool details;
+	struct rblist *groups;
+};
+
+typedef int (*metricgroup_sys_event_iter_fn)(struct pmu_event *pe, void *);
+
+struct metricgroup_iter_data {
+	metricgroup_sys_event_iter_fn fn;
+	void *data;
+};
+
+static int metricgroup__sys_event_iter(struct pmu_event *pe, void *data)
+{
+	struct metricgroup_iter_data *d = data;
+	struct perf_pmu *pmu = NULL;
+
+	if (!pe->metric_expr || !pe->compat)
+		return 0;
+
+	while ((pmu = perf_pmu__scan(pmu))) {
+
+		if (!pmu->id || strcmp(pmu->id, pe->compat))
+			continue;
+
+		return d->fn(pe, d->data);
+	}
+
+	return 0;
+}
+
+static int metricgroup__print_sys_event_iter(struct pmu_event *pe, void *data)
+{
+	struct metricgroup_print_sys_idata *d = data;
+
+	return metricgroup__print_pmu_event(pe, d->metricgroups, d->filter, d->raw,
+				     d->details, d->groups, d->metriclist);
+}
+
 void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 			bool raw, bool details)
 {
@@ -569,9 +612,6 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	struct rb_node *node, *next;
 	struct strlist *metriclist = NULL;
 
-	if (!map)
-		return;
-
 	if (!metricgroups) {
 		metriclist = strlist__new(NULL, NULL);
 		if (!metriclist)
@@ -582,7 +622,7 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 	groups.node_new = mep_new;
 	groups.node_cmp = mep_cmp;
 	groups.node_delete = mep_delete;
-	for (i = 0; ; i++) {
+	for (i = 0; map; i++) {
 		pe = &map->table[i];
 
 		if (!pe->name && !pe->metric_group && !pe->metric_name)
@@ -595,6 +635,22 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 			return;
 	}
 
+	{
+		struct metricgroup_iter_data data = {
+			.fn = metricgroup__print_sys_event_iter,
+			.data = (void *) &(struct metricgroup_print_sys_idata){
+				.metriclist = metriclist,
+				.metricgroups = metricgroups,
+				.filter = filter,
+				.raw = raw,
+				.details = details,
+				.groups = &groups,
+			},
+		};
+
+		pmu_for_each_sys_event(metricgroup__sys_event_iter, &data);
+	}
+
 	if (!filter || !rblist__empty(&groups)) {
 		if (metricgroups && !raw)
 			printf("\nMetric Groups:\n\n");
-- 
2.26.2

[PATCH v6 09/10] perf metricgroup: Support adding metrics for system PMUs

[John Garry]

Currently adding metrics for core- or uncore-based events matched by CPUID
is supported.

Extend this for system events.

Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 tools/perf/util/metricgroup.c | 66 +++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index abc5d0e28d0f..4e7bde578666 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -415,6 +415,12 @@ static bool match_metric(const char *n, const char *list)
 	return false;
 }
 
+static bool match_pe_metric(struct pmu_event *pe, const char *metric)
+{
+	return match_metric(pe->metric_group, metric) ||
+	       match_metric(pe->metric_name, metric);
+}
+
 struct mep {
 	struct rb_node nd;
 	const char *name;
@@ -757,6 +763,16 @@ int __weak arch_get_runtimeparam(struct pmu_event *pe __maybe_unused)
 	return 1;
 }
 
+struct metricgroup_add_iter_data {
+	struct list_head *metric_list;
+	const char *metric;
+	struct metric **m;
+	bool metric_no_group;
+	struct expr_ids *ids;
+	bool *has_match;
+	int *ret;
+};
+
 static int __add_metric(struct list_head *metric_list,
 			struct pmu_event *pe,
 			bool metric_no_group,
@@ -866,10 +882,11 @@ static int __add_metric(struct list_head *metric_list,
 	return 0;
 }
 
-#define map_for_each_event(__pe, __idx, __map)				\
-	for (__idx = 0, __pe = &__map->table[__idx];			\
-	     __pe->name || __pe->metric_group || __pe->metric_name;	\
-	     __pe = &__map->table[++__idx])
+#define map_for_each_event(__pe, __idx, __map)					\
+	if (__map)								\
+		for (__idx = 0, __pe = &__map->table[__idx];			\
+		     __pe->name || __pe->metric_group || __pe->metric_name;	\
+		     __pe = &__map->table[++__idx])
 
 #define map_for_each_metric(__pe, __idx, __map, __metric)		\
 	map_for_each_event(__pe, __idx, __map)				\
@@ -1037,6 +1054,29 @@ static int add_metric(struct list_head *metric_list,
 	return ret;
 }
 
+static int metricgroup__add_metric_sys_event_iter(struct pmu_event *pe,
+						  void *data)
+{
+	struct metricgroup_add_iter_data *d = data;
+	int ret;
+
+	if (!match_pe_metric(pe, d->metric))
+		return 0;
+
+	ret = add_metric(d->metric_list, pe, d->metric_no_group, d->m, NULL, d->ids);
+	if (ret)
+		return ret;
+
+	ret = resolve_metric(d->metric_no_group,
+				     d->metric_list, NULL, d->ids);
+	if (ret)
+		return ret;
+
+	*(d->has_match) = true;
+
+	return *d->ret;
+}
+
 static int metricgroup__add_metric(const char *metric, bool metric_no_group,
 				   struct strbuf *events,
 				   struct list_head *metric_list,
@@ -1067,6 +1107,22 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
 			goto out;
 	}
 
+	{
+		struct metricgroup_iter_data data = {
+			.fn = metricgroup__add_metric_sys_event_iter,
+			.data = (void *) &(struct metricgroup_add_iter_data) {
+				.metric_list = &list,
+				.metric = metric,
+				.metric_no_group = metric_no_group,
+				.m = &m,
+				.ids = &ids,
+				.has_match = &has_match,
+				.ret = &ret,
+			},
+		};
+
+		pmu_for_each_sys_event(metricgroup__sys_event_iter, &data);
+	}
 	/* End of pmu events. */
 	if (!has_match) {
 		ret = -EINVAL;
@@ -1193,8 +1249,6 @@ int metricgroup__parse_groups(const struct option *opt,
 	struct evlist *perf_evlist = *(struct evlist **)opt->value;
 	struct pmu_events_map *map = perf_pmu__find_map(NULL);
 
-	if (!map)
-		return 0;
 
 	return parse_groups(perf_evlist, str, metric_no_group,
 			    metric_no_merge, NULL, metric_events, map);
-- 
2.26.2

[PATCH v6 10/10] perf vendor events: Add JSON metrics for imx8mm DDR Perf

[John Garry]

From: Joakim Zhang <qiangqing.zhang@nxp.com>

Add JSON metrics for imx8mm DDR Perf.

Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Acked-by: Kajol Jain <kjain@linux.ibm.com>
---
 .../arch/arm64/freescale/imx8mm/sys/ddrc.json | 39 +++++++++++++++++++
 .../arm64/freescale/imx8mm/sys/metrics.json   | 18 +++++++++
 tools/perf/pmu-events/jevents.c               |  2 +
 3 files changed, 59 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
 create mode 100644 tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json

diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
new file mode 100644
index 000000000000..3b1cd708f568
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/ddrc.json
@@ -0,0 +1,39 @@
+[
+   {
+           "BriefDescription": "ddr cycles event",
+           "EventCode": "0x00",
+           "EventName": "imx8mm_ddr.cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.MX8MM"
+   },
+   {
+           "BriefDescription": "ddr read-cycles event",
+           "EventCode": "0x2a",
+           "EventName": "imx8mm_ddr.read_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.MX8MM"
+   },
+   {
+           "BriefDescription": "ddr write-cycles event",
+           "EventCode": "0x2b",
+           "EventName": "imx8mm_ddr.write_cycles",
+           "Unit": "imx8_ddr",
+           "Compat": "i.MX8MM"
+   },
+   {
+           "BriefDescription": "ddr read event",
+           "EventCode": "0x35",
+           "EventName": "imx8mm_ddr.read",
+           "Unit": "imx8_ddr",
+           "Compat": "i.MX8MM"
+   },
+   {
+           "BriefDescription": "ddr write event",
+           "EventCode": "0x38",
+           "EventName": "imx8mm_ddr.write",
+           "Unit": "imx8_ddr",
+           "Compat": "i.MX8MM"
+   }
+]
+
+
diff --git a/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
new file mode 100644
index 000000000000..8e553b67cae6
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/freescale/imx8mm/sys/metrics.json
@@ -0,0 +1,18 @@
+[
+   {
+	    "BriefDescription": "bytes all masters read from ddr based on read-cycles event",
+	    "MetricName": "imx8mm_ddr_read.all",
+	    "MetricExpr": "imx8mm_ddr.read_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4KB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.MX8MM"
+    },
+   {
+	    "BriefDescription": "bytes all masters write to ddr based on write-cycles event",
+	    "MetricName": "imx8mm_ddr_write.all",
+	    "MetricExpr": "imx8mm_ddr.write_cycles * 4 * 4",
+	    "ScaleUnit": "9.765625e-4KB",
+	    "Unit": "imx8_ddr",
+	    "Compat": "i.MX8MM"
+    }
+]
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 7a65fd2d25bb..e930096ad713 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -281,6 +281,8 @@ static struct map {
 	{ "hisi_sccl,ddrc", "hisi_sccl,ddrc" },
 	{ "hisi_sccl,hha", "hisi_sccl,hha" },
 	{ "hisi_sccl,l3c", "hisi_sccl,l3c" },
+	/* it's not realistic to keep adding these, we need something more scalable ... */
+	{ "imx8_ddr", "imx8_ddr" },
 	{ "L3PMC", "amd_l3" },
 	{ "DFPMC", "amd_df" },
 	{}
-- 
2.26.2

Re: [PATCH v6 06/10] perf metricgroup: Fix metrics using aliases covering multiple PMUs

[Arnaldo Carvalho de Melo]

Em Fri, Dec 04, 2020 at 07:10:12PM +0800, John Garry escreveu:
> Support for metric expressions using aliases which cover multiple PMUs is
> broken. Consider the following test metric expression:
> 
> "MetricExpr": "UNC_CBO_XSNP_RESPONSE.MISS_XCORE * UNC_CBO_XSNP_RESPONSE.MISS_EVICTION"
> 
> When used on my broadwell, "perf stat" gives:
> 
> unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_1/umask=0x81,event=0x22/
> unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_0/umask=0x81,event=0x22/
> unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_1/umask=0x41,event=0x22/
> unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_0/umask=0x41,event=0x22/
> Control descriptor is not initialized
> unc_cbo_xsnp_response.miss_eviction: 3645925 1000850523 1000850523
> unc_cbo_xsnp_response.miss_xcore: 106850 1000850523 1000850523
> 
>  Performance counter stats for 'system wide':
> 
>          3,645,925      unc_cbo_xsnp_response.miss_eviction # 389567086250.00 test_metric_inc
>            106,850      unc_cbo_xsnp_response.miss_xcore
> 
>        1.000883096 seconds time elapsed
> 
> 
> Notice that only the results from one PMU are included. Fix the logic of
> find_evsel_group() to enable events which apply to multiple PMUs, by
> checking if the event pmu_name matches that of the metric event.
> 
> With that, "perf stat" now gives:
> 
> unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_1/umask=0x81,event=0x22/
> unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_0/umask=0x81,event=0x22/
> unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_1/umask=0x41,event=0x22/
> unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_0/umask=0x41,event=0x22/
> Control descriptor is not initialized
> unc_cbo_xsnp_response.miss_eviction: 4237983 1000904100 1000904100
> unc_cbo_xsnp_response.miss_xcore: 218643 1000904100 1000904100
> unc_cbo_xsnp_response.miss_eviction: 4254148 1000902629 1000902629
> unc_cbo_xsnp_response.miss_xcore: 213352 1000902629 1000902629
> 
>  Performance counter stats for 'system wide':
> 
>          4,237,983      unc_cbo_xsnp_response.miss_eviction # 3668558131345.00 test_metric_inc
>            218,643      unc_cbo_xsnp_response.miss_xcore
>          4,254,148      unc_cbo_xsnp_response.miss_eviction
>            213,352      unc_cbo_xsnp_response.miss_xcore
> 
>        1.000938151 seconds time elapsed
> 

Next time please try to provides a Fixes: tag to help with
backporting/stable@kernel.org work.

- Arnaldo
 
> Signed-off-by: John Garry <john.garry@huawei.com>
> Acked-by: Kajol Jain <kjain@linux.ibm.com>
> ---
>  tools/perf/util/metricgroup.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 81d201c8b833..b89160718c04 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -279,7 +279,9 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
>  			 * when then group is left.
>  			 */
>  			if (!has_constraint &&
> -			    ev->leader != metric_events[i]->leader)
> +			    ev->leader != metric_events[i]->leader &&
> +			    !strcmp(ev->leader->pmu_name,
> +				    metric_events[i]->leader->pmu_name))
>  				break;
>  			if (!strcmp(metric_events[i]->name, ev->name)) {
>  				set_bit(ev->idx, evlist_used);
> -- 
> 2.26.2
> 

-- 

- Arnaldo

Re: [PATCH v6 08/10] perf metricgroup: Support printing metric groups for system PMUs

[Arnaldo Carvalho de Melo]

Em Fri, Dec 04, 2020 at 07:10:14PM +0800, John Garry escreveu:
> Currently printing metricgroups for core- or uncore-based events matched
> by CPUID is supported.
> 
> Extend this for system events.
> 
> Signed-off-by: John Garry <john.garry@huawei.com>
> Acked-by: Kajol Jain <kjain@linux.ibm.com>
> ---
>  tools/perf/util/metricgroup.c | 64 ++++++++++++++++++++++++++++++++---
>  1 file changed, 60 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 4c6a686b08eb..abc5d0e28d0f 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -559,6 +559,49 @@ static int metricgroup__print_pmu_event(struct pmu_event *pe,
>  	return 0;
>  }
>  
> +struct metricgroup_print_sys_idata {
> +	struct strlist *metriclist;
> +	bool metricgroups;
> +	char *filter;
> +	bool raw;
> +	bool details;
> +	struct rblist *groups;
> +};

I'm doing some reorg to avoid these holes:

[acme@five perf]$ pahole -C metricgroup_print_sys_idata ~/bin/perf
struct metricgroup_print_sys_idata {
	struct strlist *           metriclist;           /*     0     8 */
	_Bool                      metricgroups;         /*     8     1 */

	/* XXX 7 bytes hole, try to pack */

	char *                     filter;               /*    16     8 */
	_Bool                      raw;                  /*    24     1 */
	_Bool                      details;              /*    25     1 */

	/* XXX 6 bytes hole, try to pack */

	struct rblist *            groups;               /*    32     8 */

	/* size: 40, cachelines: 1, members: 6 */
	/* sum members: 27, holes: 2, sum holes: 13 */
	/* last cacheline: 40 bytes */
};
[acme@five perf]$

It ended up as:

[acme@five perf]$ pahole -C metricgroup_print_sys_idata ~/bin/perf
struct metricgroup_print_sys_idata {
	struct strlist *           metriclist;           /*     0     8 */
	char *                     filter;               /*     8     8 */
	struct rblist *            groups;               /*    16     8 */
	_Bool                      metricgroups;         /*    24     1 */
	_Bool                      raw;                  /*    25     1 */
	_Bool                      details;              /*    26     1 */

	/* size: 32, cachelines: 1, members: 6 */
	/* padding: 5 */
	/* last cacheline: 32 bytes */
};
[acme@five perf]$o

- Arnaldo

> +typedef int (*metricgroup_sys_event_iter_fn)(struct pmu_event *pe, void *);
> +
> +struct metricgroup_iter_data {
> +	metricgroup_sys_event_iter_fn fn;
> +	void *data;
> +};
> +
> +static int metricgroup__sys_event_iter(struct pmu_event *pe, void *data)
> +{
> +	struct metricgroup_iter_data *d = data;
> +	struct perf_pmu *pmu = NULL;
> +
> +	if (!pe->metric_expr || !pe->compat)
> +		return 0;
> +
> +	while ((pmu = perf_pmu__scan(pmu))) {
> +
> +		if (!pmu->id || strcmp(pmu->id, pe->compat))
> +			continue;
> +
> +		return d->fn(pe, d->data);
> +	}
> +
> +	return 0;
> +}
> +
> +static int metricgroup__print_sys_event_iter(struct pmu_event *pe, void *data)
> +{
> +	struct metricgroup_print_sys_idata *d = data;
> +
> +	return metricgroup__print_pmu_event(pe, d->metricgroups, d->filter, d->raw,
> +				     d->details, d->groups, d->metriclist);
> +}
> +
>  void metricgroup__print(bool metrics, bool metricgroups, char *filter,
>  			bool raw, bool details)
>  {
> @@ -569,9 +612,6 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
>  	struct rb_node *node, *next;
>  	struct strlist *metriclist = NULL;
>  
> -	if (!map)
> -		return;
> -
>  	if (!metricgroups) {
>  		metriclist = strlist__new(NULL, NULL);
>  		if (!metriclist)
> @@ -582,7 +622,7 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
>  	groups.node_new = mep_new;
>  	groups.node_cmp = mep_cmp;
>  	groups.node_delete = mep_delete;
> -	for (i = 0; ; i++) {
> +	for (i = 0; map; i++) {
>  		pe = &map->table[i];
>  
>  		if (!pe->name && !pe->metric_group && !pe->metric_name)
> @@ -595,6 +635,22 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
>  			return;
>  	}
>  
> +	{
> +		struct metricgroup_iter_data data = {
> +			.fn = metricgroup__print_sys_event_iter,
> +			.data = (void *) &(struct metricgroup_print_sys_idata){
> +				.metriclist = metriclist,
> +				.metricgroups = metricgroups,
> +				.filter = filter,
> +				.raw = raw,
> +				.details = details,
> +				.groups = &groups,
> +			},
> +		};
> +
> +		pmu_for_each_sys_event(metricgroup__sys_event_iter, &data);
> +	}
> +
>  	if (!filter || !rblist__empty(&groups)) {
>  		if (metricgroups && !raw)
>  			printf("\nMetric Groups:\n\n");
> -- 
> 2.26.2
> 

-- 

- Arnaldo

Re: [PATCH v6 06/10] perf metricgroup: Fix metrics using aliases covering multiple PMUs

[John Garry]

On 07/12/2020 17:19, Arnaldo Carvalho de Melo wrote:
> Next time please try to provides a Fixes: tag to help with
> backporting/stable@kernel.org  work.
> 

Hi Arnaldo,

I know you asked me this before Re. fixes tags ... but I don't know any 
cases of "metrics using aliases covering multiple PMUs" in mainline 
today (which this patch addresses). If there are some, then I guess that 
they are broken and we should seek them out.

Note that this topic was discussed here initially:
https://lore.kernel.org/linux-perf-users/CAP-5=fUy6FOszNRwJF6ZNpqQSSyrnLPV6GbkEcZMqAhUp3X0ZA@mail.gmail.com/

There has been much churn on the metric code recently, so prob not a 
straight stable backport; as such, I would prefer to know some metric 
was broken and verify we fix it.

If you have any better ideas to handle this, then please let me know.

Cheers,
John

Re: [PATCH v6 08/10] perf metricgroup: Support printing metric groups for system PMUs

[John Garry]

On 07/12/2020 17:23, Arnaldo Carvalho de Melo wrote:
>> +struct metricgroup_print_sys_idata {
>> +	struct strlist *metriclist;
>> +	bool metricgroups;
>> +	char *filter;
>> +	bool raw;
>> +	bool details;
>> +	struct rblist *groups;
>> +};
> I'm doing some reorg to avoid these holes:
> 
> [acme@five perf]$ pahole -C metricgroup_print_sys_idata ~/bin/perf
> struct metricgroup_print_sys_idata {
> 	struct strlist *           metriclist;           /*     0     8 */
> 	_Bool                      metricgroups;         /*     8     1 */
> 
> 	/* XXX 7 bytes hole, try to pack */
> 
> 	char *                     filter;               /*    16     8 */
> 	_Bool                      raw;                  /*    24     1 */
> 	_Bool                      details;              /*    25     1 */
> 
> 	/* XXX 6 bytes hole, try to pack */
> 
> 	struct rblist *            groups;               /*    32     8 */
> 
> 	/* size: 40, cachelines: 1, members: 6 */
> 	/* sum members: 27, holes: 2, sum holes: 13 */
> 	/* last cacheline: 40 bytes */
> };
> [acme@five perf]$
> 
> It ended up as:
> 
> [acme@five perf]$ pahole -C metricgroup_print_sys_idata ~/bin/perf
> struct metricgroup_print_sys_idata {
> 	struct strlist *           metriclist;           /*     0     8 */
> 	char *                     filter;               /*     8     8 */
> 	struct rblist *            groups;               /*    16     8 */
> 	_Bool                      metricgroups;         /*    24     1 */
> 	_Bool                      raw;                  /*    25     1 */
> 	_Bool                      details;              /*    26     1 */
> 
> 	/* size: 32, cachelines: 1, members: 6 */
> 	/* padding: 5 */
> 	/* last cacheline: 32 bytes */
> };
> [acme@five perf]$o
> 

Hi Arnaldo,

OK, I'll be less wasteful in my struct organization.

Thanks,
John