[PATCH v2 01/18] perf pmu: Make the loading of formats lazy

From: Ian Rogers
Date: Thu Aug 24 2023 - 00:14:27 EST

Next message: Ian Rogers: "[PATCH v2 00/18] Lazily load PMU data"
Previous message: kernel test robot: "Re: [PATCH v2 2/6] drm/panfrost: Add fdinfo support GPU load metrics"
In reply to: Ian Rogers: "[PATCH v2 00/18] Lazily load PMU data"
Next in thread: Ian Rogers: "[PATCH v2 05/18] perf parse-events: Improve error message for double setting"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

The sysfs format files are loaded eagerly in a PMU. Add a flag so that
we create the format but only load the contents when necessary.

Reduce the size of the value in struct perf_pmu_format and avoid holes
so there is no additional space requirement.

For "perf stat -e cycles true" this reduces the number of openat calls
from 648 to 573 (about 12%). The benchmark pmu scan speed is improved
by roughly 5%.

Before: $ perf bench internals pmu-scan
Computing performance of sysfs PMU event scan for 100 times
Average core PMU scanning took: 1061.100 usec (+- 9.965 usec)
Average PMU scanning took: 4725.300 usec (+- 260.599 usec)

After: $ perf bench internals pmu-scan
Computing performance of sysfs PMU event scan for 100 times
Average core PMU scanning took: 989.170 usec (+- 6.873 usec)
Average PMU scanning took: 4520.960 usec (+- 251.272 usec)

Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
---
tools/perf/tests/pmu.c | 2 +-
tools/perf/util/pmu.c | 144 +++++++++++++++++++++++++++--------------
tools/perf/util/pmu.h | 5 +-
tools/perf/util/pmu.y | 20 ++----
4 files changed, 106 insertions(+), 65 deletions(-)

diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 2c1c349a42e2..c204ed1f1a8b 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -171,7 +171,7 @@ static int test__pmu(struct test_suite *test __maybe_unused, int subtest __maybe
}

pmu->name = strdup("perf-pmu-test");
- ret = perf_pmu__format_parse(pmu, fd);
+ ret = perf_pmu__format_parse(pmu, fd, /*eager_load=*/true);
if (ret)
goto out;

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 42f3249994ab..3cfd3e58da23 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -40,6 +40,10 @@ struct perf_pmu perf_pmu__fake;
* value=PERF_PMU_FORMAT_VALUE_CONFIG and bits 0 to 7 will be set.
*/
struct perf_pmu_format {
+ /** @list: Element on list within struct perf_pmu. */
+ struct list_head list;
+ /** @bits: Which config bits are set by this format value. */
+ DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS);
/** @name: The modifier/file name. */
char *name;
/**
@@ -47,18 +51,79 @@ struct perf_pmu_format {
* are from PERF_PMU_FORMAT_VALUE_CONFIG to
* PERF_PMU_FORMAT_VALUE_CONFIG_END.
*/
- int value;
- /** @bits: Which config bits are set by this format value. */
- DECLARE_BITMAP(bits, PERF_PMU_FORMAT_BITS);
- /** @list: Element on list within struct perf_pmu. */
- struct list_head list;
+ u16 value;
+ /** @loaded: Has the contents been loaded/parsed. */
+ bool loaded;
};

+static struct perf_pmu_format *perf_pmu__new_format(struct list_head *list, char *name)
+{
+ struct perf_pmu_format *format;
+
+ format = zalloc(sizeof(*format));
+ if (!format)
+ return NULL;
+
+ format->name = strdup(name);
+ if (!format->name) {
+ free(format);
+ return NULL;
+ }
+ list_add_tail(&format->list, list);
+ return format;
+}
+
+/* Called at the end of parsing a format. */
+void perf_pmu_format__set_value(void *vformat, int config, unsigned long *bits)
+{
+ struct perf_pmu_format *format = vformat;
+
+ format->value = config;
+ memcpy(format->bits, bits, sizeof(format->bits));
+}
+
+static void __perf_pmu_format__load(struct perf_pmu_format *format, FILE *file)
+{
+ void *scanner;
+ int ret;
+
+ ret = perf_pmu_lex_init(&scanner);
+ if (ret)
+ return;
+
+ perf_pmu_set_in(file, scanner);
+ ret = perf_pmu_parse(format, scanner);
+ perf_pmu_lex_destroy(scanner);
+ format->loaded = true;
+}
+
+static void perf_pmu_format__load(struct perf_pmu *pmu, struct perf_pmu_format *format)
+{
+ char path[PATH_MAX];
+ FILE *file = NULL;
+
+ if (format->loaded)
+ return;
+
+ if (!perf_pmu__pathname_scnprintf(path, sizeof(path), pmu->name, "format"))
+ return;
+
+ assert(strlen(path) + strlen(format->name) + 2 < sizeof(path));
+ strcat(path, "/");
+ strcat(path, format->name);
+
+ file = fopen(path, "r");
+ if (!file)
+ return;
+ __perf_pmu_format__load(format, file);
+ fclose(file);
+}
+
/*
* Parse & process all the sysfs attributes located under
* the directory specified in 'dir' parameter.
*/
-int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd)
+int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd, bool eager_load)
{
struct dirent *evt_ent;
DIR *format_dir;
@@ -68,37 +133,35 @@ int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd)
if (!format_dir)
return -EINVAL;

- while (!ret && (evt_ent = readdir(format_dir))) {
+ while ((evt_ent = readdir(format_dir)) != NULL) {
+ struct perf_pmu_format *format;
char *name = evt_ent->d_name;
- int fd;
- void *scanner;
- FILE *file;

if (!strcmp(name, ".") || !strcmp(name, ".."))
continue;

-
- ret = -EINVAL;
- fd = openat(dirfd, name, O_RDONLY);
- if (fd < 0)
- break;
-
- file = fdopen(fd, "r");
- if (!file) {
- close(fd);
+ format = perf_pmu__new_format(&pmu->format, name);
+ if (!format) {
+ ret = -ENOMEM;
break;
}

- ret = perf_pmu_lex_init(&scanner);
- if (ret) {
+ if (eager_load) {
+ FILE *file;
+ int fd = openat(dirfd, name, O_RDONLY);
+
+ if (fd < 0) {
+ ret = -errno;
+ break;
+ }
+ file = fdopen(fd, "r");
+ if (!file) {
+ close(fd);
+ break;
+ }
+ __perf_pmu_format__load(format, file);
fclose(file);
- break;
}
-
- perf_pmu_set_in(file, scanner);
- ret = perf_pmu_parse(&pmu->format, name, scanner);
- perf_pmu_lex_destroy(scanner);
- fclose(file);
}

closedir(format_dir);
@@ -119,7 +182,7 @@ static int pmu_format(struct perf_pmu *pmu, int dirfd, const char *name)
return 0;

/* it'll close the fd */
- if (perf_pmu__format_parse(pmu, fd))
+ if (perf_pmu__format_parse(pmu, fd, /*eager_load=*/false))
return -1;

return 0;
@@ -962,13 +1025,15 @@ void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu)
if (pmu == &perf_pmu__fake)
return;

- list_for_each_entry(format, &pmu->format, list)
+ list_for_each_entry(format, &pmu->format, list) {
+ perf_pmu_format__load(pmu, format);
if (format->value >= PERF_PMU_FORMAT_VALUE_CONFIG_END) {
pr_warning("WARNING: '%s' format '%s' requires 'perf_event_attr::config%d'"
"which is not supported by this version of perf!\n",
pmu->name, format->name, format->value);
return;
}
+ }
}

bool evsel__is_aux_event(const struct evsel *evsel)
@@ -1041,6 +1106,7 @@ int perf_pmu__format_type(struct perf_pmu *pmu, const char *name)
if (!format)
return -1;

+ perf_pmu_format__load(pmu, format);
return format->value;
}

@@ -1177,7 +1243,7 @@ static int pmu_config_term(struct perf_pmu *pmu,
free(pmu_term);
return -EINVAL;
}
-
+ perf_pmu_format__load(pmu, format);
switch (format->value) {
case PERF_PMU_FORMAT_VALUE_CONFIG:
vp = &attr->config;
@@ -1403,24 +1469,6 @@ struct perf_pmu_alias *perf_pmu__find_alias(struct perf_pmu *pmu, const char *ev

return NULL;
}
-
-int perf_pmu__new_format(struct list_head *list, char *name,
- int config, unsigned long *bits)
-{
- struct perf_pmu_format *format;
-
- format = zalloc(sizeof(*format));
- if (!format)
- return -ENOMEM;
-
- format->name = strdup(name);
- format->value = config;
- memcpy(format->bits, bits, sizeof(format->bits));
-
- list_add_tail(&format->list, list);
- return 0;
-}
-
static void perf_pmu__del_formats(struct list_head *formats)
{
struct perf_pmu_format *fmt, *tmp;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index c4268053c979..675c9b97f7bf 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -227,9 +227,8 @@ int perf_pmu__check_alias(struct perf_pmu *pmu, struct list_head *head_terms,
struct perf_pmu_info *info);
struct perf_pmu_alias *perf_pmu__find_alias(struct perf_pmu *pmu, const char *event);

-int perf_pmu__new_format(struct list_head *list, char *name,
- int config, unsigned long *bits);
-int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd);
+int perf_pmu__format_parse(struct perf_pmu *pmu, int dirfd, bool eager_load);
+void perf_pmu_format__set_value(void *format, int config, unsigned long *bits);
bool perf_pmu__has_format(const struct perf_pmu *pmu, const char *name);

bool is_pmu_core(const char *name);
diff --git a/tools/perf/util/pmu.y b/tools/perf/util/pmu.y
index d861a5bfa3bd..600c8c158c8e 100644
--- a/tools/perf/util/pmu.y
+++ b/tools/perf/util/pmu.y
@@ -1,6 +1,5 @@
%define api.pure full
-%parse-param {struct list_head *format}
-%parse-param {char *name}
+%parse-param {void *format}
%parse-param {void *scanner}
%lex-param {void* scanner}

@@ -21,7 +20,7 @@ do { \
YYABORT; \
} while (0)

-static void perf_pmu_error(struct list_head *list, char *name, void *scanner, char const *msg);
+static void perf_pmu_error(void *format, void *scanner, const char *msg);

static void perf_pmu__set_format(unsigned long *bits, long from, long to)
{
@@ -59,16 +58,12 @@ format_term
format_term:
PP_CONFIG ':' bits
{
- ABORT_ON(perf_pmu__new_format(format, name,
- PERF_PMU_FORMAT_VALUE_CONFIG,
- $3));
+ perf_pmu_format__set_value(format, PERF_PMU_FORMAT_VALUE_CONFIG, $3);
}
|
PP_CONFIG PP_VALUE ':' bits
{
- ABORT_ON(perf_pmu__new_format(format, name,
- $2,
- $4));
+ perf_pmu_format__set_value(format, $2, $4);
}

bits:
@@ -95,9 +90,8 @@ PP_VALUE

%%

-static void perf_pmu_error(struct list_head *list __maybe_unused,
- char *name __maybe_unused,
- void *scanner __maybe_unused,
- char const *msg __maybe_unused)
+static void perf_pmu_error(void *format __maybe_unused,
+ void *scanner __maybe_unused,
+ const char *msg __maybe_unused)
{
}
--
2.42.0.rc1.204.g551eb34607-goog

Next message: Ian Rogers: "[PATCH v2 00/18] Lazily load PMU data"
Previous message: kernel test robot: "Re: [PATCH v2 2/6] drm/panfrost: Add fdinfo support GPU load metrics"
In reply to: Ian Rogers: "[PATCH v2 00/18] Lazily load PMU data"
Next in thread: Ian Rogers: "[PATCH v2 05/18] perf parse-events: Improve error message for double setting"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]