2022-08-04 22:33:52

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v4 08/17] perf pmu-events: Hide pmu_sys_event_tables

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 <[email protected]>
---
tools/perf/pmu-events/empty-pmu-events.c | 37 ++++++++++++++++++-
tools/perf/pmu-events/jevents.py | 45 ++++++++++++++++++++++--
tools/perf/pmu-events/pmu-events.h | 11 +++---
tools/perf/tests/pmu-events.c | 14 +-------
tools/perf/util/pmu.c | 27 --------------
tools/perf/util/pmu.h | 2 --
6 files changed, 84 insertions(+), 52 deletions(-)

diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c
index 4182a986f505..216ea0482c37 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 <string.h>
+#include <stddef.h>

static const struct pmu_event pme_test_soc_cpu[] = {
{
@@ -145,7 +147,12 @@ static const struct pmu_event pme_test_soc_sys[] = {
},
};

-const struct pmu_sys_events pmu_sys_event_tables[] = {
+struct pmu_sys_events {
+ const char *name;
+ const struct pmu_event *table;
+};
+
+static const struct pmu_sys_events pmu_sys_event_tables[] = {
{
.table = pme_test_soc_sys,
.name = "pme_test_soc_sys",
@@ -154,3 +161,31 @@ const struct pmu_sys_events pmu_sys_event_tables[] = {
.table = 0
},
};
+
+const struct pmu_event *find_sys_events_table(const char *name)
+{
+ for (const struct pmu_sys_events *tables = &pmu_sys_event_tables[0];
+ tables->name;
+ tables++) {
+ if (!strcmp(tables->name, name))
+ return tables->table;
+ }
+ return NULL;
+}
+
+int pmu_for_each_sys_event(pmu_event_iter_fn fn, void *data)
+{
+ for (const struct pmu_sys_events *tables = &pmu_sys_event_tables[0];
+ tables->name;
+ tables++) {
+ for (const struct pmu_event *pe = &tables->table[0];
+ pe->name || pe->metric_group || pe->metric_name;
+ pe++) {
+ int ret = fn(pe, data);
+
+ if (ret)
+ return ret;
+ }
+ }
+ return 0;
+}
diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py
index 30e0e792221a..dd21bc9eeeed 100755
--- a/tools/perf/pmu-events/jevents.py
+++ b/tools/perf/pmu-events/jevents.py
@@ -370,8 +370,14 @@ def print_mapping_table(archs: Sequence[str]) -> None:

def print_system_mapping_table() -> None:
"""C struct mapping table array for tables from /sys directories."""
- _args.output_file.write(
- '\nconst struct pmu_sys_events pmu_sys_event_tables[] = {\n')
+ _args.output_file.write("""
+struct pmu_sys_events {
+\tconst char *name;
+\tconst struct pmu_event *table;
+};
+
+static const struct pmu_sys_events pmu_sys_event_tables[] = {
+""")
for tblname in _sys_event_tables:
_args.output_file.write(f"""\t{{
\t\t.table = {tblname},
@@ -382,6 +388,34 @@ def print_system_mapping_table() -> None:
\t\t.table = 0
\t},
};
+
+const struct pmu_event *find_sys_events_table(const char *name)
+{
+ for (const struct pmu_sys_events *tables = &pmu_sys_event_tables[0];
+ tables->name;
+ tables++) {
+ if (!strcmp(tables->name, name))
+ return tables->table;
+ }
+ return NULL;
+}
+
+int pmu_for_each_sys_event(pmu_event_iter_fn fn, void *data)
+{
+ for (const struct pmu_sys_events *tables = &pmu_sys_event_tables[0];
+ tables->name;
+ tables++) {
+ for (const struct pmu_event *pe = &tables->table[0];
+ pe->name || pe->metric_group || pe->metric_name;
+ pe++) {
+ int ret = fn(pe, data);
+
+ if (ret)
+ return ret;
+ }
+ }
+ return 0;
+}
""")


@@ -413,7 +447,12 @@ def main() -> None:
'output_file', type=argparse.FileType('w'), nargs='?', default=sys.stdout)
_args = ap.parse_args()

- _args.output_file.write("#include \"pmu-events/pmu-events.h\"\n")
+ _args.output_file.write("""
+#include "pmu-events/pmu-events.h"
+#include <string.h>
+#include <stddef.h>
+
+""")
archs = []
for item in os.scandir(_args.starting_dir):
if not item.is_dir():
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index a491b117c8ac..2386212b1df0 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -43,16 +43,15 @@ struct pmu_events_map {
const struct pmu_event *table;
};

-struct pmu_sys_events {
- const char *name;
- const struct pmu_event *table;
-};
-
/*
* Global table mapping each known CPU for the architecture to its
* table of PMU events.
*/
extern const struct pmu_events_map pmu_events_map[];
-extern const struct pmu_sys_events pmu_sys_event_tables[];
+
+const struct pmu_event *find_sys_events_table(const char *name);
+
+typedef int (*pmu_event_iter_fn)(const struct pmu_event *pe, void *data);
+int pmu_for_each_sys_event(pmu_event_iter_fn fn, void *data);

#endif
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 82192f1a7bf7..a39a2c99ede6 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -286,18 +286,6 @@ static const struct pmu_events_map *__test_pmu_get_events_map(void)
return NULL;
}

-static const struct pmu_event *__test_pmu_get_sys_events_table(void)
-{
- const struct pmu_sys_events *tables = &pmu_sys_event_tables[0];
-
- for ( ; tables->name; tables++) {
- if (!strcmp("pme_test_soc_sys", tables->name))
- return tables->table;
- }
-
- return NULL;
-}
-
static int compare_pmu_events(const struct pmu_event *e1, const struct pmu_event *e2)
{
if (!is_same(e1->name, e2->name)) {
@@ -451,7 +439,7 @@ static int compare_alias_to_test_event(struct perf_pmu_alias *alias,
static int test__pmu_event_table(struct test_suite *test __maybe_unused,
int subtest __maybe_unused)
{
- const struct pmu_event *sys_event_tables = __test_pmu_get_sys_events_table();
+ const struct pmu_event *sys_event_tables = find_sys_events_table("pme_test_soc_sys");
const struct pmu_events_map *map = __test_pmu_get_events_map();
const struct pmu_event *table;
int map_events = 0, expected_events;
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 0112e1c36418..d8717c4548a4 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -868,33 +868,6 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
pmu_add_cpu_aliases_map(head, pmu, map);
}

-void pmu_for_each_sys_event(pmu_sys_event_iter_fn fn, void *data)
-{
- int i = 0;
-
- while (1) {
- const struct pmu_sys_events *event_table;
- int j = 0;
-
- event_table = &pmu_sys_event_tables[i++];
-
- if (!event_table->table)
- break;
-
- while (1) {
- const struct pmu_event *pe = &event_table->table[j++];
- int ret;
-
- if (!pe->name && !pe->metric_group && !pe->metric_name)
- break;
-
- ret = fn(pe, data);
- if (ret)
- break;
- }
- }
-}
-
struct pmu_sys_event_iter_data {
struct list_head *head;
struct perf_pmu *pmu;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 4b45fd8da5a3..7e667eec2a01 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -133,8 +133,6 @@ const struct pmu_events_map *pmu_events_map__find(void);
bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
void perf_pmu_free_alias(struct perf_pmu_alias *alias);

-typedef int (*pmu_sys_event_iter_fn)(const struct pmu_event *pe, void *data);
-void pmu_for_each_sys_event(pmu_sys_event_iter_fn fn, void *data);
int perf_pmu__convert_scale(const char *scale, char **end, double *sval);

int perf_pmu__caps_parse(struct perf_pmu *pmu);
--
2.37.1.559.g78731f0fdb-goog



2022-08-05 11:43:27

by John Garry

[permalink] [raw]
Subject: Re: [PATCH v4 08/17] perf pmu-events: Hide pmu_sys_event_tables

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<[email protected]>

Reviewed-by: John Garry <[email protected]>

> - const struct pmu_event *sys_event_tables = __test_pmu_get_sys_events_table();
> + const struct pmu_event *sys_event_tables = find_sys_events_table("pme_test_soc_sys");

Maybe we can change to not need the "pme_" prefix or even some of the
suffix, like "_sys"

> const struct pmu_events_map *map = __test_pmu_get_events_map();


2022-08-10 14:37:40

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v4 08/17] perf pmu-events: Hide pmu_sys_event_tables

On Fri, Aug 5, 2022 at 4:15 AM John Garry <[email protected]> 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<[email protected]>
>
> Reviewed-by: John Garry <[email protected]>
>
> > - const struct pmu_event *sys_event_tables = __test_pmu_get_sys_events_table();
> > + const struct pmu_event *sys_event_tables = find_sys_events_table("pme_test_soc_sys");
>
> Maybe we can change to not need the "pme_" prefix or even some of the
> suffix, like "_sys"

Ack. Not done here so the change is the smallest possible.

Thanks,
Ian

> > const struct pmu_events_map *map = __test_pmu_get_events_map();
>