Re: [PATCH v4 10/17] perf pmu-events: Hide pmu_events_map
From: John Garry
Date: Fri Aug 05 2022 - 08:31:34 EST
On 04/08/2022 23:18, Ian Rogers wrote:
Move usage of the table to pmu-events.c so it may be hidden. By
abstracting the table the implementation can later be changed.
Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
---
tools/perf/pmu-events/empty-pmu-events.c | 81 ++++++++-
tools/perf/pmu-events/jevents.py | 81 ++++++++-
tools/perf/pmu-events/pmu-events.h | 29 +--
tools/perf/tests/pmu-events.c | 218 +++++++++++------------
tools/perf/util/metricgroup.c | 15 +-
tools/perf/util/pmu.c | 34 +---
tools/perf/util/pmu.h | 2 +-
7 files changed, 280 insertions(+), 180 deletions(-)
diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
index 216ea0482c37..8ef75aff996c 100644
--- a/tools/perf/pmu-events/empty-pmu-events.c
+++ b/tools/perf/pmu-events/empty-pmu-events.c
@@ -6,6 +6,8 @@
* The test cpu/soc is provided for testing.
*/
#include "pmu-events/pmu-events.h"
+#include "util/header.h"
+#include "util/pmu.h"
#include <string.h>
#include <stddef.h>
@@ -110,7 +112,26 @@ static const struct pmu_event pme_test_soc_cpu[] = {
},
};
-const struct pmu_events_map pmu_events_map[] = {
+
+/*
+ * Map a CPU to its table of PMU events. The CPU is identified by the
+ * cpuid field, which is an arch-specific identifier for the CPU.
+ * The identifier specified in tools/perf/pmu-events/arch/xxx/mapfile
+ * must match the get_cpuid_str() in tools/perf/arch/xxx/util/header.c)
+ *
+ * The cpuid can contain any character other than the comma.
+ */
+struct pmu_events_map {
+ const char *arch;
+ const char *cpuid;
+ const struct pmu_event *table;
+};
+
+/*
+ * Global table mapping each known CPU for the architecture to its
+ * table of PMU events.
+ */
+static const struct pmu_events_map pmu_events_map[] = {
{
.arch = "testarch",
.cpuid = "testcpu",
@@ -162,6 +183,62 @@ static const struct pmu_sys_events pmu_sys_event_tables[] = {
},
};
+const struct pmu_event *perf_pmu__find_table(struct perf_pmu *pmu)
+{
+ const struct pmu_event *table = NULL;
+ char *cpuid = perf_pmu__getcpuid(pmu);
+ int i;
+
+ /* on some platforms which uses cpus map, cpuid can be NULL for
+ * PMUs other than CORE PMUs.
+ */
+ if (!cpuid)
+ return NULL;
+
+ i = 0;
+ for (;;) {
+ const struct pmu_events_map *map = &pmu_events_map[i++];
+
+ if (!map->table)
+ break;
+
+ if (!strcmp_cpuid_str(map->cpuid, cpuid)) {
+ table = map->table;
+ break;
+ }
+ }
+ free(cpuid);
+ return table;
+}
+
+const struct pmu_event *find_core_events_table(const char *arch, const char *cpuid)
+{
+ for (const struct pmu_events_map *tables = &pmu_events_map[0];
+ tables->table;
+ tables++) {
+ if (!strcmp(tables->arch, arch) && !strcmp_cpuid_str(tables->cpuid, cpuid))
+ return tables->table;
+ }
+ return NULL;
+}
+
+int pmu_for_each_core_event(pmu_event_iter_fn fn, void *data)
+{
+ for (const struct pmu_events_map *tables = &pmu_events_map[0];
+ tables->table;
+ tables++) {
+ for (const struct pmu_event *pe = &tables->table[0];
+ pe->name || pe->metric_group || pe->metric_name;
+ pe++) {
+ int ret = fn(pe, &tables->table[0], data);
+
+ if (ret)
+ return ret;
+ }
+ }
+ return 0;
+}
+
const struct pmu_event *find_sys_events_table(const char *name)
{
for (const struct pmu_sys_events *tables = &pmu_sys_event_tables[0];
@@ -181,7 +258,7 @@ int pmu_for_each_sys_event(pmu_event_iter_fn fn, void *data)
for (const struct pmu_event *pe = &tables->table[0];
pe->name || pe->metric_group || pe->metric_name;
pe++) {
- int ret = fn(pe, data);
+ int ret = fn(pe, &tables->table[0], data);
if (ret)
return ret;
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index dd21bc9eeeed..e976c5e8e80b 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -333,7 +333,27 @@ def process_one_file(parents: Sequence[str], item: os.DirEntry) -> None:
def print_mapping_table(archs: Sequence[str]) -> None:
"""Read the mapfile and generate the struct from cpuid string to event table."""
- _args.output_file.write('const struct pmu_events_map pmu_events_map[] = {\n')
+ _args.output_file.write("""
+/*
+ * Map a CPU to its table of PMU events. The CPU is identified by the
+ * cpuid field, which is an arch-specific identifier for the CPU.
+ * The identifier specified in tools/perf/pmu-events/arch/xxx/mapfile
+ * must match the get_cpuid_str() in tools/perf/arch/xxx/util/header.c)
+ *
+ * The cpuid can contain any character other than the comma.
+ */
+struct pmu_events_map {
+ const char *arch;
+ const char *cpuid;
+ const struct pmu_event *table;
+};
+
+/*
+ * Global table mapping each known CPU for the architecture to its
+ * table of PMU events.
+ */
+const struct pmu_events_map pmu_events_map[] = {
+""")
for arch in archs:
if arch == 'test':
_args.output_file.write("""{
@@ -389,6 +409,61 @@ static const struct pmu_sys_events pmu_sys_event_tables[] = {
\t},
};
+const struct pmu_event *perf_pmu__find_table(struct perf_pmu *pmu)
+{
+ const struct pmu_event *table = NULL;
+ char *cpuid = perf_pmu__getcpuid(pmu);
This seems an identical implementation to that in empty-pmu-events.c -
can we reduce this duplication? Maybe a seperate common c file which can
be linked in
The indentation seems different also - this version seems to use whitespaces
+ int i;
+
+ /* on some platforms which uses cpus map, cpuid can be NULL for
+ * PMUs other than CORE PMUs.
+ */
+ if (!cpuid)
+ return NULL;
+
+ i = 0;
+ for (;;) {
+ const struct pmu_events_map *map = &pmu_events_map[i++];
+ if (!map->table)
+ break;
+
+ if (!strcmp_cpuid_str(map->cpuid, cpuid)) {
+ table = map->table;
+ break;
+ }
+ }
+ free(cpuid);
+ return table;
+}