* [PATCH v1 1/4] perf test: Directory file descriptor leak
@ 2025-06-14 0:41 Ian Rogers
2025-06-14 0:41 ` [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs Ian Rogers
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Ian Rogers @ 2025-06-14 0:41 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, Michael Petlan, Andi Kleen,
Jiapeng Chong, Tiezhu Yang, linux-perf-users, linux-kernel
Add missed close when iterating over the script directories.
Fixes: f3295f5b067d ("perf tests: Use scandirat for shell script finding")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/tests/tests-scripts.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/tests/tests-scripts.c b/tools/perf/tests/tests-scripts.c
index 1d5759d08141..3a2a8438f9af 100644
--- a/tools/perf/tests/tests-scripts.c
+++ b/tools/perf/tests/tests-scripts.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs
2025-06-14 0:41 [PATCH v1 1/4] perf test: Directory file descriptor leak Ian Rogers
@ 2025-06-14 0:41 ` Ian Rogers
2025-06-16 16:04 ` Ian Rogers
2025-06-14 0:41 ` [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread Ian Rogers
2025-06-14 0:41 ` [PATCH v1 4/4] libperf evsel: In exit add missed puts and assert close, etc. were called Ian Rogers
2 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-06-14 0:41 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, Michael Petlan, Andi Kleen,
Jiapeng Chong, Tiezhu Yang, linux-perf-users, linux-kernel
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 <irogers@google.com>
---
tools/perf/util/print-events.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
index a786cbfb0ff5..83aaf7cda635 100644
--- a/tools/perf/util/print-events.c
+++ b/tools/perf/util/print-events.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread
2025-06-14 0:41 [PATCH v1 1/4] perf test: Directory file descriptor leak Ian Rogers
2025-06-14 0:41 ` [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs Ian Rogers
@ 2025-06-14 0:41 ` Ian Rogers
2025-06-14 4:16 ` Howard Chu
2025-06-14 0:41 ` [PATCH v1 4/4] libperf evsel: In exit add missed puts and assert close, etc. were called Ian Rogers
2 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-06-14 0:41 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, Michael Petlan, Andi Kleen,
Jiapeng Chong, Tiezhu Yang, linux-perf-users, linux-kernel
Caught by leak sanitizer running "perf trace BTF general tests".
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-trace.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index bf9b5d0630d3..39b6a13ca862 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v1 4/4] libperf evsel: In exit add missed puts and assert close, etc. were called
2025-06-14 0:41 [PATCH v1 1/4] perf test: Directory file descriptor leak Ian Rogers
2025-06-14 0:41 ` [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs Ian Rogers
2025-06-14 0:41 ` [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread Ian Rogers
@ 2025-06-14 0:41 ` Ian Rogers
2 siblings, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2025-06-14 0:41 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, Michael Petlan, Andi Kleen,
Jiapeng Chong, Tiezhu Yang, linux-perf-users, linux-kernel
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 <irogers@google.com>
---
tools/lib/perf/evsel.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
index c475319e2e41..2a85e0bfee1e 100644
--- a/tools/lib/perf/evsel.c
+++ b/tools/lib/perf/evsel.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread
2025-06-14 0:41 ` [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread Ian Rogers
@ 2025-06-14 4:16 ` Howard Chu
2025-06-16 16:27 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 13+ messages in thread
From: Howard Chu @ 2025-06-14 4:16 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Adrian Hunter, Kan Liang, Michael Petlan, Andi Kleen,
Jiapeng Chong, Tiezhu Yang, linux-perf-users, linux-kernel
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.
On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
>
> Caught by leak sanitizer running "perf trace BTF general tests".
>
> Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Howard Chu <howardchu95@gmail.com>
Thanks,
Howard
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs
2025-06-14 0:41 ` [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs Ian Rogers
@ 2025-06-16 16:04 ` Ian Rogers
2025-06-16 16:24 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-06-16 16:04 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, Michael Petlan, Andi Kleen,
Jiapeng Chong, Tiezhu Yang, linux-perf-users, linux-kernel
On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
>
> 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.
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
> Fixes: b41f1cec91c3 ("perf list: Skip unsupported events")
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/util/print-events.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/util/print-events.c b/tools/perf/util/print-events.c
> index a786cbfb0ff5..83aaf7cda635 100644
> --- a/tools/perf/util/print-events.c
> +++ b/tools/perf/util/print-events.c
> @@ -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
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs
2025-06-16 16:04 ` Ian Rogers
@ 2025-06-16 16:24 ` Arnaldo Carvalho de Melo
2025-06-16 16:30 ` Ian Rogers
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-06-16 16:24 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Michael Petlan, Andi Kleen, Jiapeng Chong, Tiezhu Yang,
linux-perf-users, linux-kernel
On Mon, Jun 16, 2025 at 09:04:41AM -0700, Ian Rogers wrote:
> On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
> > 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.
> 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.
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread
2025-06-14 4:16 ` Howard Chu
@ 2025-06-16 16:27 ` Arnaldo Carvalho de Melo
2025-06-17 21:30 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-06-16 16:27 UTC (permalink / raw)
To: Howard Chu
Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, Michael Petlan, Andi Kleen, Jiapeng Chong, Tiezhu Yang,
linux-perf-users, linux-kernel
On Fri, Jun 13, 2025 at 09:16:26PM -0700, Howard Chu wrote:
> 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.
>
> On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
> >
> > Caught by leak sanitizer running "perf trace BTF general tests".
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Acked-by: Howard Chu <howardchu95@gmail.com>
Small enough, applied to perf-tools.
- Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs
2025-06-16 16:24 ` Arnaldo Carvalho de Melo
@ 2025-06-16 16:30 ` Ian Rogers
0 siblings, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2025-06-16 16:30 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Ingo Molnar, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
Michael Petlan, Andi Kleen, Jiapeng Chong, Tiezhu Yang,
linux-perf-users, linux-kernel
On Mon, Jun 16, 2025 at 9:24 AM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Mon, Jun 16, 2025 at 09:04:41AM -0700, Ian Rogers wrote:
> > On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
> > > 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.
>
> > 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.
>
> 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().
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread
2025-06-16 16:27 ` Arnaldo Carvalho de Melo
@ 2025-06-17 21:30 ` Arnaldo Carvalho de Melo
2025-06-17 21:32 ` Ian Rogers
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-06-17 21:30 UTC (permalink / raw)
To: Howard Chu
Cc: Ian Rogers, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, Michael Petlan, Andi Kleen, Jiapeng Chong, Tiezhu Yang,
linux-perf-users, linux-kernel
On Mon, Jun 16, 2025 at 01:27:04PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Jun 13, 2025 at 09:16:26PM -0700, Howard Chu wrote:
> > 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.
> >
> > On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
> > >
> > > Caught by leak sanitizer running "perf trace BTF general tests".
> > >
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> >
> > Acked-by: Howard Chu <howardchu95@gmail.com>
>
> Small enough, applied to perf-tools.
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 <irogers@google.com>
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".
Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Howard Chu <howardchu95@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Michael Petlan <mpetlan@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
Link: https://lore.kernel.org/r/20250614004108.1650988-3-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread
2025-06-17 21:30 ` Arnaldo Carvalho de Melo
@ 2025-06-17 21:32 ` Ian Rogers
2025-06-17 21:49 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 13+ messages in thread
From: Ian Rogers @ 2025-06-17 21:32 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Howard Chu, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, Michael Petlan, Andi Kleen, Jiapeng Chong, Tiezhu Yang,
linux-perf-users, linux-kernel
On Tue, Jun 17, 2025 at 2:30 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Mon, Jun 16, 2025 at 01:27:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Fri, Jun 13, 2025 at 09:16:26PM -0700, Howard Chu wrote:
> > > 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.
> > >
> > > On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
> > > >
> > > > Caught by leak sanitizer running "perf trace BTF general tests".
> > > >
> > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > >
> > > Acked-by: Howard Chu <howardchu95@gmail.com>
> >
> > Small enough, applied to perf-tools.
>
> 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:~#
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/20250611221521.722045-1-irogers@google.com/
Ian
> ⬢ [acme@toolbx perf-tools]$ git bisect good
> e340815ea559052d8d590a145bab7d7105608e7d is the first bad commit
> commit e340815ea559052d8d590a145bab7d7105608e7d
> Author: Ian Rogers <irogers@google.com>
> 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".
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> Acked-by: Howard Chu <howardchu95@gmail.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Michael Petlan <mpetlan@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Link: https://lore.kernel.org/r/20250614004108.1650988-3-irogers@google.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread
2025-06-17 21:32 ` Ian Rogers
@ 2025-06-17 21:49 ` Arnaldo Carvalho de Melo
2025-06-23 22:26 ` Ian Rogers
0 siblings, 1 reply; 13+ messages in thread
From: Arnaldo Carvalho de Melo @ 2025-06-17 21:49 UTC (permalink / raw)
To: Ian Rogers
Cc: Howard Chu, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, Michael Petlan, Andi Kleen, Jiapeng Chong, Tiezhu Yang,
linux-perf-users, linux-kernel
On Tue, Jun 17, 2025 at 02:32:37PM -0700, Ian Rogers wrote:
> On Tue, Jun 17, 2025 at 2:30 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > On Mon, Jun 16, 2025 at 01:27:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
> > > > > Caught by leak sanitizer running "perf trace BTF general tests".
> > > > > Signed-off-by: Ian Rogers <irogers@google.com>
> > > > Acked-by: Howard Chu <howardchu95@gmail.com>
> > > Small enough, applied to perf-tools.
> > 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:~#
> 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/20250611221521.722045-1-irogers@google.com/
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread
2025-06-17 21:49 ` Arnaldo Carvalho de Melo
@ 2025-06-23 22:26 ` Ian Rogers
0 siblings, 0 replies; 13+ messages in thread
From: Ian Rogers @ 2025-06-23 22:26 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Howard Chu, Peter Zijlstra, Ingo Molnar, Namhyung Kim,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, Michael Petlan, Andi Kleen, Jiapeng Chong, Tiezhu Yang,
linux-perf-users, linux-kernel
On Tue, Jun 17, 2025 at 2:49 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> On Tue, Jun 17, 2025 at 02:32:37PM -0700, Ian Rogers wrote:
> > On Tue, Jun 17, 2025 at 2:30 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > > On Mon, Jun 16, 2025 at 01:27:04PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > > On Fri, Jun 13, 2025 at 5:41 PM Ian Rogers <irogers@google.com> wrote:
> > > > > > Caught by leak sanitizer running "perf trace BTF general tests".
>
> > > > > > Signed-off-by: Ian Rogers <irogers@google.com>
>
> > > > > Acked-by: Howard Chu <howardchu95@gmail.com>
>
> > > > Small enough, applied to perf-tools.
>
> > > 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:~#
>
> > 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/20250611221521.722045-1-irogers@google.com/
>
> 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 :-\
I sent out a fix for this issue in v2:
https://lore.kernel.org/lkml/20250617223356.2752099-3-irogers@google.com/
Patches 1 and 2 I see are in the v6.16 fixes PR:
https://lore.kernel.org/lkml/20250620155415.88215-1-acme@kernel.org/
Thanks,
Ian
> - Arnaldo
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-06-23 22:26 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-14 0:41 [PATCH v1 1/4] perf test: Directory file descriptor leak Ian Rogers
2025-06-14 0:41 ` [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs Ian Rogers
2025-06-16 16:04 ` Ian Rogers
2025-06-16 16:24 ` Arnaldo Carvalho de Melo
2025-06-16 16:30 ` Ian Rogers
2025-06-14 0:41 ` [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread Ian Rogers
2025-06-14 4:16 ` Howard Chu
2025-06-16 16:27 ` Arnaldo Carvalho de Melo
2025-06-17 21:30 ` Arnaldo Carvalho de Melo
2025-06-17 21:32 ` Ian Rogers
2025-06-17 21:49 ` Arnaldo Carvalho de Melo
2025-06-23 22:26 ` Ian Rogers
2025-06-14 0:41 ` [PATCH v1 4/4] libperf evsel: In exit add missed puts and assert close, etc. were called Ian Rogers
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).