[PATCH v1 1/4] perf test: Directory file descriptor leak

[PATCH v1 1/4] perf test: Directory file descriptor leak

[Ian Rogers]

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

[PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs

[Ian Rogers]

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

Re: [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs

[Ian Rogers]

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
>

Re: [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v1 2/4] perf evsel: Missed close when probing hybrid core PMUs

[Ian Rogers]

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

[PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread

[Ian Rogers]

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

Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread

[Howard Chu]

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

Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread

[Ian Rogers]

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

Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v1 3/4] perf trace: Add missed freeing of ordered events and thread

[Ian Rogers]

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

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

[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 <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