* [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
@ 2025-06-24 19:03 Ian Rogers
2025-06-24 19:03 ` [PATCH v1 1/5] perf dso: Add missed dso__put to dso__load_kcore Ian Rogers
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Ian Rogers @ 2025-06-24 19:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Howard Chu,
Charlie Jenkins, Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
Some recent patches showed we were leaking file descriptors:
https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/
When a test is forked the file descriptors >3 can be closed and then
any file descriptors >3 left after the test are leaks. Add this
checking to the forked test code. Prior to that clean up some file
descriptor usage so we can assert that only file descriptors are
cleaned up. Sometimes the file descriptor being held open is the
result of a memory leak, so fix those.
Ian Rogers (5):
perf dso: Add missed dso__put to dso__load_kcore
perf test code-reading: Avoid a leak of cpus and threads
perf hwmon_pmu: Hold path rather than fd
perf dso: With ref count checking, avoid dso_data holding dso live
perf test: In forked mode add check that fds aren't leaked
tools/perf/tests/builtin-test.c | 69 +++++++++++++++++++++++++++++++++
tools/perf/tests/code-reading.c | 7 ----
tools/perf/tests/hwmon_pmu.c | 11 +++---
tools/perf/util/dso.c | 4 ++
tools/perf/util/hwmon_pmu.c | 38 +++++++++++++-----
tools/perf/util/hwmon_pmu.h | 4 +-
tools/perf/util/pmus.c | 2 +-
tools/perf/util/pmus.h | 2 +-
tools/perf/util/symbol.c | 1 +
9 files changed, 112 insertions(+), 26 deletions(-)
--
2.50.0.714.g196bf9f422-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/5] perf dso: Add missed dso__put to dso__load_kcore
2025-06-24 19:03 [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Ian Rogers
@ 2025-06-24 19:03 ` Ian Rogers
2025-06-24 19:03 ` [PATCH v1 2/5] perf test code-reading: Avoid a leak of cpus and threads Ian Rogers
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2025-06-24 19:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Howard Chu,
Charlie Jenkins, Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
The kcore loading creates a set of list nodes that have reference
counted references to maps of the kcore. The list node freeing in the
success path wasn't releasing the maps, add the missing puts. It is
unclear why this leak was being missed by leak sanitizer.
Fixes: 83720209961f ("perf map: Move map list node into symbol")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/symbol.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8b30c6f16a9e..fd4583718eab 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1422,6 +1422,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
goto out_err;
}
}
+ map__zput(new_node->map);
free(new_node);
}
--
2.50.0.714.g196bf9f422-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 2/5] perf test code-reading: Avoid a leak of cpus and threads
2025-06-24 19:03 [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Ian Rogers
2025-06-24 19:03 ` [PATCH v1 1/5] perf dso: Add missed dso__put to dso__load_kcore Ian Rogers
@ 2025-06-24 19:03 ` Ian Rogers
2025-06-24 19:03 ` [PATCH v1 3/5] perf hwmon_pmu: Hold path rather than fd Ian Rogers
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2025-06-24 19:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Howard Chu,
Charlie Jenkins, Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
The perf_evlist__set_maps does the necessary gets on the arguments
passed, so the reference count bumping isn't necessary and creates a
memory leak.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/code-reading.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index cf6edbe697b2..6efb6b4bbcce 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -749,13 +749,6 @@ static int do_test_code_reading(bool try_kcore)
pr_debug("perf_evlist__open() failed!\n%s\n", errbuf);
}
- /*
- * Both cpus and threads are now owned by evlist
- * and will be freed by following perf_evlist__set_maps
- * call. Getting reference to keep them alive.
- */
- perf_cpu_map__get(cpus);
- perf_thread_map__get(threads);
perf_evlist__set_maps(&evlist->core, NULL, NULL);
evlist__delete(evlist);
evlist = NULL;
--
2.50.0.714.g196bf9f422-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 3/5] perf hwmon_pmu: Hold path rather than fd
2025-06-24 19:03 [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Ian Rogers
2025-06-24 19:03 ` [PATCH v1 1/5] perf dso: Add missed dso__put to dso__load_kcore Ian Rogers
2025-06-24 19:03 ` [PATCH v1 2/5] perf test code-reading: Avoid a leak of cpus and threads Ian Rogers
@ 2025-06-24 19:03 ` Ian Rogers
2025-06-24 19:03 ` [PATCH v1 4/5] perf dso: With ref count checking, avoid dso_data holding dso live Ian Rogers
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2025-06-24 19:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Howard Chu,
Charlie Jenkins, Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
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>
---
tools/perf/tests/hwmon_pmu.c | 11 ++++++-----
tools/perf/util/hwmon_pmu.c | 38 ++++++++++++++++++++++++++----------
tools/perf/util/hwmon_pmu.h | 4 ++--
tools/perf/util/pmus.c | 2 +-
tools/perf/util/pmus.h | 2 +-
5 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/tools/perf/tests/hwmon_pmu.c b/tools/perf/tests/hwmon_pmu.c
index 0837aca1cdfa..151f02701c8c 100644
--- 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 c25e7296f1c1..7edda010ba27 100644
--- 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 @@ static int hwmon_pmu__read_events(struct hwmon_pmu *pmu)
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 @@ int evsel__hwmon_pmu_open(struct evsel *evsel,
}
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 b3329774d2b2..dc711b289ff5 100644
--- 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 3bbd26fec78a..eed8d8005cff 100644
--- a/tools/perf/util/pmus.c
+++ b/tools/perf/util/pmus.c
@@ -762,7 +762,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 8def20e615ad..d5140804a630 100644
--- a/tools/perf/util/pmus.h
+++ b/tools/perf/util/pmus.h
@@ -29,7 +29,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);
--
2.50.0.714.g196bf9f422-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 4/5] perf dso: With ref count checking, avoid dso_data holding dso live
2025-06-24 19:03 [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Ian Rogers
` (2 preceding siblings ...)
2025-06-24 19:03 ` [PATCH v1 3/5] perf hwmon_pmu: Hold path rather than fd Ian Rogers
@ 2025-06-24 19:03 ` Ian Rogers
2025-06-24 19:03 ` [PATCH v1 5/5] perf test: In forked mode add check that fds aren't leaked Ian Rogers
2025-07-03 2:20 ` [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Namhyung Kim
5 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2025-06-24 19:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Howard Chu,
Charlie Jenkins, Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
With the dso_data embedded in a dso there is a reference counted
pointer to the dso rather than using container_of with reference count
checking. This data can hold the dso live meaning that no dso__put
ever deletes it. Add a check for this case and close the dso_data when
it happens. There isn't an infinite loop as the dso_data clears the
file descriptor prior to putting on the dso.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/dso.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 057fcf4225ac..c6c1637e098c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1612,6 +1612,10 @@ struct dso *dso__get(struct dso *dso)
void dso__put(struct dso *dso)
{
+#ifdef REFCNT_CHECKING
+ if (dso && dso__data(dso) && refcount_read(&RC_CHK_ACCESS(dso)->refcnt) == 2)
+ dso__data_close(dso);
+#endif
if (dso && refcount_dec_and_test(&RC_CHK_ACCESS(dso)->refcnt))
dso__delete(dso);
else
--
2.50.0.714.g196bf9f422-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 5/5] perf test: In forked mode add check that fds aren't leaked
2025-06-24 19:03 [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Ian Rogers
` (3 preceding siblings ...)
2025-06-24 19:03 ` [PATCH v1 4/5] perf dso: With ref count checking, avoid dso_data holding dso live Ian Rogers
@ 2025-06-24 19:03 ` Ian Rogers
2025-07-03 2:20 ` [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Namhyung Kim
5 siblings, 0 replies; 9+ messages in thread
From: Ian Rogers @ 2025-06-24 19:03 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, Howard Chu,
Charlie Jenkins, Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
When a test is forked no file descriptors should be open, however,
parent ones may have been inherited - in particular those of the pipes
of other forked child test processes. Add a loop to clean-up/close
those file descriptors prior to running the test. At the end of the
test assert that no additional file descriptors are present as this
would indicate a file descriptor leak.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/builtin-test.c | 69 +++++++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 45d3d8b3317a..4061d5d969aa 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -4,6 +4,7 @@
*
* Builtin regression testing command: ever growing number of sanity tests
*/
+#include <ctype.h>
#include <fcntl.h>
#include <errno.h>
#include <poll.h>
@@ -155,6 +156,71 @@ static struct test_workload *workloads[] = {
#define test_suite__for_each_test_case(suite, idx) \
for (idx = 0; (suite)->test_cases && (suite)->test_cases[idx].name != NULL; idx++)
+static void close_parent_fds(void)
+{
+ DIR *dir = opendir("/proc/self/fd");
+ struct dirent *ent;
+
+ while ((ent = readdir(dir))) {
+ char *end;
+ long fd;
+
+ if (ent->d_type != DT_LNK)
+ continue;
+
+ if (!isdigit(ent->d_name[0]))
+ continue;
+
+ fd = strtol(ent->d_name, &end, 10);
+ if (*end)
+ continue;
+
+ if (fd <= 3 || fd == dirfd(dir))
+ continue;
+
+ close(fd);
+ }
+ closedir(dir);
+}
+
+static void check_leaks(void)
+{
+ DIR *dir = opendir("/proc/self/fd");
+ struct dirent *ent;
+ int leaks = 0;
+
+ while ((ent = readdir(dir))) {
+ char path[PATH_MAX];
+ char *end;
+ long fd;
+ ssize_t len;
+
+ if (ent->d_type != DT_LNK)
+ continue;
+
+ if (!isdigit(ent->d_name[0]))
+ continue;
+
+ fd = strtol(ent->d_name, &end, 10);
+ if (*end)
+ continue;
+
+ if (fd <= 3 || fd == dirfd(dir))
+ continue;
+
+ leaks++;
+ len = readlinkat(dirfd(dir), ent->d_name, path, sizeof(path));
+ if (len > 0 && (size_t)len < sizeof(path))
+ path[len] = '\0';
+ else
+ strncpy(path, ent->d_name, sizeof(path));
+ pr_err("Leak of file descriptor %s that opened: '%s'\n", ent->d_name, path);
+ }
+ closedir(dir);
+ if (leaks)
+ abort();
+}
+
static int test_suite__num_test_cases(const struct test_suite *t)
{
int num;
@@ -242,6 +308,8 @@ static int run_test_child(struct child_process *process)
struct child_test *child = container_of(process, struct child_test, process);
int err;
+ close_parent_fds();
+
err = sigsetjmp(run_test_jmp_buf, 1);
if (err) {
fprintf(stderr, "\n---- unexpected signal (%d) ----\n", err);
@@ -257,6 +325,7 @@ static int run_test_child(struct child_process *process)
err = test_function(child->test, child->test_case_num)(child->test, child->test_case_num);
pr_debug("---- end(%d) ----\n", err);
+ check_leaks();
err_out:
fflush(NULL);
for (size_t i = 0; i < ARRAY_SIZE(signals); i++)
--
2.50.0.714.g196bf9f422-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
2025-06-24 19:03 [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Ian Rogers
` (4 preceding siblings ...)
2025-06-24 19:03 ` [PATCH v1 5/5] perf test: In forked mode add check that fds aren't leaked Ian Rogers
@ 2025-07-03 2:20 ` Namhyung Kim
2025-07-03 3:03 ` Ian Rogers
5 siblings, 1 reply; 9+ messages in thread
From: Namhyung Kim @ 2025-07-03 2:20 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, James Clark, Howard Chu, Charlie Jenkins,
Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Tue, Jun 24, 2025 at 12:03:20PM -0700, Ian Rogers wrote:
> Some recent patches showed we were leaking file descriptors:
> https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/
>
> When a test is forked the file descriptors >3 can be closed and then
> any file descriptors >3 left after the test are leaks. Add this
> checking to the forked test code. Prior to that clean up some file
> descriptor usage so we can assert that only file descriptors are
> cleaned up. Sometimes the file descriptor being held open is the
> result of a memory leak, so fix those.
Interesting, I can see a few more tests are failing with this. But we
can figure it out later.
Thanks,
Namhyung
>
> Ian Rogers (5):
> perf dso: Add missed dso__put to dso__load_kcore
> perf test code-reading: Avoid a leak of cpus and threads
> perf hwmon_pmu: Hold path rather than fd
> perf dso: With ref count checking, avoid dso_data holding dso live
> perf test: In forked mode add check that fds aren't leaked
>
> tools/perf/tests/builtin-test.c | 69 +++++++++++++++++++++++++++++++++
> tools/perf/tests/code-reading.c | 7 ----
> tools/perf/tests/hwmon_pmu.c | 11 +++---
> tools/perf/util/dso.c | 4 ++
> tools/perf/util/hwmon_pmu.c | 38 +++++++++++++-----
> tools/perf/util/hwmon_pmu.h | 4 +-
> tools/perf/util/pmus.c | 2 +-
> tools/perf/util/pmus.h | 2 +-
> tools/perf/util/symbol.c | 1 +
> 9 files changed, 112 insertions(+), 26 deletions(-)
>
> --
> 2.50.0.714.g196bf9f422-goog
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
2025-07-03 2:20 ` [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Namhyung Kim
@ 2025-07-03 3:03 ` Ian Rogers
2025-07-03 18:18 ` Namhyung Kim
0 siblings, 1 reply; 9+ messages in thread
From: Ian Rogers @ 2025-07-03 3:03 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, James Clark, Howard Chu, Charlie Jenkins,
Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Wed, Jul 2, 2025 at 7:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Tue, Jun 24, 2025 at 12:03:20PM -0700, Ian Rogers wrote:
> > Some recent patches showed we were leaking file descriptors:
> > https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/
> >
> > When a test is forked the file descriptors >3 can be closed and then
> > any file descriptors >3 left after the test are leaks. Add this
> > checking to the forked test code. Prior to that clean up some file
> > descriptor usage so we can assert that only file descriptors are
> > cleaned up. Sometimes the file descriptor being held open is the
> > result of a memory leak, so fix those.
>
> Interesting, I can see a few more tests are failing with this. But we
> can figure it out later.
That's cool. I was a little disappointed that just the dso kcore leak
was found by this. I was also surprised that the dso kcore memory leak
hadn't shown up with leak sanitizer and reference count checking. Let
me know if there is anything more I need to do to the patch series.
Thanks,
Ian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up
2025-07-03 3:03 ` Ian Rogers
@ 2025-07-03 18:18 ` Namhyung Kim
0 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2025-07-03 18:18 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, James Clark, Howard Chu, Charlie Jenkins,
Thomas Richter, Athira Rajeev, Stephen Brennan,
Jean-Philippe Romain, Junhao He, Dr. David Alan Gilbert,
Dmitry Vyukov, linux-perf-users, linux-kernel
On Wed, Jul 02, 2025 at 08:03:08PM -0700, Ian Rogers wrote:
> On Wed, Jul 2, 2025 at 7:20 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Tue, Jun 24, 2025 at 12:03:20PM -0700, Ian Rogers wrote:
> > > Some recent patches showed we were leaking file descriptors:
> > > https://lore.kernel.org/lkml/20250617223356.2752099-2-irogers@google.com/
> > >
> > > When a test is forked the file descriptors >3 can be closed and then
> > > any file descriptors >3 left after the test are leaks. Add this
> > > checking to the forked test code. Prior to that clean up some file
> > > descriptor usage so we can assert that only file descriptors are
> > > cleaned up. Sometimes the file descriptor being held open is the
> > > result of a memory leak, so fix those.
> >
> > Interesting, I can see a few more tests are failing with this. But we
> > can figure it out later.
>
> That's cool. I was a little disappointed that just the dso kcore leak
> was found by this. I was also surprised that the dso kcore memory leak
> hadn't shown up with leak sanitizer and reference count checking. Let
> me know if there is anything more I need to do to the patch series.
Applied to perf-tools-next, thanks!
Best Regards,
Namhyung
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-07-03 18:18 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-24 19:03 [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Ian Rogers
2025-06-24 19:03 ` [PATCH v1 1/5] perf dso: Add missed dso__put to dso__load_kcore Ian Rogers
2025-06-24 19:03 ` [PATCH v1 2/5] perf test code-reading: Avoid a leak of cpus and threads Ian Rogers
2025-06-24 19:03 ` [PATCH v1 3/5] perf hwmon_pmu: Hold path rather than fd Ian Rogers
2025-06-24 19:03 ` [PATCH v1 4/5] perf dso: With ref count checking, avoid dso_data holding dso live Ian Rogers
2025-06-24 19:03 ` [PATCH v1 5/5] perf test: In forked mode add check that fds aren't leaked Ian Rogers
2025-07-03 2:20 ` [PATCH v1 0/5] perf test: Sanity check file descriptors are cleaned up Namhyung Kim
2025-07-03 3:03 ` Ian Rogers
2025-07-03 18:18 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).