On Fri, Aug 5, 2022 at 5:31 AM John Garry [off-list ref] wrote:

On 04/08/2022 23:18, Ian Rogers wrote:

quoted

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@google.com>
---
  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

Agreed. Later on this will change, the empty version isn't compressed
and the jevents.py one is. Having a common C file would defeat the
goal of hiding the API, but ultimately we'd need to get rid of it in
later changes when the empty/compressed implementations diverge.

Thanks,
Ian

quoted

+        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;
+}

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel