Skip to contentSkip to conversation

perf test: Directory file descriptor leak

PATCH New Ian Rogers 1 participant 4 messages Jun 17, 2025 perf events

lore
Share & tools
lore

Ian Rogers

author reviewer
#0 lore raw

PATCH v2 1/4 perf test: Directory file descriptor leak

Add missed close when iterating over the script directories.

Fixes f3295f5b067d ("perf tests: Use scandirat for shell script finding")
Signed-off-by Ian Rogers <[email protected]>
---
 tools/perf/tests/tests-scripts.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/tests/tests-scripts.c +1 -0
index 1d5759d08141..3a2a8438f9af 100644
@@ -260,6 +260,7 @@ static void append_scripts_in_dir(int dir_fd,
 			continue; /* Skip scripts that have a separate driver. */
 		fd = openat(dir_fd, ent->d_name, O_PATH);
 		append_scripts_in_dir(fd, result, result_sz);
+		close(fd);
 	}
 	for (i = 0; i < n_dirs; i++) /* Clean up */
 		zfree(&entlist[i]);
Signature · 1 line
2.50.0.rc2.761.g2dc52ea45b-goog
author reviewer
#1 lore raw

PATCH v2 2/4 perf evsel: Missed close when probing hybrid core PMUs

Add missing close to avoid leaking perf events. In past perfs this mattered little as the function was just used by perf list. As the function is now used to detect hybrid PMUs leaking the perf event is somewhat more painful.

Fixes b41f1cec91c3 ("perf list: Skip unsupported events")
Signed-off-by Ian Rogers <[email protected]>
---
 tools/perf/util/print-events.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/util/print-events.c +1 -0
index a786cbfb0ff5..83aaf7cda635 100644
@@ -268,6 +268,7 @@ bool is_event_supported(u8 type, u64 config)
 			ret = evsel__open(evsel, NULL, tmap) >= 0;
 		}
 
+		evsel__close(evsel);
 		evsel__delete(evsel);
 	}
Signature · 1 line
2.50.0.rc2.761.g2dc52ea45b-goog
author reviewer
#2 lore raw

PATCH v2 3/4 perf trace: Add missed freeing of ordered events and thread

In reply to Ian Rogers

Caught by leak sanitizer running "perf trace BTF general tests". Make the ordered_events initialization unconditional and early so that trace__exit cleanup is simple - ordered_events__init doesn't allocate and just sets up 4 values and inits 3 list heads.

Signed-off-by Ian Rogers <[email protected]>

---
v2: Move the init code to avoid segv found by Arnaldo.

---
 tools/perf/builtin-trace.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
tools/perf/builtin-trace.c +5 -5
index bf9b5d0630d3..d50e86fccf60 100644
@@ -5359,6 +5359,7 @@ static int trace__config(const char *var, const char *value, void *arg)
 
 static void trace__exit(struct trace *trace)
 {
+	thread__zput(trace->current);
 	strlist__delete(trace->ev_qualifier);
 	zfree(&trace->ev_qualifier_ids.entries);
 	if (trace->syscalls.table) {

@@ -5369,6 +5370,7 @@ static void trace__exit(struct trace *trace)
 	zfree(&trace->perfconfig_events);
 	evlist__delete(trace->evlist);
 	trace->evlist = NULL;
+	ordered_events__free(&trace->oe.data);
 #ifdef HAVE_LIBBPF_SUPPORT
 	btf__free(trace->btf);
 	trace->btf = NULL;

@@ -5518,6 +5520,9 @@ int cmd_trace(int argc, const char **argv)
 	sigchld_act.sa_sigaction = sighandler_chld;
 	sigaction(SIGCHLD, &sigchld_act, NULL);
 
+	ordered_events__init(&trace.oe.data, ordered_events__deliver_event, &trace);
+	ordered_events__set_copy_on_queue(&trace.oe.data, true);
+
 	trace.evlist = evlist__new();
 
 	if (trace.evlist == NULL) {

@@ -5676,11 +5681,6 @@ int cmd_trace(int argc, const char **argv)
 			trace__load_vmlinux_btf(&trace);
 	}
 
-	if (trace.sort_events) {
-		ordered_events__init(&trace.oe.data, ordered_events__deliver_event, &trace);
-		ordered_events__set_copy_on_queue(&trace.oe.data, true);
-	}
-
 	/*
 	 * If we are augmenting syscalls, then combine what we put in the
 	 * __augmented_syscalls__ BPF map with what is in the
Signature · 1 line
2.50.0.rc2.761.g2dc52ea45b-goog
author reviewer
#3 lore raw

PATCH v2 4/4 libperf evsel: In exit add missed puts and assert close, etc. were called

In reply to Ian Rogers

A missed evsel__close before evsel__delete was the source of leaking perf events due to a hybrid test. Add asserts in debug builds so that this shouldn't happen in the future. Add puts missing on the cpu map and thread maps.

Signed-off-by Ian Rogers <[email protected]>
---
 tools/lib/perf/evsel.c | 6 ++++++
 1 file changed, 6 insertions(+)
tools/lib/perf/evsel.c +6 -0
index c475319e2e41..2a85e0bfee1e 100644
@@ -42,6 +42,12 @@ struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr)
 
 void perf_evsel__delete(struct perf_evsel *evsel)
 {
+	assert(evsel->fd == NULL);  /* If not fds were not closed. */
+	assert(evsel->mmap == NULL); /* If not munmap wasn't called. */
+	assert(evsel->sample_id == NULL); /* If not free_id wasn't called. */
+	perf_cpu_map__put(evsel->cpus);
+	perf_cpu_map__put(evsel->own_cpus);
+	perf_thread_map__put(evsel->threads);
 	free(evsel);
 }
Signature · 1 line
2.50.0.rc2.761.g2dc52ea45b-goog