perf hwmon_pmu: Hold path rather than fd

Hold the path to the hwmon_pmu rather than the file descriptor. The
file descriptor is somewhat problematic in that it reflects the
directory state when opened, something that may vary in testing. Using
a path simplifies testing and to some extent cleanup as the hwmon_pmu
is owned by the pmus list and intentionally global and leaked when
perf terminates, the file descriptor being left open looks like a
leak.

Signed-off-by: Ian Rogers <irogers@google.com>
Link: https://lore.kernel.org/r/20250624190326.2038704-4-irogers@google.com
Signed-off-by: Namhyung Kim <namhyung@kernel.org>

diff --git a/tools/perf/tests/hwmon_pmu.c b/tools/perf/tests/hwmon_pmu.c
index 0837aca1cdfa7ca92f970cf9ff297a30ebae8d14..151f02701c8cc3358a8bcbd2e2375fe75f707e14 100644 (file)
--- a/tools/perf/tests/hwmon_pmu.c
+++ b/tools/perf/tests/hwmon_pmu.c
@@ -93,9 +93,10 @@ static struct perf_pmu *test_pmu_get(char *dir, size_t sz)
                pr_err("Failed to mkdir hwmon directory\n");
                goto err_out;
        }
-       hwmon_dirfd = openat(test_dirfd, "hwmon1234", O_DIRECTORY);
+       strncat(dir, "/hwmon1234", sz - strlen(dir));
+       hwmon_dirfd = open(dir, O_PATH|O_DIRECTORY);
        if (hwmon_dirfd < 0) {
-               pr_err("Failed to open test hwmon directory \"%s/hwmon1234\"\n", dir);
+               pr_err("Failed to open test hwmon directory \"%s\"\n", dir);
                goto err_out;
        }
        file = openat(hwmon_dirfd, "name", O_WRONLY | O_CREAT, 0600);
@@ -130,18 +131,18 @@ static struct perf_pmu *test_pmu_get(char *dir, size_t sz)
        }
 
        /* Make the PMU reading the files created above. */
-       hwm = perf_pmus__add_test_hwmon_pmu(hwmon_dirfd, "hwmon1234", test_hwmon_name);
+       hwm = perf_pmus__add_test_hwmon_pmu(dir, "hwmon1234", test_hwmon_name);
        if (!hwm)
                pr_err("Test hwmon creation failed\n");
 
 err_out:
        if (!hwm) {
                test_pmu_put(dir, hwm);
-               if (hwmon_dirfd >= 0)
-                       close(hwmon_dirfd);
        }
        if (test_dirfd >= 0)
                close(test_dirfd);
+       if (hwmon_dirfd >= 0)
+               close(hwmon_dirfd);
        return hwm;
 }
 

diff --git a/tools/perf/util/hwmon_pmu.c b/tools/perf/util/hwmon_pmu.c
index c25e7296f1c10c22105b8433bd04da8c2708b97f..7edda010ba27163ee4c964e86c7ccbc0ed89b51f 100644 (file)
--- a/tools/perf/util/hwmon_pmu.c
+++ b/tools/perf/util/hwmon_pmu.c
@@ -104,7 +104,7 @@ static const char *const hwmon_units[HWMON_TYPE_MAX] = {
 struct hwmon_pmu {
        struct perf_pmu pmu;
        struct hashmap events;
-       int hwmon_dir_fd;
+       char *hwmon_dir;
 };
 
 /**
@@ -245,7 +245,7 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)
                return 0;
 
        /* Use openat so that the directory contents are refreshed. */
-       io_dir__init(&dir, openat(pmu->hwmon_dir_fd, ".", O_CLOEXEC | O_DIRECTORY | O_RDONLY));
+       io_dir__init(&dir, open(pmu->hwmon_dir, O_CLOEXEC | O_DIRECTORY | O_RDONLY));
 
        if (dir.dirfd < 0)
                return -ENOENT;
@@ -283,7 +283,7 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)
                __set_bit(item, alarm ? value->alarm_items : value->items);
                if (item == HWMON_ITEM_LABEL) {
                        char buf[128];
-                       int fd = openat(pmu->hwmon_dir_fd, ent->d_name, O_RDONLY);
+                       int fd = openat(dir.dirfd, ent->d_name, O_RDONLY);
                        ssize_t read_len;
 
                        if (fd < 0)
@@ -342,7 +342,8 @@ err_out:
        return err;
 }
 
-struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, int hwmon_dir, const char *sysfs_name, const char *name)
+struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, const char *hwmon_dir,
+                               const char *sysfs_name, const char *name)
 {
        char buf[32];
        struct hwmon_pmu *hwm;
@@ -365,7 +366,11 @@ struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, int hwmon_dir, const cha
                return NULL;
        }
 
-       hwm->hwmon_dir_fd = hwmon_dir;
+       hwm->hwmon_dir = strdup(hwmon_dir);
+       if (!hwm->hwmon_dir) {
+               perf_pmu__delete(&hwm->pmu);
+               return NULL;
+       }
        hwm->pmu.alias_name = strdup(sysfs_name);
        if (!hwm->pmu.alias_name) {
                perf_pmu__delete(&hwm->pmu);
@@ -399,7 +404,7 @@ void hwmon_pmu__exit(struct perf_pmu *pmu)
                free(value);
        }
        hashmap__clear(&hwm->events);
-       close(hwm->hwmon_dir_fd);
+       zfree(&hwm->hwmon_dir);
 }
 
 static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, size_t out_buf_len,
@@ -409,6 +414,10 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, si
        size_t bit;
        char buf[64];
        size_t len = 0;
+       int dir = open(hwm->hwmon_dir, O_CLOEXEC | O_DIRECTORY | O_RDONLY);
+
+       if (dir < 0)
+               return 0;
 
        for_each_set_bit(bit, items, HWMON_ITEM__MAX) {
                int fd;
@@ -421,7 +430,7 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, si
                        key.num,
                        hwmon_item_strs[bit],
                        is_alarm ? "_alarm" : "");
-               fd = openat(hwm->hwmon_dir_fd, buf, O_RDONLY);
+               fd = openat(dir, buf, O_RDONLY);
                if (fd > 0) {
                        ssize_t read_len = read(fd, buf, sizeof(buf));
 
@@ -443,6 +452,7 @@ static size_t hwmon_pmu__describe_items(struct hwmon_pmu *hwm, char *out_buf, si
                        close(fd);
                }
        }
+       close(dir);
        return len;
 }
 
@@ -712,6 +722,7 @@ int perf_pmus__read_hwmon_pmus(struct list_head *pmus)
                size_t line_len;
                int hwmon_dir, name_fd;
                struct io io;
+               char buf2[128];
 
                if (class_hwmon_ent->d_type != DT_LNK)
                        continue;
@@ -730,12 +741,13 @@ int perf_pmus__read_hwmon_pmus(struct list_head *pmus)
                        close(hwmon_dir);
                        continue;
                }
-               io__init(&io, name_fd, buf, sizeof(buf));
+               io__init(&io, name_fd, buf2, sizeof(buf2));
                io__getline(&io, &line, &line_len);
                if (line_len > 0 && line[line_len - 1] == '\n')
                        line[line_len - 1] = '\0';
-               hwmon_pmu__new(pmus, hwmon_dir, class_hwmon_ent->d_name, line);
+               hwmon_pmu__new(pmus, buf, class_hwmon_ent->d_name, line);
                close(name_fd);
+               close(hwmon_dir);
        }
        free(line);
        close(class_hwmon_dir.dirfd);
@@ -753,6 +765,10 @@ int evsel__hwmon_pmu_open(struct evsel *evsel,
                .type_and_num = evsel->core.attr.config,
        };
        int idx = 0, thread = 0, nthreads, err = 0;
+       int dir = open(hwm->hwmon_dir, O_CLOEXEC | O_DIRECTORY | O_RDONLY);
+
+       if (dir < 0)
+               return -errno;
 
        nthreads = perf_thread_map__nr(threads);
        for (idx = start_cpu_map_idx; idx < end_cpu_map_idx; idx++) {
@@ -763,7 +779,7 @@ int evsel__hwmon_pmu_open(struct evsel *evsel,
                        snprintf(buf, sizeof(buf), "%s%d_input",
                                 hwmon_type_strs[key.type], key.num);
 
-                       fd = openat(hwm->hwmon_dir_fd, buf, O_RDONLY);
+                       fd = openat(dir, buf, O_RDONLY);
                        FD(evsel, idx, thread) = fd;
                        if (fd < 0) {
                                err = -errno;
@@ -771,6 +787,7 @@ int evsel__hwmon_pmu_open(struct evsel *evsel,
                        }
                }
        }
+       close(dir);
        return 0;
 out_close:
        if (err)
@@ -784,6 +801,7 @@ out_close:
                }
                thread = nthreads;
        } while (--idx >= 0);
+       close(dir);
        return err;
 }
 

diff --git a/tools/perf/util/hwmon_pmu.h b/tools/perf/util/hwmon_pmu.h
index b3329774d2b22d0553da558cd70268a64af61455..dc711b289ff566f0b73ab44b4c2d7c0afb5cff07 100644 (file)
--- a/tools/perf/util/hwmon_pmu.h
+++ b/tools/perf/util/hwmon_pmu.h
@@ -135,14 +135,14 @@ bool parse_hwmon_filename(const char *filename,
  * hwmon_pmu__new() - Allocate and construct a hwmon PMU.
  *
  * @pmus: The list of PMUs to be added to.
- * @hwmon_dir: An O_DIRECTORY file descriptor for a hwmon directory.
+ * @hwmon_dir: The path to a hwmon directory.
  * @sysfs_name: Name of the hwmon sysfs directory like hwmon0.
  * @name: The contents of the "name" file in the hwmon directory.
  *
  * Exposed for testing. Regular construction should happen via
  * perf_pmus__read_hwmon_pmus.
  */
-struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, int hwmon_dir,
+struct perf_pmu *hwmon_pmu__new(struct list_head *pmus, const char *hwmon_dir,
                                const char *sysfs_name, const char *name);
 void hwmon_pmu__exit(struct perf_pmu *pmu);
 

diff --git a/tools/perf/util/pmus.c b/tools/perf/util/pmus.c
index 81c2ed689db21e09e2b56194652084bb8ede973f..409b909cfa02744b62cddb62be8b0334e5b921fe 100644 (file)
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -861,7 +861,7 @@ struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs_dirfd, const char *name)
        return perf_pmu__lookup(&other_pmus, test_sysfs_dirfd, name, /*eager_load=*/true);
 }
 
-struct perf_pmu *perf_pmus__add_test_hwmon_pmu(int hwmon_dir,
+struct perf_pmu *perf_pmus__add_test_hwmon_pmu(const char *hwmon_dir,
                                               const char *sysfs_name,
                                               const char *name)
 {

diff --git a/tools/perf/util/pmus.h b/tools/perf/util/pmus.h
index 33ecf765a92f6538f2721b78625e0f457baf6b8c..86842ee5f539a512af31566a33b5f1fcf11d3f10 100644 (file)
--- a/tools/perf/util/pmus.h
+++ b/tools/perf/util/pmus.h
@@ -31,7 +31,7 @@ int perf_pmus__num_core_pmus(void);
 bool perf_pmus__supports_extended_type(void);
 
 struct perf_pmu *perf_pmus__add_test_pmu(int test_sysfs_dirfd, const char *name);
-struct perf_pmu *perf_pmus__add_test_hwmon_pmu(int hwmon_dir,
+struct perf_pmu *perf_pmus__add_test_hwmon_pmu(const char *hwmon_dir,
                                               const char *sysfs_name,
                                               const char *name);
 struct perf_pmu *perf_pmus__fake_pmu(void);