perf test: Directory file descriptor leak

Add missed close when iterating over the script directories.

---
 tools/perf/tests/tests-scripts.c | 1 +
 1 file changed, 1 insertion(+)

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]);

2.50.0.rc1.591.g9c95f17f64-goog

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.

---
 tools/perf/util/print-events.c | 1 +
 1 file changed, 1 insertion(+)

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);
 	}
 

2.50.0.rc1.591.g9c95f17f64-goog

Given this leads to leaking perf events on hybrid systems it would be nice to land this 1 liner in the next v6.15 rc pull request fixes if possible.

Thanks,
Ian

Sure.

Just one nit: while looking at:

void evsel__close(struct evsel *evsel)

{
        if (evsel__is_retire_lat(evsel))
                evsel__tpebs_close(evsel);
        perf_evsel__close(&evsel->core);
        perf_evsel__free_id(&evsel->core);
}

And then:

void evsel__delete(struct evsel *evsel)

{       
        if (!evsel)
                return;

        evsel__exit(evsel);
        free(evsel);
}

and:

void evsel__exit(struct evsel *evsel)

{       
        assert(list_empty(&evsel->core.node));
        assert(evsel->evlist == NULL);
        if (evsel__is_retire_lat(evsel))
                evsel__tpebs_close(evsel);

<SNIP>

I think that tpebs_close could be done just at evsel__close(), no?

The way it works I think there is no problem with calling it now twice, but it fits better in evsel__close().

- Arnaldo

Agreed. It'd be nice if we could assert that it was closed given this has been a problem. Retirement latency events are pretty unusual so I'm not overly worried about needing this :-)

Thanks,
Ian

Caught by leak sanitizer running "perf trace BTF general tests".

---
 tools/perf/builtin-trace.c | 2 ++
 1 file changed, 2 insertions(+)

index bf9b5d0630d3..39b6a13ca862 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;

2.50.0.rc1.591.g9c95f17f64-goog

Hello Ian,

before:
~~~

$ sudo valgrind --leak-check=full /tmp/perf/perf trace --sort-events
-e openat* --max-events=1 -- echo 1
==248676== Memcheck, a memory error detector
==248676== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==248676== Using Valgrind-3.23.0 and LibVEX; rerun with -h for copyright info
==248676== Command: /tmp/perf/perf trace --sort-events -e openat*
--max-events=1 -- echo 1
...
==248676== 3,842 (256 direct, 3,586 indirect) bytes in 1 blocks are
definitely lost in loss record 2,773 of 2,808
==248676==    at 0x484FBE3: calloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==248676==    by 0x3634E1: thread__new (in /tmp/perf/perf)
==248676==    by 0x363BBE: threads__findnew (in /tmp/perf/perf)
==248676==    by 0x34DAB6: machine__findnew_thread (in /tmp/perf/perf)
==248676==    by 0x34E655: machine__process_comm_event (in /tmp/perf/perf)
==248676==    by 0x2071B1: __trace__deliver_event.isra.0 (in /tmp/perf/perf)
==248676==    by 0x2072EA: ordered_events__deliver_event (in /tmp/perf/perf)
==248676==    by 0x3608D2: __ordered_events__flush (in /tmp/perf/perf)
==248676==    by 0x20CB19: trace__run (in /tmp/perf/perf)
==248676==    by 0x21183D: cmd_trace (in /tmp/perf/perf)
==248676==    by 0x215E6F: run_builtin (in /tmp/perf/perf)
==248676==    by 0x21618A: handle_internal_command (in /tmp/perf/perf)
...
==248676== 4,992 bytes in 16 blocks are definitely lost in loss record
2,776 of 2,808
==248676==    at 0x484FE10: realloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==248676==    by 0x57CCBCD: __vasprintf_internal (vasprintf.c:123)
==248676==    by 0x5878152: __asprintf_chk (asprintf_chk.c:34)
==248676==    by 0x31ED18: parse_events_formats_error_string (in /tmp/perf/perf)
==248676==    by 0x36DCA7: perf_pmu__config_terms (in /tmp/perf/perf)
==248676==    by 0x370CD8: perf_pmu__name_from_config (in /tmp/perf/perf)
==248676==    by 0x43216A: arch_is_topdown_metrics (in /tmp/perf/perf)
==248676==    by 0x432AD7: arch_evsel__must_be_in_group (in /tmp/perf/perf)
==248676==    by 0x31C657: __parse_events (in /tmp/perf/perf)
==248676==    by 0x31CD1E: parse_event (in /tmp/perf/perf)
==248676==    by 0x3A2D76: perf_can_comm_exec (in /tmp/perf/perf)
==248676==    by 0x3A374F: evlist__config (in /tmp/perf/perf)
...
==248676==
==248676== LEAK SUMMARY:
==248676==    definitely lost: 5,248 bytes in 17 blocks
==248676==    indirectly lost: 3,586 bytes in 12 blocks
==248676==      possibly lost: 311,943 bytes in 5,292 blocks
==248676==    still reachable: 740,220 bytes in 21,186 blocks
==248676==         suppressed: 0 bytes in 0 blocks
==248676== Reachable blocks (those to which a pointer was found) are not shown.
==248676== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==248676==
==248676== For lists of detected and suppressed errors, rerun with: -s
==248676== ERROR SUMMARY: 56 errors from 56 contexts (suppressed: 0 from 0)
~~~

after:
~~~

$ sudo valgrind --leak-check=full /tmp/perf/perf trace --sort-events
-e openat* --max-events=1 -- echo 1
==245044== Memcheck, a memory error detector
==245044== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==245044== Using Valgrind-3.23.0 and LibVEX; rerun with -h for copyright info
==245044== Command: /tmp/perf/perf trace --sort-events -e openat*
--max-events=1 -- echo 1
==245044==
--245044-- WARNING: unhandled eBPF command 36
--245044-- WARNING: unhandled eBPF command 28
--245044-- WARNING: unhandled eBPF command 28
--245044-- WARNING: unhandled eBPF command 28
--245044-- WARNING: unhandled eBPF command 28
--245044-- WARNING: unhandled eBPF command 28
libbpf: failed to read kernel BTF from '/sys/kernel/btf/vmlinux': -ENODEV
libbpf: failed to read kernel BTF from '/sys/kernel/btf/vmlinux': -ENODEV
libbpf: failed to read kernel BTF from '/sys/kernel/btf/vmlinux': -ENODEV
...
==245044== 4,992 bytes in 16 blocks are definitely lost in loss record
2,763 of 2,794
==245044==    at 0x484FE10: realloc (in
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==245044==    by 0x57CCBCD: __vasprintf_internal (vasprintf.c:123)
==245044==    by 0x5878152: __asprintf_chk (asprintf_chk.c:34)
==245044==    by 0x31ED98: parse_events_formats_error_string (in /tmp/perf/perf)
==245044==    by 0x36DD27: perf_pmu__config_terms (in /tmp/perf/perf)
==245044==    by 0x370D58: perf_pmu__name_from_config (in /tmp/perf/perf)
==245044==    by 0x4321EA: arch_is_topdown_metrics (in /tmp/perf/perf)
==245044==    by 0x432B57: arch_evsel__must_be_in_group (in /tmp/perf/perf)
==245044==    by 0x31C6D7: __parse_events (in /tmp/perf/perf)
==245044==    by 0x31CD9E: parse_event (in /tmp/perf/perf)
==245044==    by 0x3A2DF6: perf_can_comm_exec (in /tmp/perf/perf)
==245044==    by 0x3A37CF: evlist__config (in /tmp/perf/perf)
...
==245044== LEAK SUMMARY:
==245044==    definitely lost: 4,992 bytes in 16 blocks
==245044==    indirectly lost: 0 bytes in 0 blocks
==245044==      possibly lost: 245,988 bytes in 5,288 blocks
==245044==    still reachable: 740,211 bytes in 21,185 blocks
==245044==         suppressed: 0 bytes in 0 blocks
==245044== Reachable blocks (those to which a pointer was found) are not shown.
==245044== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==245044==
==245044== For lists of detected and suppressed errors, rerun with: -s
==245044== ERROR SUMMARY: 51 errors from 51 contexts (suppressed: 0 from 0)
~~~

TL;DR: (definitely lost: 5,248 bytes in 17 blocks, 3,586 bytes in 12 blocks) -> (definitely lost: 4,992 bytes in 16 blocks, indirectly
lost: 0 bytes in 0 blocks), the leak of thread__new() is fixed.

Thank you so much for fixing this.

Thanks,
Howard

Small enough, applied to perf-tools.

- Arnaldo

root@number:~# perf trace -e *sleep ls
anaconda-ks.cfg  bin  bla  commands  dtel  firefly  logind.conf  perf-install.txt  python
perf: Segmentation fault
Obtained 11 stack frames.
perf() [0x5c595e]
perf() [0x5c59f9]
/lib64/libc.so.6(+0x19c30) [0x7fd43ce27c30]
perf() [0x5dc497]
perf() [0x492d54]
perf() [0x49860e]
perf() [0x49890e]
perf() [0x413413]
/lib64/libc.so.6(+0x35f5) [0x7fd43ce115f5]
/lib64/libc.so.6(__libc_start_main+0x88) [0x7fd43ce116a8]
perf() [0x413a45]
Segmentation fault (core dumped)
root@number:~# 

⬢ [acme@toolbx perf-tools]$ git bisect good e340815ea559052d8d590a145bab7d7105608e7d is the first bad commit commit e340815ea559052d8d590a145bab7d7105608e7d

Author: Ian Rogers <[email protected]>
Date:   Fri Jun 13 17:41:06 2025 -0700

perf trace: Add missed freeing of ordered events and thread

Caught by leak sanitizer running "perf trace BTF general tests".

 tools/perf/builtin-trace.c | 2 ++
 1 file changed, 2 insertions(+)

⬢ [acme@toolbx perf-tools]$

Removing it:

root@number:~# perf trace -e *sleep sleep 1
     0.000 (1000.057 ms): sleep/2693449 clock_nanosleep(rqtp: { .tv_sec: 1, .tv_nsec: 0 }, rmtp: 0x7fffedc43ab0) = 0
root@number:~#

- Arnaldo

Thanks, I'll take a look to see if I can spot what's broken. Seeing this stack trace makes me remember we haven't landed:
https://lore.kernel.org/lkml/[email protected]/

Ian

Yeah, I just pushed perf-tools to soak in linux-next/pending-fixes for a few days and will switch to processing patches for perf-tools-next, will try and pick that one, I also noticed that the backtrace wasn't symbolized, thus your patch wasn't there :-\

- Arnaldo

I sent out a fix for this issue in v2:
https://lore.kernel.org/lkml/[email protected]/
Patches 1 and 2 I see are in the v6.16 fixes PR:
https://lore.kernel.org/lkml/[email protected]/

Thanks,
Ian

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.

---
 tools/lib/perf/evsel.c | 6 ++++++
 1 file changed, 6 insertions(+)

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);
 }
 

2.50.0.rc1.591.g9c95f17f64-goog