Allow events/aliases to be eagerly loaded for a PMU. Factor out the
pmu_aliases_parse to allow this. Parse a test event and check it
configures the attribute as expected. There is overlap with the
parse-events tests, but this test is done with a PMU created in a temp
directory and doesn't rely on PMUs in sysfs.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/pmu.c | 77 ++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/pmu.c  | 69 +++++++++++++++++++++++++------------
 2 files changed, 125 insertions(+), 21 deletions(-)

diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
index 424ebdb0f09d..6e18a4c447ce 100644
--- a/tools/perf/tests/pmu.c
+++ b/tools/perf/tests/pmu.c
@@ -1,4 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
+#include "evlist.h"
+#include "evsel.h"
 #include "parse-events.h"
 #include "pmu.h"
 #include "tests.h"
@@ -54,6 +56,9 @@ static struct perf_pmu *test_pmu_get(char *dir, size_t sz)
 		{ "krava22", "config2:8,18,48,58\n", },
 		{ "krava23", "config2:28-29,38\n", },
 	};
+	const char *test_event = "krava01=15,krava02=170,krava03=1,krava11=27,krava12=1,"
+		"krava13=2,krava21=119,krava22=11,krava23=2\n";
+
 	char name[PATH_MAX];
 	int dirfd, file;
 	struct perf_pmu *pmu = NULL;
@@ -116,6 +121,24 @@ static struct perf_pmu *test_pmu_get(char *dir, size_t sz)
 		close(file);
 	}
 
+	/* Create test event. */
+	if (mkdirat(dirfd, "perf-pmu-test/events", 0755) < 0) {
+		pr_err("Failed to mkdir PMU events directory\n");
+		goto err_out;
+	}
+	file = openat(dirfd, "perf-pmu-test/events/test-event", O_WRONLY | O_CREAT, 0600);
+	if (!file) {
+		pr_err("Failed to open for writing file \"type\"\n");
+		goto err_out;
+	}
+	len = strlen(test_event);
+	if (write(file, test_event, len) < len) {
+		close(file);
+		pr_err("Failed to write to 'test-event' file\n");
+		goto err_out;
+	}
+	close(file);
+
 	/* Make the PMU reading the files created above. */
 	pmu = perf_pmus__add_test_pmu(dirfd, "perf-pmu-test");
 	if (!pmu)
@@ -176,8 +199,62 @@ static int test__pmu_format(struct test_suite *test __maybe_unused, int subtest
 	return ret;
 }
 
+static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
+{
+	char dir[PATH_MAX];
+	struct parse_events_error err;
+	struct evlist *evlist;
+	struct evsel *evsel;
+	struct perf_event_attr *attr;
+	int ret;
+	struct perf_pmu *pmu = test_pmu_get(dir, sizeof(dir));
+	const char *event = "perf-pmu-test/test-event/";
+
+
+	if (!pmu)
+		return TEST_FAIL;
+
+	evlist = evlist__new();
+	if (evlist == NULL) {
+		pr_err("Failed allocation");
+		goto err_out;
+	}
+	parse_events_error__init(&err);
+	ret = parse_events(evlist, event, &err);
+	if (ret) {
+		pr_debug("failed to parse event '%s', err %d\n", event, ret);
+		parse_events_error__print(&err, event);
+		ret = TEST_FAIL;
+		if (parse_events_error__contains(&err, "can't access trace events"))
+			ret = TEST_SKIP;
+		goto err_out;
+	}
+	evsel = evlist__first(evlist);
+	attr = &evsel->core.attr;
+	if (attr->config  != 0xc00000000002a823) {
+		pr_err("Unexpected config value %llx\n", attr->config);
+		goto err_out;
+	}
+	if (attr->config1 != 0x8000400000000145) {
+		pr_err("Unexpected config1 value %llx\n", attr->config1);
+		goto err_out;
+	}
+	if (attr->config2 != 0x0400000020041d07) {
+		pr_err("Unexpected config2 value %llx\n", attr->config2);
+		goto err_out;
+	}
+
+	ret = TEST_OK;
+err_out:
+	parse_events_error__exit(&err);
+	evlist__delete(evlist);
+	test_pmu_put(dir, pmu);
+	return ret;
+}
+
 static struct test_case tests__pmu[] = {
 	TEST_CASE("Parsing with PMU format directory", pmu_format),
+	TEST_CASE("Parsing with PMU event", pmu_events),
 	{	.name = NULL, }
 };
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index fbbc535ed93f..7849be4bfea1 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -597,33 +597,18 @@ static inline bool pmu_alias_info_file(const char *name)
  * Reading the pmu event aliases definition, which should be located at:
  * /sys/bus/event_source/devices/<dev>/events as sysfs group attributes.
  */
-static int pmu_aliases_parse(struct perf_pmu *pmu)
+static int __pmu_aliases_parse(struct perf_pmu *pmu, int events_dir_fd)
 {
-	char path[PATH_MAX];
 	struct dirent *evt_ent;
 	DIR *event_dir;
-	size_t len;
-	int fd, dir_fd;
 
-	len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
-	if (!len)
-		return 0;
-	scnprintf(path + len, sizeof(path) - len, "%s/events", pmu->name);
-
-	dir_fd = open(path, O_DIRECTORY);
-	if (dir_fd == -1) {
-		pmu->sysfs_aliases_loaded = true;
-		return 0;
-	}
-
-	event_dir = fdopendir(dir_fd);
-	if (!event_dir){
-		close (dir_fd);
+	event_dir = fdopendir(events_dir_fd);
+	if (!event_dir)
 		return -EINVAL;
-	}
 
 	while ((evt_ent = readdir(event_dir))) {
 		char *name = evt_ent->d_name;
+		int fd;
 		FILE *file;
 
 		if (!strcmp(name, ".") || !strcmp(name, ".."))
@@ -635,7 +620,7 @@ static int pmu_aliases_parse(struct perf_pmu *pmu)
 		if (pmu_alias_info_file(name))
 			continue;
 
-		fd = openat(dir_fd, name, O_RDONLY);
+		fd = openat(events_dir_fd, name, O_RDONLY);
 		if (fd == -1) {
 			pr_debug("Cannot open %s\n", name);
 			continue;
@@ -653,11 +638,50 @@ static int pmu_aliases_parse(struct perf_pmu *pmu)
 	}
 
 	closedir(event_dir);
-	close (dir_fd);
 	pmu->sysfs_aliases_loaded = true;
 	return 0;
 }
 
+static int pmu_aliases_parse(struct perf_pmu *pmu)
+{
+	char path[PATH_MAX];
+	size_t len;
+	int events_dir_fd, ret;
+
+	if (pmu->sysfs_aliases_loaded)
+		return 0;
+
+	len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
+	if (!len)
+		return 0;
+	scnprintf(path + len, sizeof(path) - len, "%s/events", pmu->name);
+
+	events_dir_fd = open(path, O_DIRECTORY);
+	if (events_dir_fd == -1) {
+		pmu->sysfs_aliases_loaded = true;
+		return 0;
+	}
+	ret = __pmu_aliases_parse(pmu, events_dir_fd);
+	close(events_dir_fd);
+	return ret;
+}
+
+static int pmu_aliases_parse_eager(struct perf_pmu *pmu, int sysfs_fd)
+{
+	char path[FILENAME_MAX + 7];
+	int ret, events_dir_fd;
+
+	scnprintf(path, sizeof(path), "%s/events", pmu->name);
+	events_dir_fd = openat(sysfs_fd, path, O_DIRECTORY, 0);
+	if (events_dir_fd == -1) {
+		pmu->sysfs_aliases_loaded = true;
+		return 0;
+	}
+	ret = __pmu_aliases_parse(pmu, events_dir_fd);
+	close(events_dir_fd);
+	return ret;
+}
+
 static int pmu_alias_terms(struct perf_pmu_alias *alias, int err_loc, struct list_head *terms)
 {
 	struct parse_events_term *term, *cloned;
@@ -1042,6 +1066,9 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
 
 	perf_pmu__arch_init(pmu);
 
+	if (eager_load)
+		pmu_aliases_parse_eager(pmu, dirfd);
+
 	return pmu;
 err:
 	zfree(&pmu->name);
-- 
2.45.0.rc0.197.gbae5840b3b-goog

On 2024-05-02 12:01 a.m., Ian Rogers wrote:
> Allow events/aliases to be eagerly loaded for a PMU. Factor out the
> pmu_aliases_parse to allow this. Parse a test event and check it
> configures the attribute as expected. There is overlap with the
> parse-events tests, but this test is done with a PMU created in a temp
> directory and doesn't rely on PMUs in sysfs.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/pmu.c | 77 ++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/pmu.c  | 69 +++++++++++++++++++++++++------------
>  2 files changed, 125 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/perf/tests/pmu.c b/tools/perf/tests/pmu.c
> index 424ebdb0f09d..6e18a4c447ce 100644
> --- a/tools/perf/tests/pmu.c
> +++ b/tools/perf/tests/pmu.c
> @@ -1,4 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
> +#include "evlist.h"
> +#include "evsel.h"
>  #include "parse-events.h"
>  #include "pmu.h"
>  #include "tests.h"
> @@ -54,6 +56,9 @@ static struct perf_pmu *test_pmu_get(char *dir, size_t sz)
>  		{ "krava22", "config2:8,18,48,58\n", },
>  		{ "krava23", "config2:28-29,38\n", },
>  	};
> +	const char *test_event = "krava01=15,krava02=170,krava03=1,krava11=27,krava12=1,"
> +		"krava13=2,krava21=119,krava22=11,krava23=2\n";
> +
>  	char name[PATH_MAX];
>  	int dirfd, file;
>  	struct perf_pmu *pmu = NULL;
> @@ -116,6 +121,24 @@ static struct perf_pmu *test_pmu_get(char *dir, size_t sz)
>  		close(file);
>  	}
>  
> +	/* Create test event. */
> +	if (mkdirat(dirfd, "perf-pmu-test/events", 0755) < 0) {
> +		pr_err("Failed to mkdir PMU events directory\n");
> +		goto err_out;
> +	}
> +	file = openat(dirfd, "perf-pmu-test/events/test-event", O_WRONLY | O_CREAT, 0600);
> +	if (!file) {
> +		pr_err("Failed to open for writing file \"type\"\n");
> +		goto err_out;
> +	}
> +	len = strlen(test_event);
> +	if (write(file, test_event, len) < len) {
> +		close(file);
> +		pr_err("Failed to write to 'test-event' file\n");
> +		goto err_out;
> +	}
> +	close(file);
> +
>  	/* Make the PMU reading the files created above. */
>  	pmu = perf_pmus__add_test_pmu(dirfd, "perf-pmu-test");
>  	if (!pmu)
> @@ -176,8 +199,62 @@ static int test__pmu_format(struct test_suite *test __maybe_unused, int subtest
>  	return ret;
>  }
>  
> +static int test__pmu_events(struct test_suite *test __maybe_unused, int subtest __maybe_unused)
> +{
> +	char dir[PATH_MAX];
> +	struct parse_events_error err;
> +	struct evlist *evlist;
> +	struct evsel *evsel;
> +	struct perf_event_attr *attr;
> +	int ret;
> +	struct perf_pmu *pmu = test_pmu_get(dir, sizeof(dir));
> +	const char *event = "perf-pmu-test/test-event/";
> +
> +
> +	if (!pmu)
> +		return TEST_FAIL;
> +
> +	evlist = evlist__new();
> +	if (evlist == NULL) {
> +		pr_err("Failed allocation");
> +		goto err_out;
> +	}
> +	parse_events_error__init(&err);
> +	ret = parse_events(evlist, event, &err);
> +	if (ret) {
> +		pr_debug("failed to parse event '%s', err %d\n", event, ret);
> +		parse_events_error__print(&err, event);
> +		ret = TEST_FAIL;
> +		if (parse_events_error__contains(&err, "can't access trace events"))
> +			ret = TEST_SKIP;
> +		goto err_out;
> +	}
> +	evsel = evlist__first(evlist);
> +	attr = &evsel->core.attr;
> +	if (attr->config  != 0xc00000000002a823) {
> +		pr_err("Unexpected config value %llx\n", attr->config);
> +		goto err_out;
> +	}
> +	if (attr->config1 != 0x8000400000000145) {
> +		pr_err("Unexpected config1 value %llx\n", attr->config1);
> +		goto err_out;
> +	}
> +	if (attr->config2 != 0x0400000020041d07) {
> +		pr_err("Unexpected config2 value %llx\n", attr->config2);
> +		goto err_out;
> +	}
> +
> +	ret = TEST_OK;
> +err_out:
> +	parse_events_error__exit(&err);
> +	evlist__delete(evlist);
> +	test_pmu_put(dir, pmu);
> +	return ret;

Got a warning when I compile the patch.

tests/pmu.c: In function ‘test__pmu_events’:
tests/pmu.c:257:16: error: ‘ret’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
  257 |         return ret;
      |                ^~~

Thanks,
Kan

> +}
> +
>  static struct test_case tests__pmu[] = {
>  	TEST_CASE("Parsing with PMU format directory", pmu_format),
> +	TEST_CASE("Parsing with PMU event", pmu_events),
>  	{	.name = NULL, }
>  };
>  
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index fbbc535ed93f..7849be4bfea1 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -597,33 +597,18 @@ static inline bool pmu_alias_info_file(const char *name)
>   * Reading the pmu event aliases definition, which should be located at:
>   * /sys/bus/event_source/devices/<dev>/events as sysfs group attributes.
>   */
> -static int pmu_aliases_parse(struct perf_pmu *pmu)
> +static int __pmu_aliases_parse(struct perf_pmu *pmu, int events_dir_fd)
>  {
> -	char path[PATH_MAX];
>  	struct dirent *evt_ent;
>  	DIR *event_dir;
> -	size_t len;
> -	int fd, dir_fd;
>  
> -	len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
> -	if (!len)
> -		return 0;
> -	scnprintf(path + len, sizeof(path) - len, "%s/events", pmu->name);
> -
> -	dir_fd = open(path, O_DIRECTORY);
> -	if (dir_fd == -1) {
> -		pmu->sysfs_aliases_loaded = true;
> -		return 0;
> -	}
> -
> -	event_dir = fdopendir(dir_fd);
> -	if (!event_dir){
> -		close (dir_fd);
> +	event_dir = fdopendir(events_dir_fd);
> +	if (!event_dir)
>  		return -EINVAL;
> -	}
>  
>  	while ((evt_ent = readdir(event_dir))) {
>  		char *name = evt_ent->d_name;
> +		int fd;
>  		FILE *file;
>  
>  		if (!strcmp(name, ".") || !strcmp(name, ".."))
> @@ -635,7 +620,7 @@ static int pmu_aliases_parse(struct perf_pmu *pmu)
>  		if (pmu_alias_info_file(name))
>  			continue;
>  
> -		fd = openat(dir_fd, name, O_RDONLY);
> +		fd = openat(events_dir_fd, name, O_RDONLY);
>  		if (fd == -1) {
>  			pr_debug("Cannot open %s\n", name);
>  			continue;
> @@ -653,11 +638,50 @@ static int pmu_aliases_parse(struct perf_pmu *pmu)
>  	}
>  
>  	closedir(event_dir);
> -	close (dir_fd);
>  	pmu->sysfs_aliases_loaded = true;
>  	return 0;
>  }
>  
> +static int pmu_aliases_parse(struct perf_pmu *pmu)
> +{
> +	char path[PATH_MAX];
> +	size_t len;
> +	int events_dir_fd, ret;
> +
> +	if (pmu->sysfs_aliases_loaded)
> +		return 0;
> +
> +	len = perf_pmu__event_source_devices_scnprintf(path, sizeof(path));
> +	if (!len)
> +		return 0;
> +	scnprintf(path + len, sizeof(path) - len, "%s/events", pmu->name);
> +
> +	events_dir_fd = open(path, O_DIRECTORY);
> +	if (events_dir_fd == -1) {
> +		pmu->sysfs_aliases_loaded = true;
> +		return 0;
> +	}
> +	ret = __pmu_aliases_parse(pmu, events_dir_fd);
> +	close(events_dir_fd);
> +	return ret;
> +}
> +
> +static int pmu_aliases_parse_eager(struct perf_pmu *pmu, int sysfs_fd)
> +{
> +	char path[FILENAME_MAX + 7];
> +	int ret, events_dir_fd;
> +
> +	scnprintf(path, sizeof(path), "%s/events", pmu->name);
> +	events_dir_fd = openat(sysfs_fd, path, O_DIRECTORY, 0);
> +	if (events_dir_fd == -1) {
> +		pmu->sysfs_aliases_loaded = true;
> +		return 0;
> +	}
> +	ret = __pmu_aliases_parse(pmu, events_dir_fd);
> +	close(events_dir_fd);
> +	return ret;
> +}
> +
>  static int pmu_alias_terms(struct perf_pmu_alias *alias, int err_loc, struct list_head *terms)
>  {
>  	struct parse_events_term *term, *cloned;
> @@ -1042,6 +1066,9 @@ struct perf_pmu *perf_pmu__lookup(struct list_head *pmus, int dirfd, const char
>  
>  	perf_pmu__arch_init(pmu);
>  
> +	if (eager_load)
> +		pmu_aliases_parse_eager(pmu, dirfd);
> +
>  	return pmu;
>  err:
>  	zfree(&pmu->name);