Messages in this thread | | | From | Ian Rogers <> | Date | Wed, 10 Aug 2022 07:29:19 -0700 | Subject | Re: [PATCH v4 10/17] perf pmu-events: Hide pmu_events_map |
| |
On Fri, Aug 5, 2022 at 5:31 AM John Garry <john.garry@huawei.com> wrote: > > 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@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
> > + 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; > > +}
| |