[PATCH v2 0/8] Refactor cpuid and metric table lookup code

[PATCH v2 0/8] Refactor cpuid and metric table lookup code

[Ian Rogers]

Xu Yang <xu.yang_2@nxp.com> reported issues with the system metric
lookup:
https://lore.kernel.org/linux-perf-users/20241106085441.3945502-1-xu.yang_2@nxp.com/
These patches remove a lot of the logic relating CPUIDs to PMUs so
that the PMU isn't part of the question when finding a metric table.
For time reasons, it doesn't go as far as allowing system metrics
without a metric table as a metric table is needed for metrics to
refer to other metrics, and the refactoring of that resolution is a
hassle.

Ian Rogers (7):
  perf header: Move is_cpu_online to numa bench
  perf header: Refactor get_cpuid to take a CPU for ARM
  perf arm64 header: Use cpu argument in get_cpuid
  perf header: Avoid transitive PMU includes
  perf header: Pass a perf_cpu rather than a PMU to get_cpuid_str
  perf jevents: Add map_for_cpu
  perf pmu: Move pmu_metrics_table__find and remove ARM override

Xu Yang (1):
  perf jevents: fix breakage when do perf stat on system metric

 tools/perf/arch/arm64/util/arm-spe.c     | 14 +---
 tools/perf/arch/arm64/util/header.c      | 73 ++++++++++-----------
 tools/perf/arch/arm64/util/pmu.c         | 20 ------
 tools/perf/arch/loongarch/util/header.c  |  4 +-
 tools/perf/arch/powerpc/util/header.c    |  4 +-
 tools/perf/arch/riscv/util/header.c      |  4 +-
 tools/perf/arch/s390/util/header.c       |  6 +-
 tools/perf/arch/x86/util/auxtrace.c      |  3 +-
 tools/perf/arch/x86/util/header.c        |  5 +-
 tools/perf/bench/numa.c                  | 53 +++++++++++++++
 tools/perf/builtin-kvm.c                 |  4 +-
 tools/perf/pmu-events/empty-pmu-events.c | 39 ++++++-----
 tools/perf/pmu-events/jevents.py         | 39 ++++++-----
 tools/perf/pmu-events/pmu-events.h       |  2 +-
 tools/perf/tests/expr.c                  |  5 +-
 tools/perf/util/env.c                    |  4 +-
 tools/perf/util/expr.c                   |  6 +-
 tools/perf/util/header.c                 | 82 ++++++++----------------
 tools/perf/util/header.h                 | 23 +++----
 tools/perf/util/pmu.c                    | 25 --------
 tools/perf/util/pmu.h                    |  2 -
 tools/perf/util/probe-event.c            |  1 +
 22 files changed, 189 insertions(+), 229 deletions(-)

-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 1/8] perf jevents: fix breakage when do perf stat on system metric

[Ian Rogers]

From: Xu Yang <xu.yang_2@nxp.com>

When do perf stat on sys metric, perf tool output nothing now:
$ perf stat -a -M imx95_ddr_read.all -I 1000
$

This command runs on an arm64 machine and the Soc has one DDR hw pmu
except one armv8_cortex_a55 pmu. Their maps show as follows:

const struct pmu_events_map pmu_events_map[] = {
{
	.arch = "arm64",
	.cpuid = "0x00000000410fd050",
	.event_table = {
		.pmus = pmu_events__arm_cortex_a55,
		.num_pmus = ARRAY_SIZE(pmu_events__arm_cortex_a55)
	},
	.metric_table = {
		.pmus = NULL,
		.num_pmus = 0
	}
},

static const struct pmu_sys_events pmu_sys_event_tables[] = {
{
	.event_table = {
		.pmus = pmu_events__freescale_imx95_sys,
		.num_pmus = ARRAY_SIZE(pmu_events__freescale_imx95_sys)
	},
	.metric_table = {
		.pmus = pmu_metrics__freescale_imx95_sys,
		.num_pmus = ARRAY_SIZE(pmu_metrics__freescale_imx95_sys)
	},
	.name = "pmu_events__freescale_imx95_sys",
},

Currently, pmu_metrics_table__find() will return NULL when only do perf
stat on sys metric. Then parse_groups() will never be called to parse
sys metric_name, finally perf tool will exit directly. This should be a
common problem. To fix the issue, this will keep the logic before commit
f20c15d13f01 ("perf pmu-events: Remember the perf_events_map for a PMU")
to return a empty metric table rather than a NULL pointer. This should
be fine since the removed part just check if the table match provided
metric_name. Without these code, the code in parse_groups() will also
check the validity of metrci_name too.

Fixes: f20c15d13f01 ("perf pmu-events: Remember the perf_events_map for a PMU")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
Acked-by: Ian Rogers <irogers@google.com>
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/pmu-events/empty-pmu-events.c | 12 +-----------
 tools/perf/pmu-events/jevents.py         | 12 +-----------
 2 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
index 2b7516946ded..b8719dab264d 100644
--- a/tools/perf/pmu-events/empty-pmu-events.c
+++ b/tools/perf/pmu-events/empty-pmu-events.c
@@ -585,17 +585,7 @@ const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pm
         if (!map)
                 return NULL;
 
-        if (!pmu)
-                return &map->metric_table;
-
-        for (size_t i = 0; i < map->metric_table.num_pmus; i++) {
-                const struct pmu_table_entry *table_pmu = &map->metric_table.pmus[i];
-                const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
-
-                if (pmu__name_match(pmu, pmu_name))
-                           return &map->metric_table;
-        }
-        return NULL;
+	return &map->metric_table;
 }
 
 const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 6e71b09dbc2a..70f4fd5395fb 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -1101,17 +1101,7 @@ const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pm
         if (!map)
                 return NULL;
 
-        if (!pmu)
-                return &map->metric_table;
-
-        for (size_t i = 0; i < map->metric_table.num_pmus; i++) {
-                const struct pmu_table_entry *table_pmu = &map->metric_table.pmus[i];
-                const char *pmu_name = &big_c_string[table_pmu->pmu_name.offset];
-
-                if (pmu__name_match(pmu, pmu_name))
-                           return &map->metric_table;
-        }
-        return NULL;
+	return &map->metric_table;
 }
 
 const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 2/8] perf header: Move is_cpu_online to numa bench

[Ian Rogers]

The helper function is only used in the NUMA benchmark as typically
online CPUs are determined through
perf_cpu_map__new_online_cpus(). Reduce the scope of the function for
now.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/bench/numa.c  | 53 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/header.c | 51 --------------------------------------
 tools/perf/util/header.h |  1 -
 3 files changed, 53 insertions(+), 52 deletions(-)

diff --git a/tools/perf/bench/numa.c b/tools/perf/bench/numa.c
index 1fbd7c947abc..19be2aaf4dc0 100644
--- a/tools/perf/bench/numa.c
+++ b/tools/perf/bench/numa.c
@@ -27,6 +27,7 @@
 #include <sys/resource.h>
 #include <sys/wait.h>
 #include <sys/prctl.h>
+#include <sys/stat.h>
 #include <sys/types.h>
 #include <linux/kernel.h>
 #include <linux/time64.h>
@@ -35,6 +36,7 @@
 
 #include "../util/header.h"
 #include "../util/mutex.h"
+#include <api/fs/fs.h>
 #include <numa.h>
 #include <numaif.h>
 
@@ -533,6 +535,57 @@ static int parse_cpu_list(const char *arg)
 	return 0;
 }
 
+/*
+ * Check whether a CPU is online
+ *
+ * Returns:
+ *     1 -> if CPU is online
+ *     0 -> if CPU is offline
+ *    -1 -> error case
+ */
+static int is_cpu_online(unsigned int cpu)
+{
+	char *str;
+	size_t strlen;
+	char buf[256];
+	int status = -1;
+	struct stat statbuf;
+
+	snprintf(buf, sizeof(buf),
+		"/sys/devices/system/cpu/cpu%d", cpu);
+	if (stat(buf, &statbuf) != 0)
+		return 0;
+
+	/*
+	 * Check if /sys/devices/system/cpu/cpux/online file
+	 * exists. Some cases cpu0 won't have online file since
+	 * it is not expected to be turned off generally.
+	 * In kernels without CONFIG_HOTPLUG_CPU, this
+	 * file won't exist
+	 */
+	snprintf(buf, sizeof(buf),
+		"/sys/devices/system/cpu/cpu%d/online", cpu);
+	if (stat(buf, &statbuf) != 0)
+		return 1;
+
+	/*
+	 * Read online file using sysfs__read_str.
+	 * If read or open fails, return -1.
+	 * If read succeeds, return value from file
+	 * which gets stored in "str"
+	 */
+	snprintf(buf, sizeof(buf),
+		"devices/system/cpu/cpu%d/online", cpu);
+
+	if (sysfs__read_str(buf, &str, &strlen) < 0)
+		return status;
+
+	status = atoi(str);
+
+	free(str);
+	return status;
+}
+
 static int parse_setup_cpu_list(void)
 {
 	struct thread_data *td;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index a6386d12afd7..55c28e4377ec 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -987,57 +987,6 @@ static int write_dir_format(struct feat_fd *ff,
 	return do_write(ff, &data->dir.version, sizeof(data->dir.version));
 }
 
-/*
- * Check whether a CPU is online
- *
- * Returns:
- *     1 -> if CPU is online
- *     0 -> if CPU is offline
- *    -1 -> error case
- */
-int is_cpu_online(unsigned int cpu)
-{
-	char *str;
-	size_t strlen;
-	char buf[256];
-	int status = -1;
-	struct stat statbuf;
-
-	snprintf(buf, sizeof(buf),
-		"/sys/devices/system/cpu/cpu%d", cpu);
-	if (stat(buf, &statbuf) != 0)
-		return 0;
-
-	/*
-	 * Check if /sys/devices/system/cpu/cpux/online file
-	 * exists. Some cases cpu0 won't have online file since
-	 * it is not expected to be turned off generally.
-	 * In kernels without CONFIG_HOTPLUG_CPU, this
-	 * file won't exist
-	 */
-	snprintf(buf, sizeof(buf),
-		"/sys/devices/system/cpu/cpu%d/online", cpu);
-	if (stat(buf, &statbuf) != 0)
-		return 1;
-
-	/*
-	 * Read online file using sysfs__read_str.
-	 * If read or open fails, return -1.
-	 * If read succeeds, return value from file
-	 * which gets stored in "str"
-	 */
-	snprintf(buf, sizeof(buf),
-		"devices/system/cpu/cpu%d/online", cpu);
-
-	if (sysfs__read_str(buf, &str, &strlen) < 0)
-		return status;
-
-	status = atoi(str);
-
-	free(str);
-	return status;
-}
-
 #ifdef HAVE_LIBBPF_SUPPORT
 static int write_bpf_prog_info(struct feat_fd *ff,
 			       struct evlist *evlist __maybe_unused)
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index a63a361f20f4..e91e89d22537 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -196,7 +196,6 @@ int write_padded(struct feat_fd *fd, const void *bf,
 
 #define MAX_CACHE_LVL 4
 
-int is_cpu_online(unsigned int cpu);
 int build_caches_for_cpu(u32 cpu, struct cpu_cache_level caches[], u32 *cntp);
 
 /*
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 3/8] perf header: Refactor get_cpuid to take a CPU for ARM

[Ian Rogers]

ARM BIG.little has no notion of a constant CPUID for both core
types. To reflect this reality, change the get_cpuid function to also
pass in a possibly unused logical cpu. If the dummy value (-1) is
passed in then ARM can, as currently happens, select the first logical
CPU's "CPUID". The changes to ARM getcpuid happen in a follow up
change.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/arch/arm64/util/header.c     | 2 +-
 tools/perf/arch/loongarch/util/header.c | 2 +-
 tools/perf/arch/powerpc/util/header.c   | 2 +-
 tools/perf/arch/riscv/util/header.c     | 2 +-
 tools/perf/arch/s390/util/header.c      | 2 +-
 tools/perf/arch/x86/util/auxtrace.c     | 3 ++-
 tools/perf/arch/x86/util/header.c       | 2 +-
 tools/perf/builtin-kvm.c                | 4 +++-
 tools/perf/util/env.c                   | 3 ++-
 tools/perf/util/header.c                | 9 +++++----
 tools/perf/util/header.h                | 2 +-
 11 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
index 741df3614a09..760c21784713 100644
--- a/tools/perf/arch/arm64/util/header.c
+++ b/tools/perf/arch/arm64/util/header.c
@@ -50,7 +50,7 @@ static int _get_cpuid(char *buf, size_t sz, struct perf_cpu_map *cpus)
 	return ret;
 }
 
-int get_cpuid(char *buf, size_t sz)
+int get_cpuid(char *buf, size_t sz, struct perf_cpu cpu __maybe_unused)
 {
 	struct perf_cpu_map *cpus = perf_cpu_map__new_online_cpus();
 	int ret;
diff --git a/tools/perf/arch/loongarch/util/header.c b/tools/perf/arch/loongarch/util/header.c
index d962dff55512..f1f0b116962d 100644
--- a/tools/perf/arch/loongarch/util/header.c
+++ b/tools/perf/arch/loongarch/util/header.c
@@ -70,7 +70,7 @@ static char *_get_cpuid(void)
 	return cpuid;
 }
 
-int get_cpuid(char *buffer, size_t sz)
+int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 {
 	int ret = 0;
 	char *cpuid = _get_cpuid();
diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 251cdbf58a4d..6d1a63a2922f 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -24,7 +24,7 @@ static bool is_compat_mode(void)
 }
 
 int
-get_cpuid(char *buffer, size_t sz)
+get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 {
 	unsigned long pvr;
 	int nb;
diff --git a/tools/perf/arch/riscv/util/header.c b/tools/perf/arch/riscv/util/header.c
index 1b29030021ee..ebac294c877f 100644
--- a/tools/perf/arch/riscv/util/header.c
+++ b/tools/perf/arch/riscv/util/header.c
@@ -81,7 +81,7 @@ static char *_get_cpuid(void)
 	return cpuid;
 }
 
-int get_cpuid(char *buffer, size_t sz)
+int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 {
 	char *cpuid = _get_cpuid();
 	int ret = 0;
diff --git a/tools/perf/arch/s390/util/header.c b/tools/perf/arch/s390/util/header.c
index 7933f6871c81..2add1a561242 100644
--- a/tools/perf/arch/s390/util/header.c
+++ b/tools/perf/arch/s390/util/header.c
@@ -27,7 +27,7 @@
 #define SYSINFO		"/proc/sysinfo"
 #define SRVLVL		"/proc/service_levels"
 
-int get_cpuid(char *buffer, size_t sz)
+int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 {
 	char *cp, *line = NULL, *line2;
 	char type[8], model[33], version[8], manufacturer[32], authorization[8];
diff --git a/tools/perf/arch/x86/util/auxtrace.c b/tools/perf/arch/x86/util/auxtrace.c
index 354780ff1605..ecbf61a7eb3a 100644
--- a/tools/perf/arch/x86/util/auxtrace.c
+++ b/tools/perf/arch/x86/util/auxtrace.c
@@ -55,11 +55,12 @@ struct auxtrace_record *auxtrace_record__init(struct evlist *evlist,
 					      int *err)
 {
 	char buffer[64];
+	struct perf_cpu cpu = perf_cpu_map__min(evlist->core.all_cpus);
 	int ret;
 
 	*err = 0;
 
-	ret = get_cpuid(buffer, sizeof(buffer));
+	ret = get_cpuid(buffer, sizeof(buffer), cpu);
 	if (ret) {
 		*err = ret;
 		return NULL;
diff --git a/tools/perf/arch/x86/util/header.c b/tools/perf/arch/x86/util/header.c
index a51444a77a5f..690f86cbbb1c 100644
--- a/tools/perf/arch/x86/util/header.c
+++ b/tools/perf/arch/x86/util/header.c
@@ -58,7 +58,7 @@ __get_cpuid(char *buffer, size_t sz, const char *fmt)
 }
 
 int
-get_cpuid(char *buffer, size_t sz)
+get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 {
 	return __get_cpuid(buffer, sz, "%s,%u,%u,%u$");
 }
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 099ce3ebf67c..274568d712d1 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1226,7 +1226,9 @@ static int cpu_isa_config(struct perf_kvm_stat *kvm)
 	int err;
 
 	if (kvm->live) {
-		err = get_cpuid(buf, sizeof(buf));
+		struct perf_cpu cpu = {-1};
+
+		err = get_cpuid(buf, sizeof(buf), cpu);
 		if (err != 0) {
 			pr_err("Failed to look up CPU type: %s\n",
 			       str_error_r(err, buf, sizeof(buf)));
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index ccb464026642..93cabdd7ff43 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -373,7 +373,8 @@ int perf_env__read_pmu_mappings(struct perf_env *env)
 int perf_env__read_cpuid(struct perf_env *env)
 {
 	char cpuid[128];
-	int err = get_cpuid(cpuid, sizeof(cpuid));
+	struct perf_cpu cpu = {-1};
+	int err = get_cpuid(cpuid, sizeof(cpuid), cpu);
 
 	if (err)
 		return err;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 55c28e4377ec..60b42ddc3dac 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -856,18 +856,19 @@ int __weak strcmp_cpuid_str(const char *mapcpuid, const char *cpuid)
  * default get_cpuid(): nothing gets recorded
  * actual implementation must be in arch/$(SRCARCH)/util/header.c
  */
-int __weak get_cpuid(char *buffer __maybe_unused, size_t sz __maybe_unused)
+int __weak get_cpuid(char *buffer __maybe_unused, size_t sz __maybe_unused,
+		     struct perf_cpu cpu __maybe_unused)
 {
 	return ENOSYS; /* Not implemented */
 }
 
-static int write_cpuid(struct feat_fd *ff,
-		       struct evlist *evlist __maybe_unused)
+static int write_cpuid(struct feat_fd *ff, struct evlist *evlist)
 {
+	struct perf_cpu cpu = perf_cpu_map__min(evlist->core.all_cpus);
 	char buffer[64];
 	int ret;
 
-	ret = get_cpuid(buffer, sizeof(buffer));
+	ret = get_cpuid(buffer, sizeof(buffer), cpu);
 	if (ret)
 		return -1;
 
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index e91e89d22537..b77f89de12dd 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -201,7 +201,7 @@ int build_caches_for_cpu(u32 cpu, struct cpu_cache_level caches[], u32 *cntp);
 /*
  * arch specific callback
  */
-int get_cpuid(char *buffer, size_t sz);
+int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu);
 
 char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused);
 int strcmp_cpuid_str(const char *s1, const char *s2);
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 4/8] perf arm64 header: Use cpu argument in get_cpuid

[Ian Rogers]

Use the cpu to read the MIDR file requested. If the "any" value (-1)
is passed that keep the behavior of returning the first MIDR file that
can be read.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/arch/arm64/util/header.c | 63 ++++++++++++++---------------
 tools/perf/util/header.h            |  1 +
 2 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
index 760c21784713..f0907daad3ae 100644
--- a/tools/perf/arch/arm64/util/header.c
+++ b/tools/perf/arch/arm64/util/header.c
@@ -14,55 +14,52 @@
 #define MIDR_REVISION_MASK      GENMASK(3, 0)
 #define MIDR_VARIANT_MASK	GENMASK(23, 20)
 
-static int _get_cpuid(char *buf, size_t sz, struct perf_cpu_map *cpus)
+static int _get_cpuid(char *buf, size_t sz, struct perf_cpu cpu)
 {
+	char path[PATH_MAX];
+	FILE *file;
 	const char *sysfs = sysfs__mountpoint();
-	struct perf_cpu cpu;
-	int idx, ret = EINVAL;
 
+	assert(cpu.cpu != -1);
 	if (!sysfs || sz < MIDR_SIZE)
 		return EINVAL;
 
-	perf_cpu_map__for_each_cpu(cpu, idx, cpus) {
-		char path[PATH_MAX];
-		FILE *file;
-
-		scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d" MIDR,
-			  sysfs, cpu.cpu);
-
-		file = fopen(path, "r");
-		if (!file) {
-			pr_debug("fopen failed for file %s\n", path);
-			continue;
-		}
-
-		if (!fgets(buf, MIDR_SIZE, file)) {
-			fclose(file);
-			continue;
-		}
-		fclose(file);
+	scnprintf(path, PATH_MAX, "%s/devices/system/cpu/cpu%d" MIDR, sysfs, cpu.cpu);
 
-		/* got midr break loop */
-		ret = 0;
-		break;
+	file = fopen(path, "r");
+	if (!file) {
+		pr_debug("fopen failed for file %s\n", path);
+		return EINVAL;
 	}
 
-	return ret;
+	if (!fgets(buf, MIDR_SIZE, file)) {
+		pr_debug("Failed to read file %s\n", path);
+		fclose(file);
+		return EINVAL;
+	}
+	fclose(file);
+	return 0;
 }
 
-int get_cpuid(char *buf, size_t sz, struct perf_cpu cpu __maybe_unused)
+int get_cpuid(char *buf, size_t sz, struct perf_cpu cpu)
 {
-	struct perf_cpu_map *cpus = perf_cpu_map__new_online_cpus();
-	int ret;
+	struct perf_cpu_map *cpus;
+	int idx;
+
+	if (cpu.cpu != -1)
+		return _get_cpuid(buf, sz, cpu);
 
+	cpus = perf_cpu_map__new_online_cpus();
 	if (!cpus)
 		return EINVAL;
 
-	ret = _get_cpuid(buf, sz, cpus);
-
-	perf_cpu_map__put(cpus);
+	perf_cpu_map__for_each_cpu(cpu, idx, cpus) {
+		int ret = _get_cpuid(buf, sz, cpu);
 
-	return ret;
+		if (ret == 0)
+			return 0;
+	}
+	return EINVAL;
 }
 
 char *get_cpuid_str(struct perf_pmu *pmu)
@@ -78,7 +75,7 @@ char *get_cpuid_str(struct perf_pmu *pmu)
 		return NULL;
 
 	/* read midr from list of cpus mapped to this pmu */
-	res = _get_cpuid(buf, MIDR_SIZE, pmu->cpus);
+	res = get_cpuid(buf, MIDR_SIZE, perf_cpu_map__min(pmu->cpus));
 	if (res) {
 		pr_err("failed to get cpuid string for PMU %s\n", pmu->name);
 		free(buf);
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index b77f89de12dd..3bb768455a60 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -11,6 +11,7 @@
 #include <linux/types.h>
 #include "env.h"
 #include "pmu.h"
+#include <perf/cpumap.h>
 
 enum {
 	HEADER_RESERVED		= 0,	/* always cleared */
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 5/8] perf header: Avoid transitive PMU includes

[Ian Rogers]

Currently satisfied via header.h. Note, pmu.h includes parse-events.h.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/expr.c       | 2 ++
 tools/perf/util/env.c         | 1 +
 tools/perf/util/probe-event.c | 1 +
 3 files changed, 4 insertions(+)

diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index e3aa9d4fcf3a..d60f1ac1d720 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -4,6 +4,8 @@
 #include "util/expr.h"
 #include "util/hashmap.h"
 #include "util/header.h"
+#include "util/pmu.h"
+#include "util/pmus.h"
 #include "util/smt.h"
 #include "tests.h"
 #include <math.h>
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 93cabdd7ff43..e2843ca2edd9 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -12,6 +12,7 @@
 #include <sys/utsname.h>
 #include <stdlib.h>
 #include <string.h>
+#include "pmu.h"
 #include "pmus.h"
 #include "strbuf.h"
 #include "trace/beauty/beauty.h"
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index a580aa2cd512..d399c7e03341 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -40,6 +40,7 @@
 #include "session.h"
 #include "string2.h"
 #include "strbuf.h"
+#include "parse-events.h"
 
 #include <subcmd/pager.h>
 #include <linux/ctype.h>
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 6/8] perf header: Pass a perf_cpu rather than a PMU to get_cpuid_str

[Ian Rogers]

On ARM the cpuid is dependent on the core type of the CPU in
question. The PMU was passed for the sake of the CPU map but this
means in places a temporary PMU is created just to pass a CPU
value. Just pass the CPU and fix up the callers.

As there are no longer PMU users in header.h, shuffle forward
declarations earlier to work around build failures.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/arch/arm64/util/arm-spe.c     | 14 ++------------
 tools/perf/arch/arm64/util/header.c      | 12 ++++--------
 tools/perf/arch/loongarch/util/header.c  |  2 +-
 tools/perf/arch/powerpc/util/header.c    |  2 +-
 tools/perf/arch/riscv/util/header.c      |  2 +-
 tools/perf/arch/s390/util/header.c       |  4 ++--
 tools/perf/arch/x86/util/header.c        |  3 +--
 tools/perf/pmu-events/empty-pmu-events.c |  5 ++++-
 tools/perf/pmu-events/jevents.py         |  5 ++++-
 tools/perf/tests/expr.c                  |  7 +++----
 tools/perf/util/expr.c                   |  6 +++---
 tools/perf/util/header.c                 | 22 +++++++++++++++++++++-
 tools/perf/util/header.h                 | 19 ++++++++++---------
 tools/perf/util/pmu.c                    | 20 --------------------
 tools/perf/util/pmu.h                    |  1 -
 15 files changed, 57 insertions(+), 67 deletions(-)

diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c
index 7ccdea3cd461..22b19dcc6beb 100644
--- a/tools/perf/arch/arm64/util/arm-spe.c
+++ b/tools/perf/arch/arm64/util/arm-spe.c
@@ -23,6 +23,7 @@
 #include "../../../util/debug.h"
 #include "../../../util/auxtrace.h"
 #include "../../../util/record.h"
+#include "../../../util/header.h"
 #include "../../../util/arm-spe.h"
 #include <tools/libc_compat.h> // reallocarray
 
@@ -85,22 +86,11 @@ static int arm_spe_save_cpu_header(struct auxtrace_record *itr,
 	struct arm_spe_recording *sper =
 			container_of(itr, struct arm_spe_recording, itr);
 	struct perf_pmu *pmu = NULL;
-	struct perf_pmu tmp_pmu;
-	char cpu_id_str[16];
 	char *cpuid = NULL;
 	u64 val;
 
-	snprintf(cpu_id_str, sizeof(cpu_id_str), "%d", cpu.cpu);
-	tmp_pmu.cpus = perf_cpu_map__new(cpu_id_str);
-	if (!tmp_pmu.cpus)
-		return -ENOMEM;
-
 	/* Read CPU MIDR */
-	cpuid = perf_pmu__getcpuid(&tmp_pmu);
-
-	/* The CPU map will not be used anymore, release it */
-	perf_cpu_map__put(tmp_pmu.cpus);
-
+	cpuid = get_cpuid_allow_env_override(cpu);
 	if (!cpuid)
 		return -ENOMEM;
 	val = strtol(cpuid, NULL, 16);
diff --git a/tools/perf/arch/arm64/util/header.c b/tools/perf/arch/arm64/util/header.c
index f0907daad3ae..f445a2dd6293 100644
--- a/tools/perf/arch/arm64/util/header.c
+++ b/tools/perf/arch/arm64/util/header.c
@@ -62,22 +62,18 @@ int get_cpuid(char *buf, size_t sz, struct perf_cpu cpu)
 	return EINVAL;
 }
 
-char *get_cpuid_str(struct perf_pmu *pmu)
+char *get_cpuid_str(struct perf_cpu cpu)
 {
-	char *buf = NULL;
+	char *buf = malloc(MIDR_SIZE);
 	int res;
 
-	if (!pmu || !pmu->cpus)
-		return NULL;
-
-	buf = malloc(MIDR_SIZE);
 	if (!buf)
 		return NULL;
 
 	/* read midr from list of cpus mapped to this pmu */
-	res = get_cpuid(buf, MIDR_SIZE, perf_cpu_map__min(pmu->cpus));
+	res = get_cpuid(buf, MIDR_SIZE, cpu);
 	if (res) {
-		pr_err("failed to get cpuid string for PMU %s\n", pmu->name);
+		pr_err("failed to get cpuid string for CPU %d\n", cpu.cpu);
 		free(buf);
 		buf = NULL;
 	}
diff --git a/tools/perf/arch/loongarch/util/header.c b/tools/perf/arch/loongarch/util/header.c
index f1f0b116962d..0c6d823334a2 100644
--- a/tools/perf/arch/loongarch/util/header.c
+++ b/tools/perf/arch/loongarch/util/header.c
@@ -90,7 +90,7 @@ int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 	return ret;
 }
 
-char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
+char *get_cpuid_str(struct perf_cpu cpu __maybe_unused)
 {
 	return _get_cpuid();
 }
diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index 6d1a63a2922f..c7df534dbf8f 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -42,7 +42,7 @@ get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 }
 
 char *
-get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
+get_cpuid_str(struct perf_cpu cpu __maybe_unused)
 {
 	char *bufp;
 	unsigned long pvr;
diff --git a/tools/perf/arch/riscv/util/header.c b/tools/perf/arch/riscv/util/header.c
index ebac294c877f..4b839203d4a5 100644
--- a/tools/perf/arch/riscv/util/header.c
+++ b/tools/perf/arch/riscv/util/header.c
@@ -98,7 +98,7 @@ int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 }
 
 char *
-get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
+get_cpuid_str(struct perf_cpu cpu __maybe_unused)
 {
 	return _get_cpuid();
 }
diff --git a/tools/perf/arch/s390/util/header.c b/tools/perf/arch/s390/util/header.c
index 2add1a561242..db54677a17d2 100644
--- a/tools/perf/arch/s390/util/header.c
+++ b/tools/perf/arch/s390/util/header.c
@@ -137,11 +137,11 @@ int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 	return (nbytes >= sz) ? ENOBUFS : 0;
 }
 
-char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
+char *get_cpuid_str(struct perf_cpu cpu)
 {
 	char *buf = malloc(128);
 
-	if (buf && get_cpuid(buf, 128))
+	if (buf && get_cpuid(buf, 128, cpu))
 		zfree(&buf);
 	return buf;
 }
diff --git a/tools/perf/arch/x86/util/header.c b/tools/perf/arch/x86/util/header.c
index 690f86cbbb1c..412977f8aa83 100644
--- a/tools/perf/arch/x86/util/header.c
+++ b/tools/perf/arch/x86/util/header.c
@@ -63,8 +63,7 @@ get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu __maybe_unused)
 	return __get_cpuid(buffer, sz, "%s,%u,%u,%u$");
 }
 
-char *
-get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
+char *get_cpuid_str(struct perf_cpu cpu __maybe_unused)
 {
 	char *buf = malloc(128);
 
diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
index b8719dab264d..51ca8d61c15d 100644
--- a/tools/perf/pmu-events/empty-pmu-events.c
+++ b/tools/perf/pmu-events/empty-pmu-events.c
@@ -515,13 +515,16 @@ static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
         } last_map_search;
         static bool has_last_result, has_last_map_search;
         const struct pmu_events_map *map = NULL;
+        struct perf_cpu cpu = {-1};
         char *cpuid = NULL;
         size_t i;
 
         if (has_last_result && last_result.pmu == pmu)
                 return last_result.map;
 
-        cpuid = perf_pmu__getcpuid(pmu);
+        if (pmu)
+                cpu = perf_cpu_map__min(pmu->cpus);
+        cpuid = get_cpuid_allow_env_override(cpu);
 
         /*
          * On some platforms which uses cpus map, cpuid can be NULL for
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 70f4fd5395fb..b5ff872e2beb 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -1031,13 +1031,16 @@ static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
         } last_map_search;
         static bool has_last_result, has_last_map_search;
         const struct pmu_events_map *map = NULL;
+        struct perf_cpu cpu = {-1};
         char *cpuid = NULL;
         size_t i;
 
         if (has_last_result && last_result.pmu == pmu)
                 return last_result.map;
 
-        cpuid = perf_pmu__getcpuid(pmu);
+        if (pmu)
+                cpu = perf_cpu_map__min(pmu->cpus);
+        cpuid = get_cpuid_allow_env_override(cpu);
 
         /*
          * On some platforms which uses cpus map, cpuid can be NULL for
diff --git a/tools/perf/tests/expr.c b/tools/perf/tests/expr.c
index d60f1ac1d720..41ff1affdfcd 100644
--- a/tools/perf/tests/expr.c
+++ b/tools/perf/tests/expr.c
@@ -4,10 +4,9 @@
 #include "util/expr.h"
 #include "util/hashmap.h"
 #include "util/header.h"
-#include "util/pmu.h"
-#include "util/pmus.h"
 #include "util/smt.h"
 #include "tests.h"
+#include <perf/cpumap.h>
 #include <math.h>
 #include <stdlib.h>
 #include <string.h>
@@ -78,8 +77,8 @@ static int test__expr(struct test_suite *t __maybe_unused, int subtest __maybe_u
 	struct expr_parse_ctx *ctx;
 	bool is_intel = false;
 	char strcmp_cpuid_buf[256];
-	struct perf_pmu *pmu = perf_pmus__find_core_pmu();
-	char *cpuid = perf_pmu__getcpuid(pmu);
+	struct perf_cpu cpu = {-1};
+	char *cpuid = get_cpuid_allow_env_override(cpu);
 	char *escaped_cpuid1, *escaped_cpuid2;
 
 	TEST_ASSERT_VAL("get_cpuid", cpuid);
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index 5e3732bc2fa5..f289044a1f7c 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -8,7 +8,6 @@
 #include "debug.h"
 #include "evlist.h"
 #include "expr.h"
-#include "pmu.h"
 #include "smt.h"
 #include "tool_pmu.h"
 #include <util/expr-bison.h>
@@ -16,6 +15,7 @@
 #include "util/hashmap.h"
 #include "util/header.h"
 #include "util/pmu.h"
+#include <perf/cpumap.h>
 #include <linux/err.h>
 #include <linux/kernel.h>
 #include <linux/zalloc.h>
@@ -456,8 +456,8 @@ double expr__strcmp_cpuid_str(const struct expr_parse_ctx *ctx __maybe_unused,
 		       bool compute_ids __maybe_unused, const char *test_id)
 {
 	double ret;
-	struct perf_pmu *pmu = perf_pmus__find_core_pmu();
-	char *cpuid = perf_pmu__getcpuid(pmu);
+	struct perf_cpu cpu = {-1};
+	char *cpuid = get_cpuid_allow_env_override(cpu);
 
 	if (!cpuid)
 		return NAN;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 60b42ddc3dac..06ca5762f61f 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -819,11 +819,31 @@ static int write_group_desc(struct feat_fd *ff,
  * Each architecture should provide a more precise id string that
  * can be use to match the architecture's "mapfile".
  */
-char * __weak get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
+char * __weak get_cpuid_str(struct perf_cpu cpu __maybe_unused)
 {
 	return NULL;
 }
 
+char *get_cpuid_allow_env_override(struct perf_cpu cpu)
+{
+	char *cpuid;
+	static bool printed;
+
+	cpuid = getenv("PERF_CPUID");
+	if (cpuid)
+		cpuid = strdup(cpuid);
+	if (!cpuid)
+		cpuid = get_cpuid_str(cpu);
+	if (!cpuid)
+		return NULL;
+
+	if (!printed) {
+		pr_debug("Using CPUID %s\n", cpuid);
+		printed = true;
+	}
+	return cpuid;
+}
+
 /* Return zero when the cpuid from the mapfile.csv matches the
  * cpuid string generated on this platform.
  * Otherwise return non-zero.
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 3bb768455a60..5201af6305f4 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -10,9 +10,14 @@
 #include <linux/bitmap.h>
 #include <linux/types.h>
 #include "env.h"
-#include "pmu.h"
 #include <perf/cpumap.h>
 
+struct evlist;
+union perf_event;
+struct perf_header;
+struct perf_session;
+struct perf_tool;
+
 enum {
 	HEADER_RESERVED		= 0,	/* always cleared */
 	HEADER_FIRST_FEATURE	= 1,
@@ -92,8 +97,6 @@ struct perf_pipe_file_header {
 	u64				size;
 };
 
-struct perf_header;
-
 int perf_file_header__read(struct perf_file_header *header,
 			   struct perf_header *ph, int fd);
 
@@ -125,11 +128,6 @@ struct perf_header_feature_ops {
 	bool	   synthesize;
 };
 
-struct evlist;
-struct perf_session;
-struct perf_tool;
-union perf_event;
-
 extern const char perf_version_string[];
 
 int perf_session__read_header(struct perf_session *session);
@@ -204,6 +202,9 @@ int build_caches_for_cpu(u32 cpu, struct cpu_cache_level caches[], u32 *cntp);
  */
 int get_cpuid(char *buffer, size_t sz, struct perf_cpu cpu);
 
-char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused);
+char *get_cpuid_str(struct perf_cpu cpu);
+
+char *get_cpuid_allow_env_override(struct perf_cpu cpu);
+
 int strcmp_cpuid_str(const char *s1, const char *s2);
 #endif /* __PERF_HEADER_H */
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 0789758598c0..514cb865f57b 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -818,26 +818,6 @@ static int is_sysfs_pmu_core(const char *name)
 	return file_available(path);
 }
 
-char *perf_pmu__getcpuid(struct perf_pmu *pmu)
-{
-	char *cpuid;
-	static bool printed;
-
-	cpuid = getenv("PERF_CPUID");
-	if (cpuid)
-		cpuid = strdup(cpuid);
-	if (!cpuid)
-		cpuid = get_cpuid_str(pmu);
-	if (!cpuid)
-		return NULL;
-
-	if (!printed) {
-		pr_debug("Using CPUID %s\n", cpuid);
-		printed = true;
-	}
-	return cpuid;
-}
-
 __weak const struct pmu_metrics_table *pmu_metrics_table__find(void)
 {
 	return perf_pmu__find_metrics_table(NULL);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index b86b3c3685a2..fba3fc608b64 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -260,7 +260,6 @@ void perf_pmu__arch_init(struct perf_pmu *pmu);
 void pmu_add_cpu_aliases_table(struct perf_pmu *pmu,
 			       const struct pmu_events_table *table);
 
-char *perf_pmu__getcpuid(struct perf_pmu *pmu);
 const struct pmu_metrics_table *pmu_metrics_table__find(void);
 bool pmu_uncore_identifier_match(const char *compat, const char *id);
 
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 7/8] perf jevents: Add map_for_cpu

[Ian Rogers]

The PMU is no longer part of the map finding process and for metrics
doesn't make sense as they lack a PMU.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/pmu-events/empty-pmu-events.c | 20 +++++++++++++-------
 tools/perf/pmu-events/jevents.py         | 20 +++++++++++++-------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
index 51ca8d61c15d..17306e316a3c 100644
--- a/tools/perf/pmu-events/empty-pmu-events.c
+++ b/tools/perf/pmu-events/empty-pmu-events.c
@@ -503,11 +503,11 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table,
         return 0;
 }
 
-static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
+static const struct pmu_events_map *map_for_cpu(struct perf_cpu cpu)
 {
         static struct {
                 const struct pmu_events_map *map;
-                struct perf_pmu *pmu;
+                struct perf_cpu cpu;
         } last_result;
         static struct {
                 const struct pmu_events_map *map;
@@ -515,15 +515,12 @@ static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
         } last_map_search;
         static bool has_last_result, has_last_map_search;
         const struct pmu_events_map *map = NULL;
-        struct perf_cpu cpu = {-1};
         char *cpuid = NULL;
         size_t i;
 
-        if (has_last_result && last_result.pmu == pmu)
+        if (has_last_result && last_result.cpu.cpu == cpu.cpu)
                 return last_result.map;
 
-        if (pmu)
-                cpu = perf_cpu_map__min(pmu->cpus);
         cpuid = get_cpuid_allow_env_override(cpu);
 
         /*
@@ -555,12 +552,21 @@ static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
                has_last_map_search = true;
         }
 out_update_last_result:
-        last_result.pmu = pmu;
+        last_result.cpu = cpu;
         last_result.map = map;
         has_last_result = true;
         return map;
 }
 
+static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
+{
+        struct perf_cpu cpu = {-1};
+
+        if (pmu)
+                cpu = perf_cpu_map__min(pmu->cpus);
+        return map_for_cpu(cpu);
+}
+
 const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
 {
         const struct pmu_events_map *map = map_for_pmu(pmu);
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index b5ff872e2beb..e44b72e56ac3 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -1019,11 +1019,11 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table,
         return 0;
 }
 
-static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
+static const struct pmu_events_map *map_for_cpu(struct perf_cpu cpu)
 {
         static struct {
                 const struct pmu_events_map *map;
-                struct perf_pmu *pmu;
+                struct perf_cpu cpu;
         } last_result;
         static struct {
                 const struct pmu_events_map *map;
@@ -1031,15 +1031,12 @@ static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
         } last_map_search;
         static bool has_last_result, has_last_map_search;
         const struct pmu_events_map *map = NULL;
-        struct perf_cpu cpu = {-1};
         char *cpuid = NULL;
         size_t i;
 
-        if (has_last_result && last_result.pmu == pmu)
+        if (has_last_result && last_result.cpu.cpu == cpu.cpu)
                 return last_result.map;
 
-        if (pmu)
-                cpu = perf_cpu_map__min(pmu->cpus);
         cpuid = get_cpuid_allow_env_override(cpu);
 
         /*
@@ -1071,12 +1068,21 @@ static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
                has_last_map_search = true;
         }
 out_update_last_result:
-        last_result.pmu = pmu;
+        last_result.cpu = cpu;
         last_result.map = map;
         has_last_result = true;
         return map;
 }
 
+static const struct pmu_events_map *map_for_pmu(struct perf_pmu *pmu)
+{
+        struct perf_cpu cpu = {-1};
+
+        if (pmu)
+                cpu = perf_cpu_map__min(pmu->cpus);
+        return map_for_cpu(cpu);
+}
+
 const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
 {
         const struct pmu_events_map *map = map_for_pmu(pmu);
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v2 8/8] perf pmu: Move pmu_metrics_table__find and remove ARM override

[Ian Rogers]

Move pmu_metrics_table__find to the jevents.py generated pmu-events.c
and remove indirection override for ARM. The movement removes
perf_pmu__find_metrics_table that exists to enable the ARM
override. The ARM override isn't necessary as just the CPUID, not PMU,
is used in the metric table lookup. On non-ARM the CPU argument is
just ignored for the CPUID, for ARM -1 is passed so that the CPUID for
the first logical CPU is read.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/arch/arm64/util/pmu.c         | 20 --------------------
 tools/perf/pmu-events/empty-pmu-events.c | 10 ++++------
 tools/perf/pmu-events/jevents.py         | 10 ++++------
 tools/perf/pmu-events/pmu-events.h       |  2 +-
 tools/perf/util/pmu.c                    |  5 -----
 tools/perf/util/pmu.h                    |  1 -
 6 files changed, 9 insertions(+), 39 deletions(-)

diff --git a/tools/perf/arch/arm64/util/pmu.c b/tools/perf/arch/arm64/util/pmu.c
index a0964b191fcb..895fb0d0610c 100644
--- a/tools/perf/arch/arm64/util/pmu.c
+++ b/tools/perf/arch/arm64/util/pmu.c
@@ -1,29 +1,9 @@
 // SPDX-License-Identifier: GPL-2.0
 
-#include <internal/cpumap.h>
-#include "../../../util/cpumap.h"
-#include "../../../util/header.h"
 #include "../../../util/pmu.h"
 #include "../../../util/pmus.h"
 #include "../../../util/tool_pmu.h"
 #include <api/fs/fs.h>
-#include <math.h>
-
-const struct pmu_metrics_table *pmu_metrics_table__find(void)
-{
-	struct perf_pmu *pmu;
-
-	/* Metrics aren't currently supported on heterogeneous Arm systems */
-	if (perf_pmus__num_core_pmus() > 1)
-		return NULL;
-
-	/* Doesn't matter which one here because they'll all be the same */
-	pmu = perf_pmus__find_core_pmu();
-	if (pmu)
-		return perf_pmu__find_metrics_table(pmu);
-
-	return NULL;
-}
 
 u64 tool_pmu__cpu_slots_per_cycle(void)
 {
diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
index 17306e316a3c..1c7a2cfa321f 100644
--- a/tools/perf/pmu-events/empty-pmu-events.c
+++ b/tools/perf/pmu-events/empty-pmu-events.c
@@ -587,14 +587,12 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
         return NULL;
 }
 
-const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu)
+const struct pmu_metrics_table *pmu_metrics_table__find(void)
 {
-        const struct pmu_events_map *map = map_for_pmu(pmu);
-
-        if (!map)
-                return NULL;
+        struct perf_cpu cpu = {-1};
+        const struct pmu_events_map *map = map_for_cpu(cpu);
 
-	return &map->metric_table;
+        return map ? &map->metric_table : NULL;
 }
 
 const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index e44b72e56ac3..d781a377757a 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -1103,14 +1103,12 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
         return NULL;
 }
 
-const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu)
+const struct pmu_metrics_table *pmu_metrics_table__find(void)
 {
-        const struct pmu_events_map *map = map_for_pmu(pmu);
-
-        if (!map)
-                return NULL;
+        struct perf_cpu cpu = {-1};
+        const struct pmu_events_map *map = map_for_cpu(cpu);
 
-	return &map->metric_table;
+        return map ? &map->metric_table : NULL;
 }
 
 const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index 5435ad92180c..675562e6f770 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -103,7 +103,7 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table, pm
 				     void *data);
 
 const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu);
-const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu);
+const struct pmu_metrics_table *pmu_metrics_table__find(void);
 const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid);
 const struct pmu_metrics_table *find_core_metrics_table(const char *arch, const char *cpuid);
 int pmu_for_each_core_event(pmu_event_iter_fn fn, void *data);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 514cb865f57b..45838651b361 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -818,11 +818,6 @@ static int is_sysfs_pmu_core(const char *name)
 	return file_available(path);
 }
 
-__weak const struct pmu_metrics_table *pmu_metrics_table__find(void)
-{
-	return perf_pmu__find_metrics_table(NULL);
-}
-
 /**
  * Return the length of the PMU name not including the suffix for uncore PMUs.
  *
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index fba3fc608b64..7b3e71194e49 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -260,7 +260,6 @@ void perf_pmu__arch_init(struct perf_pmu *pmu);
 void pmu_add_cpu_aliases_table(struct perf_pmu *pmu,
 			       const struct pmu_events_table *table);
 
-const struct pmu_metrics_table *pmu_metrics_table__find(void);
 bool pmu_uncore_identifier_match(const char *compat, const char *id);
 
 int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
-- 
2.47.0.199.ga7371fff76-goog

Re: [PATCH v2 0/8] Refactor cpuid and metric table lookup code

[Xu Yang]

On Thu, Nov 07, 2024 at 08:20:27AM -0800, Ian Rogers wrote:
> Xu Yang <xu.yang_2@nxp.com> reported issues with the system metric
> lookup:
> https://lore.kernel.org/linux-perf-users/20241106085441.3945502-1-xu.yang_2@nxp.com/
> These patches remove a lot of the logic relating CPUIDs to PMUs so
> that the PMU isn't part of the question when finding a metric table.
> For time reasons, it doesn't go as far as allowing system metrics
> without a metric table as a metric table is needed for metrics to
> refer to other metrics, and the refactoring of that resolution is a
> hassle.

For this patchset:
Tested-by: Xu Yang <xu.yang_2@nxp.com>

Thanks,
Xu Yang

> 
> Ian Rogers (7):
>   perf header: Move is_cpu_online to numa bench
>   perf header: Refactor get_cpuid to take a CPU for ARM
>   perf arm64 header: Use cpu argument in get_cpuid
>   perf header: Avoid transitive PMU includes
>   perf header: Pass a perf_cpu rather than a PMU to get_cpuid_str
>   perf jevents: Add map_for_cpu
>   perf pmu: Move pmu_metrics_table__find and remove ARM override
> 
> Xu Yang (1):
>   perf jevents: fix breakage when do perf stat on system metric
> 
>  tools/perf/arch/arm64/util/arm-spe.c     | 14 +---
>  tools/perf/arch/arm64/util/header.c      | 73 ++++++++++-----------
>  tools/perf/arch/arm64/util/pmu.c         | 20 ------
>  tools/perf/arch/loongarch/util/header.c  |  4 +-
>  tools/perf/arch/powerpc/util/header.c    |  4 +-
>  tools/perf/arch/riscv/util/header.c      |  4 +-
>  tools/perf/arch/s390/util/header.c       |  6 +-
>  tools/perf/arch/x86/util/auxtrace.c      |  3 +-
>  tools/perf/arch/x86/util/header.c        |  5 +-
>  tools/perf/bench/numa.c                  | 53 +++++++++++++++
>  tools/perf/builtin-kvm.c                 |  4 +-
>  tools/perf/pmu-events/empty-pmu-events.c | 39 ++++++-----
>  tools/perf/pmu-events/jevents.py         | 39 ++++++-----
>  tools/perf/pmu-events/pmu-events.h       |  2 +-
>  tools/perf/tests/expr.c                  |  5 +-
>  tools/perf/util/env.c                    |  4 +-
>  tools/perf/util/expr.c                   |  6 +-
>  tools/perf/util/header.c                 | 82 ++++++++----------------
>  tools/perf/util/header.h                 | 23 +++----
>  tools/perf/util/pmu.c                    | 25 --------
>  tools/perf/util/pmu.h                    |  2 -
>  tools/perf/util/probe-event.c            |  1 +
>  22 files changed, 189 insertions(+), 229 deletions(-)
> 
> -- 
> 2.47.0.199.ga7371fff76-goog
> 

Re: [PATCH v2 8/8] perf pmu: Move pmu_metrics_table__find and remove ARM override

[Yicong Yang]

Hi,

On 2024/11/8 0:20, Ian Rogers wrote:
> Move pmu_metrics_table__find to the jevents.py generated pmu-events.c
> and remove indirection override for ARM. The movement removes
> perf_pmu__find_metrics_table that exists to enable the ARM
> override. The ARM override isn't necessary as just the CPUID, not PMU,
> is used in the metric table lookup. On non-ARM the CPU argument is
> just ignored for the CPUID, for ARM -1 is passed so that the CPUID for
> the first logical CPU is read.

Since the logic here's already been touching, is it possible to step it further to just
ignore the CPUID matching when finding the system metrics/events tables? It's may not be
that reasonable for finding a system metrics/events from the CPUID, since one system PMU may
exists on different platforms with different CPU types.

FYI, there's a similiar problem when trying to count the system metrics but fails [1].
I've tested with this series but the problem still exists.

[1] https://lore.kernel.org/linux-perf-users/20241010074430.16685-1-hejunhao3@huawei.com/

Thanks.

> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/arch/arm64/util/pmu.c         | 20 --------------------
>  tools/perf/pmu-events/empty-pmu-events.c | 10 ++++------
>  tools/perf/pmu-events/jevents.py         | 10 ++++------
>  tools/perf/pmu-events/pmu-events.h       |  2 +-
>  tools/perf/util/pmu.c                    |  5 -----
>  tools/perf/util/pmu.h                    |  1 -
>  6 files changed, 9 insertions(+), 39 deletions(-)
> 
> diff --git a/tools/perf/arch/arm64/util/pmu.c b/tools/perf/arch/arm64/util/pmu.c
> index a0964b191fcb..895fb0d0610c 100644
> --- a/tools/perf/arch/arm64/util/pmu.c
> +++ b/tools/perf/arch/arm64/util/pmu.c
> @@ -1,29 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
> -#include <internal/cpumap.h>
> -#include "../../../util/cpumap.h"
> -#include "../../../util/header.h"
>  #include "../../../util/pmu.h"
>  #include "../../../util/pmus.h"
>  #include "../../../util/tool_pmu.h"
>  #include <api/fs/fs.h>
> -#include <math.h>
> -
> -const struct pmu_metrics_table *pmu_metrics_table__find(void)
> -{
> -	struct perf_pmu *pmu;
> -
> -	/* Metrics aren't currently supported on heterogeneous Arm systems */
> -	if (perf_pmus__num_core_pmus() > 1)
> -		return NULL;
> -
> -	/* Doesn't matter which one here because they'll all be the same */
> -	pmu = perf_pmus__find_core_pmu();
> -	if (pmu)
> -		return perf_pmu__find_metrics_table(pmu);
> -
> -	return NULL;
> -}
>  
>  u64 tool_pmu__cpu_slots_per_cycle(void)
>  {
> diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
> index 17306e316a3c..1c7a2cfa321f 100644
> --- a/tools/perf/pmu-events/empty-pmu-events.c
> +++ b/tools/perf/pmu-events/empty-pmu-events.c
> @@ -587,14 +587,12 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
>          return NULL;
>  }
>  
> -const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu)
> +const struct pmu_metrics_table *pmu_metrics_table__find(void)
>  {
> -        const struct pmu_events_map *map = map_for_pmu(pmu);
> -
> -        if (!map)
> -                return NULL;
> +        struct perf_cpu cpu = {-1};
> +        const struct pmu_events_map *map = map_for_cpu(cpu);
>  
> -	return &map->metric_table;
> +        return map ? &map->metric_table : NULL;
>  }
>  
>  const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
> diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> index e44b72e56ac3..d781a377757a 100755
> --- a/tools/perf/pmu-events/jevents.py
> +++ b/tools/perf/pmu-events/jevents.py
> @@ -1103,14 +1103,12 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
>          return NULL;
>  }
>  
> -const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu)
> +const struct pmu_metrics_table *pmu_metrics_table__find(void)
>  {
> -        const struct pmu_events_map *map = map_for_pmu(pmu);
> -
> -        if (!map)
> -                return NULL;
> +        struct perf_cpu cpu = {-1};
> +        const struct pmu_events_map *map = map_for_cpu(cpu);
>  
> -	return &map->metric_table;
> +        return map ? &map->metric_table : NULL;
>  }
>  
>  const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
> diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
> index 5435ad92180c..675562e6f770 100644
> --- a/tools/perf/pmu-events/pmu-events.h
> +++ b/tools/perf/pmu-events/pmu-events.h
> @@ -103,7 +103,7 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table, pm
>  				     void *data);
>  
>  const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu);
> -const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu);
> +const struct pmu_metrics_table *pmu_metrics_table__find(void);
>  const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid);
>  const struct pmu_metrics_table *find_core_metrics_table(const char *arch, const char *cpuid);
>  int pmu_for_each_core_event(pmu_event_iter_fn fn, void *data);
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 514cb865f57b..45838651b361 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -818,11 +818,6 @@ static int is_sysfs_pmu_core(const char *name)
>  	return file_available(path);
>  }
>  
> -__weak const struct pmu_metrics_table *pmu_metrics_table__find(void)
> -{
> -	return perf_pmu__find_metrics_table(NULL);
> -}
> -
>  /**
>   * Return the length of the PMU name not including the suffix for uncore PMUs.
>   *
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index fba3fc608b64..7b3e71194e49 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -260,7 +260,6 @@ void perf_pmu__arch_init(struct perf_pmu *pmu);
>  void pmu_add_cpu_aliases_table(struct perf_pmu *pmu,
>  			       const struct pmu_events_table *table);
>  
> -const struct pmu_metrics_table *pmu_metrics_table__find(void);
>  bool pmu_uncore_identifier_match(const char *compat, const char *id);
>  
>  int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
> 

Re: [PATCH v2 8/8] perf pmu: Move pmu_metrics_table__find and remove ARM override

[Ian Rogers]

On Sat, Nov 9, 2024 at 2:54 AM Yicong Yang <yangyicong@huawei.com> wrote:
>
> Hi,
>
> On 2024/11/8 0:20, Ian Rogers wrote:
> > Move pmu_metrics_table__find to the jevents.py generated pmu-events.c
> > and remove indirection override for ARM. The movement removes
> > perf_pmu__find_metrics_table that exists to enable the ARM
> > override. The ARM override isn't necessary as just the CPUID, not PMU,
> > is used in the metric table lookup. On non-ARM the CPU argument is
> > just ignored for the CPUID, for ARM -1 is passed so that the CPUID for
> > the first logical CPU is read.
>
> Since the logic here's already been touching, is it possible to step it further to just
> ignore the CPUID matching when finding the system metrics/events tables? It's may not be
> that reasonable for finding a system metrics/events from the CPUID, since one system PMU may
> exists on different platforms with different CPU types.

The issue is for conciseness reasons we let metrics and metric
thresholds refer to other metrics, for example:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json#n78
```
    {
        "BriefDescription": "This category represents fraction of
slots where no uops are being delivered due to a lack of required
resources for accepting new uops in the Backend",
        "MetricExpr": "topdown\\-be\\-bound / (topdown\\-fe\\-bound +
topdown\\-bad\\-spec + topdown\\-retiring + topdown\\-be\\-bound) + 5
* cpu@INT_MISC.RECOVERY_CYCLES\\,cmask\\=1\\,edge@ / tma_info_slots",
        "MetricGroup": "TmaL1;TopdownL1;tma_L1_group",
        "MetricName": "tma_backend_bound",
        "MetricThreshold": "tma_backend_bound > 0.2",
        "MetricgroupNoGroup": "TopdownL1",
        "PublicDescription": "This category represents fraction of
slots where no uops are being delivered due to a lack of required
resources for accepting new uops in the Backend. Backend is the
portion of the processor core where the out-of-order scheduler
dispatches ready uops into their respective execution units; and once
completed these uops get retired according to program order. For
example; stalls due to data-cache misses or stalls due to the divider
unit being overloaded are both categorized under Backend Bound.
Backend Bound is further divided into two main categories: Memory
Bound and Core Bound. Sample with: TOPDOWN.BACKEND_BOUND_SLOTS",
        "ScaleUnit": "100%"
    },
```

The system metrics were added on top of this and we never rethought
the design. For a metric to refer to another metric there needs to be
some kind of place we look up from and for that we use the CPUID
associated table. Perhaps the easiest thing is that if no CPUID table
is matched we have an empty table.

> FYI, there's a similiar problem when trying to count the system metrics but fails [1].
> I've tested with this series but the problem still exists.

Not sure what you are asking me for here.

Thanks,
Ian

> [1] https://lore.kernel.org/linux-perf-users/20241010074430.16685-1-hejunhao3@huawei.com/
>
> Thanks.
>
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/arch/arm64/util/pmu.c         | 20 --------------------
> >  tools/perf/pmu-events/empty-pmu-events.c | 10 ++++------
> >  tools/perf/pmu-events/jevents.py         | 10 ++++------
> >  tools/perf/pmu-events/pmu-events.h       |  2 +-
> >  tools/perf/util/pmu.c                    |  5 -----
> >  tools/perf/util/pmu.h                    |  1 -
> >  6 files changed, 9 insertions(+), 39 deletions(-)
> >
> > diff --git a/tools/perf/arch/arm64/util/pmu.c b/tools/perf/arch/arm64/util/pmu.c
> > index a0964b191fcb..895fb0d0610c 100644
> > --- a/tools/perf/arch/arm64/util/pmu.c
> > +++ b/tools/perf/arch/arm64/util/pmu.c
> > @@ -1,29 +1,9 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >
> > -#include <internal/cpumap.h>
> > -#include "../../../util/cpumap.h"
> > -#include "../../../util/header.h"
> >  #include "../../../util/pmu.h"
> >  #include "../../../util/pmus.h"
> >  #include "../../../util/tool_pmu.h"
> >  #include <api/fs/fs.h>
> > -#include <math.h>
> > -
> > -const struct pmu_metrics_table *pmu_metrics_table__find(void)
> > -{
> > -     struct perf_pmu *pmu;
> > -
> > -     /* Metrics aren't currently supported on heterogeneous Arm systems */
> > -     if (perf_pmus__num_core_pmus() > 1)
> > -             return NULL;
> > -
> > -     /* Doesn't matter which one here because they'll all be the same */
> > -     pmu = perf_pmus__find_core_pmu();
> > -     if (pmu)
> > -             return perf_pmu__find_metrics_table(pmu);
> > -
> > -     return NULL;
> > -}
> >
> >  u64 tool_pmu__cpu_slots_per_cycle(void)
> >  {
> > diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
> > index 17306e316a3c..1c7a2cfa321f 100644
> > --- a/tools/perf/pmu-events/empty-pmu-events.c
> > +++ b/tools/perf/pmu-events/empty-pmu-events.c
> > @@ -587,14 +587,12 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
> >          return NULL;
> >  }
> >
> > -const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu)
> > +const struct pmu_metrics_table *pmu_metrics_table__find(void)
> >  {
> > -        const struct pmu_events_map *map = map_for_pmu(pmu);
> > -
> > -        if (!map)
> > -                return NULL;
> > +        struct perf_cpu cpu = {-1};
> > +        const struct pmu_events_map *map = map_for_cpu(cpu);
> >
> > -     return &map->metric_table;
> > +        return map ? &map->metric_table : NULL;
> >  }
> >
> >  const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
> > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
> > index e44b72e56ac3..d781a377757a 100755
> > --- a/tools/perf/pmu-events/jevents.py
> > +++ b/tools/perf/pmu-events/jevents.py
> > @@ -1103,14 +1103,12 @@ const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu)
> >          return NULL;
> >  }
> >
> > -const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu)
> > +const struct pmu_metrics_table *pmu_metrics_table__find(void)
> >  {
> > -        const struct pmu_events_map *map = map_for_pmu(pmu);
> > -
> > -        if (!map)
> > -                return NULL;
> > +        struct perf_cpu cpu = {-1};
> > +        const struct pmu_events_map *map = map_for_cpu(cpu);
> >
> > -     return &map->metric_table;
> > +        return map ? &map->metric_table : NULL;
> >  }
> >
> >  const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid)
> > diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
> > index 5435ad92180c..675562e6f770 100644
> > --- a/tools/perf/pmu-events/pmu-events.h
> > +++ b/tools/perf/pmu-events/pmu-events.h
> > @@ -103,7 +103,7 @@ int pmu_metrics_table__for_each_metric(const struct pmu_metrics_table *table, pm
> >                                    void *data);
> >
> >  const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu);
> > -const struct pmu_metrics_table *perf_pmu__find_metrics_table(struct perf_pmu *pmu);
> > +const struct pmu_metrics_table *pmu_metrics_table__find(void);
> >  const struct pmu_events_table *find_core_events_table(const char *arch, const char *cpuid);
> >  const struct pmu_metrics_table *find_core_metrics_table(const char *arch, const char *cpuid);
> >  int pmu_for_each_core_event(pmu_event_iter_fn fn, void *data);
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 514cb865f57b..45838651b361 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -818,11 +818,6 @@ static int is_sysfs_pmu_core(const char *name)
> >       return file_available(path);
> >  }
> >
> > -__weak const struct pmu_metrics_table *pmu_metrics_table__find(void)
> > -{
> > -     return perf_pmu__find_metrics_table(NULL);
> > -}
> > -
> >  /**
> >   * Return the length of the PMU name not including the suffix for uncore PMUs.
> >   *
> > diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> > index fba3fc608b64..7b3e71194e49 100644
> > --- a/tools/perf/util/pmu.h
> > +++ b/tools/perf/util/pmu.h
> > @@ -260,7 +260,6 @@ void perf_pmu__arch_init(struct perf_pmu *pmu);
> >  void pmu_add_cpu_aliases_table(struct perf_pmu *pmu,
> >                              const struct pmu_events_table *table);
> >
> > -const struct pmu_metrics_table *pmu_metrics_table__find(void);
> >  bool pmu_uncore_identifier_match(const char *compat, const char *id);
> >
> >  int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
> >

Re: [PATCH v2 8/8] perf pmu: Move pmu_metrics_table__find and remove ARM override

[Yicong Yang]

On 2024/11/10 0:10, Ian Rogers wrote:
> On Sat, Nov 9, 2024 at 2:54 AM Yicong Yang <yangyicong@huawei.com> wrote:
>>
>> Hi,
>>
>> On 2024/11/8 0:20, Ian Rogers wrote:
>>> Move pmu_metrics_table__find to the jevents.py generated pmu-events.c
>>> and remove indirection override for ARM. The movement removes
>>> perf_pmu__find_metrics_table that exists to enable the ARM
>>> override. The ARM override isn't necessary as just the CPUID, not PMU,
>>> is used in the metric table lookup. On non-ARM the CPU argument is
>>> just ignored for the CPUID, for ARM -1 is passed so that the CPUID for
>>> the first logical CPU is read.
>>
>> Since the logic here's already been touching, is it possible to step it further to just
>> ignore the CPUID matching when finding the system metrics/events tables? It's may not be
>> that reasonable for finding a system metrics/events from the CPUID, since one system PMU may
>> exists on different platforms with different CPU types.
> 
> The issue is for conciseness reasons we let metrics and metric
> thresholds refer to other metrics, for example:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json#n78
> ```
>     {
>         "BriefDescription": "This category represents fraction of
> slots where no uops are being delivered due to a lack of required
> resources for accepting new uops in the Backend",
>         "MetricExpr": "topdown\\-be\\-bound / (topdown\\-fe\\-bound +
> topdown\\-bad\\-spec + topdown\\-retiring + topdown\\-be\\-bound) + 5
> * cpu@INT_MISC.RECOVERY_CYCLES\\,cmask\\=1\\,edge@ / tma_info_slots",
>         "MetricGroup": "TmaL1;TopdownL1;tma_L1_group",
>         "MetricName": "tma_backend_bound",
>         "MetricThreshold": "tma_backend_bound > 0.2",
>         "MetricgroupNoGroup": "TopdownL1",
>         "PublicDescription": "This category represents fraction of
> slots where no uops are being delivered due to a lack of required
> resources for accepting new uops in the Backend. Backend is the
> portion of the processor core where the out-of-order scheduler
> dispatches ready uops into their respective execution units; and once
> completed these uops get retired according to program order. For
> example; stalls due to data-cache misses or stalls due to the divider
> unit being overloaded are both categorized under Backend Bound.
> Backend Bound is further divided into two main categories: Memory
> Bound and Core Bound. Sample with: TOPDOWN.BACKEND_BOUND_SLOTS",
>         "ScaleUnit": "100%"
>     },
> ```
> 
> The system metrics were added on top of this and we never rethought
> the design. For a metric to refer to another metric there needs to be
> some kind of place we look up from and for that we use the CPUID
> associated table. Perhaps the easiest thing is that if no CPUID table
> is matched we have an empty table.
> 

thanks for the clarificaiton. ok then it's different from my problem and I
shouldn't have mixed them up.

Thanks.

Re: [PATCH v2 0/8] Refactor cpuid and metric table lookup code

[Ian Rogers]

On Thu, Nov 7, 2024 at 6:57 PM Xu Yang <xu.yang_2@nxp.com> wrote:
>
> On Thu, Nov 07, 2024 at 08:20:27AM -0800, Ian Rogers wrote:
> > Xu Yang <xu.yang_2@nxp.com> reported issues with the system metric
> > lookup:
> > https://lore.kernel.org/linux-perf-users/20241106085441.3945502-1-xu.yang_2@nxp.com/
> > These patches remove a lot of the logic relating CPUIDs to PMUs so
> > that the PMU isn't part of the question when finding a metric table.
> > For time reasons, it doesn't go as far as allowing system metrics
> > without a metric table as a metric table is needed for metrics to
> > refer to other metrics, and the refactoring of that resolution is a
> > hassle.
>
> For this patchset:
> Tested-by: Xu Yang <xu.yang_2@nxp.com>

Ping. Would be nice for this to land given the ARM fix and general cleanup.

Thanks,
Ian

Re: [PATCH v2 0/8] Refactor cpuid and metric table lookup code

[James Clark]

On 07/11/2024 4:20 pm, Ian Rogers wrote:
> Xu Yang <xu.yang_2@nxp.com> reported issues with the system metric
> lookup:
> https://lore.kernel.org/linux-perf-users/20241106085441.3945502-1-xu.yang_2@nxp.com/
> These patches remove a lot of the logic relating CPUIDs to PMUs so
> that the PMU isn't part of the question when finding a metric table.
> For time reasons, it doesn't go as far as allowing system metrics
> without a metric table as a metric table is needed for metrics to
> refer to other metrics, and the refactoring of that resolution is a
> hassle.
> 
> Ian Rogers (7):
>    perf header: Move is_cpu_online to numa bench
>    perf header: Refactor get_cpuid to take a CPU for ARM
>    perf arm64 header: Use cpu argument in get_cpuid
>    perf header: Avoid transitive PMU includes
>    perf header: Pass a perf_cpu rather than a PMU to get_cpuid_str
>    perf jevents: Add map_for_cpu
>    perf pmu: Move pmu_metrics_table__find and remove ARM override
> 
> Xu Yang (1):
>    perf jevents: fix breakage when do perf stat on system metric
> 
>   tools/perf/arch/arm64/util/arm-spe.c     | 14 +---
>   tools/perf/arch/arm64/util/header.c      | 73 ++++++++++-----------
>   tools/perf/arch/arm64/util/pmu.c         | 20 ------
>   tools/perf/arch/loongarch/util/header.c  |  4 +-
>   tools/perf/arch/powerpc/util/header.c    |  4 +-
>   tools/perf/arch/riscv/util/header.c      |  4 +-
>   tools/perf/arch/s390/util/header.c       |  6 +-
>   tools/perf/arch/x86/util/auxtrace.c      |  3 +-
>   tools/perf/arch/x86/util/header.c        |  5 +-
>   tools/perf/bench/numa.c                  | 53 +++++++++++++++
>   tools/perf/builtin-kvm.c                 |  4 +-
>   tools/perf/pmu-events/empty-pmu-events.c | 39 ++++++-----
>   tools/perf/pmu-events/jevents.py         | 39 ++++++-----
>   tools/perf/pmu-events/pmu-events.h       |  2 +-
>   tools/perf/tests/expr.c                  |  5 +-
>   tools/perf/util/env.c                    |  4 +-
>   tools/perf/util/expr.c                   |  6 +-
>   tools/perf/util/header.c                 | 82 ++++++++----------------
>   tools/perf/util/header.h                 | 23 +++----
>   tools/perf/util/pmu.c                    | 25 --------
>   tools/perf/util/pmu.h                    |  2 -
>   tools/perf/util/probe-event.c            |  1 +
>   22 files changed, 189 insertions(+), 229 deletions(-)
> 

Reviewed-by: James Clark <james.clark@linaro.org>

Re: [PATCH v2 0/8] Refactor cpuid and metric table lookup code

[Arnaldo Carvalho de Melo]

On Fri, Nov 15, 2024 at 03:35:55PM +0000, James Clark wrote:
> 
> 
> On 07/11/2024 4:20 pm, Ian Rogers wrote:
> > Xu Yang <xu.yang_2@nxp.com> reported issues with the system metric
> > lookup:
> > https://lore.kernel.org/linux-perf-users/20241106085441.3945502-1-xu.yang_2@nxp.com/
> > These patches remove a lot of the logic relating CPUIDs to PMUs so
> > that the PMU isn't part of the question when finding a metric table.
> > For time reasons, it doesn't go as far as allowing system metrics
> > without a metric table as a metric table is needed for metrics to
> > refer to other metrics, and the refactoring of that resolution is a
> > hassle.
> > 
> > Ian Rogers (7):
> >    perf header: Move is_cpu_online to numa bench
> >    perf header: Refactor get_cpuid to take a CPU for ARM
> >    perf arm64 header: Use cpu argument in get_cpuid
> >    perf header: Avoid transitive PMU includes
> >    perf header: Pass a perf_cpu rather than a PMU to get_cpuid_str
> >    perf jevents: Add map_for_cpu
> >    perf pmu: Move pmu_metrics_table__find and remove ARM override
> > 
> > Xu Yang (1):
> >    perf jevents: fix breakage when do perf stat on system metric
> > 
> >   tools/perf/arch/arm64/util/arm-spe.c     | 14 +---
> >   tools/perf/arch/arm64/util/header.c      | 73 ++++++++++-----------
> >   tools/perf/arch/arm64/util/pmu.c         | 20 ------
> >   tools/perf/arch/loongarch/util/header.c  |  4 +-
> >   tools/perf/arch/powerpc/util/header.c    |  4 +-
> >   tools/perf/arch/riscv/util/header.c      |  4 +-
> >   tools/perf/arch/s390/util/header.c       |  6 +-
> >   tools/perf/arch/x86/util/auxtrace.c      |  3 +-
> >   tools/perf/arch/x86/util/header.c        |  5 +-
> >   tools/perf/bench/numa.c                  | 53 +++++++++++++++
> >   tools/perf/builtin-kvm.c                 |  4 +-
> >   tools/perf/pmu-events/empty-pmu-events.c | 39 ++++++-----
> >   tools/perf/pmu-events/jevents.py         | 39 ++++++-----
> >   tools/perf/pmu-events/pmu-events.h       |  2 +-
> >   tools/perf/tests/expr.c                  |  5 +-
> >   tools/perf/util/env.c                    |  4 +-
> >   tools/perf/util/expr.c                   |  6 +-
> >   tools/perf/util/header.c                 | 82 ++++++++----------------
> >   tools/perf/util/header.h                 | 23 +++----
> >   tools/perf/util/pmu.c                    | 25 --------
> >   tools/perf/util/pmu.h                    |  2 -
> >   tools/perf/util/probe-event.c            |  1 +
> >   22 files changed, 189 insertions(+), 229 deletions(-)
> > 
> 
> Reviewed-by: James Clark <james.clark@linaro.org>

Thanks for reviewing James, I'm doing a round of tests to push what I
have in tmp.perf-tools-next to perf-tools-next so that it gets test
merged on linux-next and later today I'll escape from the holiday and
take a last look on this series, apply and test.

- Arnaldo

Re: [PATCH v2 0/8] Refactor cpuid and metric table lookup code

[Arnaldo Carvalho de Melo]

On Fri, Nov 15, 2024 at 12:43:05PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Nov 15, 2024 at 03:35:55PM +0000, James Clark wrote:
> > 
> > 
> > On 07/11/2024 4:20 pm, Ian Rogers wrote:
> > > Xu Yang <xu.yang_2@nxp.com> reported issues with the system metric
> > > lookup:
> > > https://lore.kernel.org/linux-perf-users/20241106085441.3945502-1-xu.yang_2@nxp.com/
> > > These patches remove a lot of the logic relating CPUIDs to PMUs so
> > > that the PMU isn't part of the question when finding a metric table.
> > > For time reasons, it doesn't go as far as allowing system metrics
> > > without a metric table as a metric table is needed for metrics to
> > > refer to other metrics, and the refactoring of that resolution is a
> > > hassle.
> > > 
> > > Ian Rogers (7):
> > >    perf header: Move is_cpu_online to numa bench
> > >    perf header: Refactor get_cpuid to take a CPU for ARM
> > >    perf arm64 header: Use cpu argument in get_cpuid
> > >    perf header: Avoid transitive PMU includes
> > >    perf header: Pass a perf_cpu rather than a PMU to get_cpuid_str
> > >    perf jevents: Add map_for_cpu
> > >    perf pmu: Move pmu_metrics_table__find and remove ARM override
> > > 
> > > Xu Yang (1):
> > >    perf jevents: fix breakage when do perf stat on system metric
> > > 
> > >   tools/perf/arch/arm64/util/arm-spe.c     | 14 +---
> > >   tools/perf/arch/arm64/util/header.c      | 73 ++++++++++-----------
> > >   tools/perf/arch/arm64/util/pmu.c         | 20 ------
> > >   tools/perf/arch/loongarch/util/header.c  |  4 +-
> > >   tools/perf/arch/powerpc/util/header.c    |  4 +-
> > >   tools/perf/arch/riscv/util/header.c      |  4 +-
> > >   tools/perf/arch/s390/util/header.c       |  6 +-
> > >   tools/perf/arch/x86/util/auxtrace.c      |  3 +-
> > >   tools/perf/arch/x86/util/header.c        |  5 +-
> > >   tools/perf/bench/numa.c                  | 53 +++++++++++++++
> > >   tools/perf/builtin-kvm.c                 |  4 +-
> > >   tools/perf/pmu-events/empty-pmu-events.c | 39 ++++++-----
> > >   tools/perf/pmu-events/jevents.py         | 39 ++++++-----
> > >   tools/perf/pmu-events/pmu-events.h       |  2 +-
> > >   tools/perf/tests/expr.c                  |  5 +-
> > >   tools/perf/util/env.c                    |  4 +-
> > >   tools/perf/util/expr.c                   |  6 +-
> > >   tools/perf/util/header.c                 | 82 ++++++++----------------
> > >   tools/perf/util/header.h                 | 23 +++----
> > >   tools/perf/util/pmu.c                    | 25 --------
> > >   tools/perf/util/pmu.h                    |  2 -
> > >   tools/perf/util/probe-event.c            |  1 +
> > >   22 files changed, 189 insertions(+), 229 deletions(-)
> > > 
> > 
> > Reviewed-by: James Clark <james.clark@linaro.org>
> 
> Thanks for reviewing James, I'm doing a round of tests to push what I
> have in tmp.perf-tools-next to perf-tools-next so that it gets test
> merged on linux-next and later today I'll escape from the holiday and
> take a last look on this series, apply and test.

Applied locally, pushing to tmp.perf-tools-next as I'm still testing the
hwmon case, building it on an 32-bit ARM machine:

processor	: 3
model name	: ARMv7 Processor rev 4 (v7l)
BogoMIPS	: 38.40
Features	: half thumb fastmult vfp edsp neon vfpv3 tls vfpv4 idiva idivt vfpd32 lpae evtstrm crc32 
CPU implementer	: 0x41
CPU architecture: 7
CPU variant	: 0x0
CPU part	: 0xd03
CPU revision	: 4

Hardware	: BCM2835
Revision	: a020d3
Serial		: 00000000783939e0
Model		: Raspberry Pi 3 Model B Plus Rev 1.3

- Arnaldo

Re: [PATCH v2 0/8] Refactor cpuid and metric table lookup code

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (fixes)
by Arnaldo Carvalho de Melo <acme@redhat.com>:

On Thu,  7 Nov 2024 08:20:27 -0800 you wrote:
> Xu Yang <xu.yang_2@nxp.com> reported issues with the system metric
> lookup:
> https://lore.kernel.org/linux-perf-users/20241106085441.3945502-1-xu.yang_2@nxp.com/
> These patches remove a lot of the logic relating CPUIDs to PMUs so
> that the PMU isn't part of the question when finding a metric table.
> For time reasons, it doesn't go as far as allowing system metrics
> without a metric table as a metric table is needed for metrics to
> refer to other metrics, and the refactoring of that resolution is a
> hassle.
> 
> [...]

Here is the summary with links:
  - [v2,1/8] perf jevents: fix breakage when do perf stat on system metric
    https://git.kernel.org/riscv/c/4a159e6049f3
  - [v2,2/8] perf header: Move is_cpu_online to numa bench
    https://git.kernel.org/riscv/c/c6fafe36bab3
  - [v2,3/8] perf header: Refactor get_cpuid to take a CPU for ARM
    https://git.kernel.org/riscv/c/cec0d6572a44
  - [v2,4/8] perf arm64 header: Use cpu argument in get_cpuid
    https://git.kernel.org/riscv/c/538737da9625
  - [v2,5/8] perf header: Avoid transitive PMU includes
    https://git.kernel.org/riscv/c/7463ee17a740
  - [v2,6/8] perf header: Pass a perf_cpu rather than a PMU to get_cpuid_str
    https://git.kernel.org/riscv/c/494c403ff159
  - [v2,7/8] perf jevents: Add map_for_cpu
    https://git.kernel.org/riscv/c/0434410fa45c
  - [v2,8/8] perf pmu: Move pmu_metrics_table__find and remove ARM override
    https://git.kernel.org/riscv/c/8f997865ee9e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html