* [PATCH 0/4] perf timechart: Fix memory leaks
@ 2026-06-05 23:11 Namhyung Kim
2026-06-05 23:11 ` [PATCH 1/4] perf timechart: Release event samples at the end Namhyung Kim
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
Hello,
This patchset fixes memory leaks in the perf timechart code.
I've extended and verified the timechart test with ASAN.
Thanks,
Namhyung
Namhyung Kim (4):
perf timechart: Release event samples at the end
perf timechart: Fix memory leaks during record
perf timechart: Fix memory leaks in draw_wakeups()
perf test: Update perf timechart test
tools/perf/builtin-timechart.c | 93 ++++++++++++++++++++++++-----
tools/perf/tests/shell/timechart.sh | 24 +++++---
2 files changed, 94 insertions(+), 23 deletions(-)
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/4] perf timechart: Release event samples at the end
2026-06-05 23:11 [PATCH 0/4] perf timechart: Fix memory leaks Namhyung Kim
@ 2026-06-05 23:11 ` Namhyung Kim
2026-06-05 23:21 ` Arnaldo Carvalho de Melo
2026-06-05 23:11 ` [PATCH 2/4] perf timechart: Fix memory leaks during record Namhyung Kim
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
Add timechart__release() to free all data structures added during the
sample processing.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-timechart.c | 54 ++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 04dbb944a42720b4..7c4ace51302e260c 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1548,6 +1548,59 @@ static void write_svg_file(struct timechart *tchart, const char *filename)
svg_close();
}
+static void timechart__release(struct timechart *tchart)
+{
+ struct per_pid *p = tchart->all_data;
+ struct power_event *pwr = tchart->power_events;
+ struct wake_event *we = tchart->wake_events;
+
+ while (p) {
+ struct per_pid *next_pid = p->next;
+ struct per_pidcomm *c = p->all;
+
+ while (c) {
+ struct per_pidcomm *next_comm = c->next;
+ struct cpu_sample *cs = c->samples;
+ struct io_sample *ios = c->io_samples;
+
+ while (cs) {
+ struct cpu_sample *next = cs->next;
+
+ free((char *)cs->backtrace);
+ free(cs);
+ cs = next;
+ }
+
+ while (ios) {
+ struct io_sample *next = ios->next;
+
+ free(ios);
+ ios = next;
+ }
+
+ free(c->comm);
+ free(c);
+ c = next_comm;
+ }
+ p = next_pid;
+ }
+
+ while (pwr) {
+ struct power_event *next = pwr->next;
+
+ free(pwr);
+ pwr = next;
+ }
+
+ while (we) {
+ struct wake_event *next = we->next;
+
+ free((char *)we->backtrace);
+ free(we);
+ we = next;
+ }
+}
+
static int process_header(struct perf_file_section *section __maybe_unused,
struct perf_header *ph,
int feat,
@@ -2079,6 +2132,7 @@ int cmd_timechart(int argc, const char **argv)
ret = __cmd_timechart(&tchart, output_name);
out:
+ timechart__release(&tchart);
zfree(&cpus_cstate_start_times);
zfree(&cpus_cstate_state);
zfree(&cpus_pstate_start_times);
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/4] perf timechart: Fix memory leaks during record
2026-06-05 23:11 [PATCH 0/4] perf timechart: Fix memory leaks Namhyung Kim
2026-06-05 23:11 ` [PATCH 1/4] perf timechart: Release event samples at the end Namhyung Kim
@ 2026-06-05 23:11 ` Namhyung Kim
2026-06-05 23:23 ` Arnaldo Carvalho de Melo
2026-06-05 23:11 ` [PATCH 3/4] perf timechart: Fix memory leaks in draw_wakeups() Namhyung Kim
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
The command line options can be passed without copy as their lifetime is
limited within the same function.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-timechart.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 7c4ace51302e260c..d56adb41dcf9bb69 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1582,6 +1582,8 @@ static void timechart__release(struct timechart *tchart)
free(c);
c = next_comm;
}
+
+ free(p);
p = next_pid;
}
@@ -1820,6 +1822,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
"syscalls:sys_exit_select",
};
unsigned int poll_events_nr = ARRAY_SIZE(poll_events);
+ int ret;
rec_argc = common_args_nr +
disk_events_nr * 4 +
@@ -1838,7 +1841,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
p = rec_argv;
for (i = 0; i < common_args_nr; i++)
- *p++ = strdup(common_args[i]);
+ *p++ = common_args[i];
for (i = 0; i < disk_events_nr; i++) {
if (!is_valid_tracepoint(disk_events[i])) {
@@ -1847,7 +1850,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
}
*p++ = "-e";
- *p++ = strdup(disk_events[i]);
+ *p++ = disk_events[i];
*p++ = "--filter";
*p++ = filter;
}
@@ -1858,7 +1861,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
}
*p++ = "-e";
- *p++ = strdup(net_events[i]);
+ *p++ = net_events[i];
*p++ = "--filter";
*p++ = filter;
}
@@ -1869,7 +1872,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
}
*p++ = "-e";
- *p++ = strdup(poll_events[i]);
+ *p++ = poll_events[i];
*p++ = "--filter";
*p++ = filter;
}
@@ -1877,7 +1880,11 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
for (i = 0; i < (unsigned int)argc; i++)
*p++ = argv[i];
- return cmd_record(rec_argc, rec_argv);
+ ret = cmd_record(rec_argc, rec_argv);
+
+ free(rec_argv);
+ free(filter);
+ return ret;
}
@@ -1888,6 +1895,7 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
const char **rec_argv;
const char **p;
unsigned int record_elems;
+ int ret;
const char * const common_args[] = {
"record", "-a", "-R", "-c", "1", "-o", output_data,
@@ -1952,24 +1960,27 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
p = rec_argv;
for (i = 0; i < common_args_nr; i++)
- *p++ = strdup(common_args[i]);
+ *p++ = common_args[i];
for (i = 0; i < backtrace_args_no; i++)
- *p++ = strdup(backtrace_args[i]);
+ *p++ = backtrace_args[i];
for (i = 0; i < tasks_args_nr; i++)
- *p++ = strdup(tasks_args[i]);
+ *p++ = tasks_args[i];
for (i = 0; i < power_args_nr; i++)
- *p++ = strdup(power_args[i]);
+ *p++ = power_args[i];
for (i = 0; i < old_power_args_nr; i++)
- *p++ = strdup(old_power_args[i]);
+ *p++ = old_power_args[i];
for (j = 0; j < (unsigned int)argc; j++)
*p++ = argv[j];
- return cmd_record(rec_argc, rec_argv);
+ ret = cmd_record(rec_argc, rec_argv);
+
+ free(rec_argv);
+ return ret;
}
static int
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] perf timechart: Fix memory leaks in draw_wakeups()
2026-06-05 23:11 [PATCH 0/4] perf timechart: Fix memory leaks Namhyung Kim
2026-06-05 23:11 ` [PATCH 1/4] perf timechart: Release event samples at the end Namhyung Kim
2026-06-05 23:11 ` [PATCH 2/4] perf timechart: Fix memory leaks during record Namhyung Kim
@ 2026-06-05 23:11 ` Namhyung Kim
2026-06-05 23:11 ` [PATCH 4/4] perf test: Update perf timechart test Namhyung Kim
2026-06-05 23:19 ` [PATCH 0/4] perf timechart: Fix memory leaks Arnaldo Carvalho de Melo
4 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
The second loop for per_pidcommd is meaningful only when it doesn't have
from and to tasks. Also make sure c->Y is set before copying the comm
string otherwise it will be overwritten by next one.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-timechart.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index d56adb41dcf9bb69..e8d01c8723959e80 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -1110,12 +1110,12 @@ static void draw_wakeups(struct timechart *tchart)
c = c->next;
}
c = p->all;
- while (c) {
- if (p->pid == we->waker && !from) {
+ while (c && !from && !to) {
+ if (c->Y && p->pid == we->waker && !from) {
from = c->Y;
task_from = strdup(c->comm);
}
- if (p->pid == we->wakee && !to) {
+ if (c->Y && p->pid == we->wakee && !to) {
to = c->Y;
task_to = strdup(c->comm);
}
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] perf test: Update perf timechart test
2026-06-05 23:11 [PATCH 0/4] perf timechart: Fix memory leaks Namhyung Kim
` (2 preceding siblings ...)
2026-06-05 23:11 ` [PATCH 3/4] perf timechart: Fix memory leaks in draw_wakeups() Namhyung Kim
@ 2026-06-05 23:11 ` Namhyung Kim
2026-06-05 23:19 ` [PATCH 0/4] perf timechart: Fix memory leaks Arnaldo Carvalho de Melo
4 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
To include IO-only and backtrace modes to test different code paths.
$ sudo perf test -vv timechart
135: perf timechart tests : Running
135: perf timechart tests:
---- start ----
test child forked, pid 2413665
perf timechart Basic test
perf timechart Basic test [Success]
perf timechart IO-only test
perf timechart IO-only test [Success]
perf timechart Backtrace test
perf timechart Backtrace test [Success]
---- end(0) ----
135: perf timechart tests : Ok
=== Test Summary ===
Passed main tests : 1
Passed subtests : 0
Skipped tests : 0
Failed tests : 0
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/tests/shell/timechart.sh | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/tools/perf/tests/shell/timechart.sh b/tools/perf/tests/shell/timechart.sh
index b14b3472c2846b32..40072b32171a9a1c 100755
--- a/tools/perf/tests/shell/timechart.sh
+++ b/tools/perf/tests/shell/timechart.sh
@@ -9,7 +9,7 @@ perfdata=$(mktemp /tmp/__perf_timechart_test.perf.data.XXXXX)
output=$(mktemp /tmp/__perf_timechart_test.output.XXXXX.svg)
cleanup() {
- rm -f "${perfdata}"
+ rm -f "${perfdata}"*
rm -f "${output}"
trap - EXIT TERM INT
}
@@ -22,38 +22,41 @@ trap_cleanup() {
trap trap_cleanup EXIT TERM INT
test_timechart() {
- echo "Basic perf timechart test"
+ NAME=$1
+ OPTION=$2
+
+ echo "perf timechart ${NAME} test"
# Try to record timechart data.
# perf timechart record uses system-wide recording and specific tracepoints.
# If it fails (e.g. permissions, missing tracepoints), skip the test.
- if ! perf timechart record -o "${perfdata}" true > /dev/null 2>&1; then
- echo "Basic perf timechart test [Skipped: perf timechart record failed (permissions/events?)]"
+ if ! perf timechart record -o "${perfdata}" ${OPTION} true > /dev/null 2>&1; then
+ echo "perf timechart ${NAME} test [Skipped: perf timechart record failed (permissions/events?)]"
return
fi
# Generate the timechart
if ! perf timechart -i "${perfdata}" -o "${output}" > /dev/null 2>&1; then
- echo "Basic perf timechart test [Failed: perf timechart command failed]"
+ echo "perf timechart ${NAME} test [Failed: perf timechart command failed]"
err=1
return
fi
# Check if output file exists and is not empty
if [ ! -s "${output}" ]; then
- echo "Basic perf timechart test [Failed: output file is empty or missing]"
+ echo "perf timechart ${NAME} test [Failed: output file is empty or missing]"
err=1
return
fi
# Check if it looks like an SVG
if ! grep -q "svg" "${output}"; then
- echo "Basic perf timechart test [Failed: output doesn't look like SVG]"
+ echo "perf timechart ${NAME} test [Failed: output doesn't look like SVG]"
err=1
return
fi
- echo "Basic perf timechart test [Success]"
+ echo "perf timechart ${NAME} test [Success]"
}
if ! perf check feature -q libtraceevent ; then
@@ -62,6 +65,9 @@ if ! perf check feature -q libtraceevent ; then
exit 2
fi
-test_timechart
+test_timechart "Basic" ""
+test_timechart "IO-only" "-I"
+test_timechart "Backtrace" "-g"
+
cleanup
exit $err
--
2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] perf timechart: Fix memory leaks
2026-06-05 23:11 [PATCH 0/4] perf timechart: Fix memory leaks Namhyung Kim
` (3 preceding siblings ...)
2026-06-05 23:11 ` [PATCH 4/4] perf test: Update perf timechart test Namhyung Kim
@ 2026-06-05 23:19 ` Arnaldo Carvalho de Melo
2026-06-05 23:28 ` Namhyung Kim
4 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-05 23:19 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Jun 05, 2026 at 04:11:51PM -0700, Namhyung Kim wrote:
> Hello,
>
> This patchset fixes memory leaks in the perf timechart code.
> I've extended and verified the timechart test with ASAN.
Hi Namhyung,
You submitted a series for timechart before, so I had to go on
looking if this was a second version or a different series.
I suggest you always have a version in the cover letter, i.e.:
[PATCH v1 0/4] perf timechart: Fix memory leaks
To avoid this brief distraction :-)
Regards,
- Arnaldo
> Thanks,
> Namhyung
>
>
> Namhyung Kim (4):
> perf timechart: Release event samples at the end
> perf timechart: Fix memory leaks during record
> perf timechart: Fix memory leaks in draw_wakeups()
> perf test: Update perf timechart test
>
> tools/perf/builtin-timechart.c | 93 ++++++++++++++++++++++++-----
> tools/perf/tests/shell/timechart.sh | 24 +++++---
> 2 files changed, 94 insertions(+), 23 deletions(-)
>
> --
> 2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] perf timechart: Release event samples at the end
2026-06-05 23:11 ` [PATCH 1/4] perf timechart: Release event samples at the end Namhyung Kim
@ 2026-06-05 23:21 ` Arnaldo Carvalho de Melo
2026-06-05 23:31 ` Namhyung Kim
0 siblings, 1 reply; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-05 23:21 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Jun 05, 2026 at 04:11:52PM -0700, Namhyung Kim wrote:
> Add timechart__release() to free all data structures added during the
> sample processing.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-timechart.c | 54 ++++++++++++++++++++++++++++++++++
> 1 file changed, 54 insertions(+)
>
> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 04dbb944a42720b4..7c4ace51302e260c 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
> @@ -1548,6 +1548,59 @@ static void write_svg_file(struct timechart *tchart, const char *filename)
> svg_close();
> }
>
> +static void timechart__release(struct timechart *tchart)
> +{
> + struct per_pid *p = tchart->all_data;
> + struct power_event *pwr = tchart->power_events;
> + struct wake_event *we = tchart->wake_events;
> +
> + while (p) {
> + struct per_pid *next_pid = p->next;
> + struct per_pidcomm *c = p->all;
> +
> + while (c) {
> + struct per_pidcomm *next_comm = c->next;
> + struct cpu_sample *cs = c->samples;
> + struct io_sample *ios = c->io_samples;
> +
> + while (cs) {
> + struct cpu_sample *next = cs->next;
> +
> + free((char *)cs->backtrace);
zfree()? This way if somebody has a
pointer to cs and tries to use
cs->backtrace after this free, it will
segfault instead of using garbage or
worse, setting some other unrelated
struct to some cpu_sample field supposed
value.
> + free(cs);
> + cs = next;
> + }
> +
> + while (ios) {
> + struct io_sample *next = ios->next;
> +
> + free(ios);
io->next = NULL;
> + ios = next;
> + }
> +
> + free(c->comm);
zfree
> + free(c);
> + c = next_comm;
> + }
> + p = next_pid;
> + }
> +
> + while (pwr) {
> + struct power_event *next = pwr->next;
> +
> + free(pwr);
pwr->next = NULL;
> + pwr = next;
> + }
> +
> + while (we) {
> + struct wake_event *next = we->next;
> +
> + free((char *)we->backtrace);
zfree
> + free(we);
> + we = next;
> + }
> +}
> +
> static int process_header(struct perf_file_section *section __maybe_unused,
> struct perf_header *ph,
> int feat,
> @@ -2079,6 +2132,7 @@ int cmd_timechart(int argc, const char **argv)
>
> ret = __cmd_timechart(&tchart, output_name);
> out:
> + timechart__release(&tchart);
> zfree(&cpus_cstate_start_times);
> zfree(&cpus_cstate_state);
> zfree(&cpus_pstate_start_times);
> --
> 2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/4] perf timechart: Fix memory leaks during record
2026-06-05 23:11 ` [PATCH 2/4] perf timechart: Fix memory leaks during record Namhyung Kim
@ 2026-06-05 23:23 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-05 23:23 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Jun 05, 2026 at 04:11:53PM -0700, Namhyung Kim wrote:
> The command line options can be passed without copy as their lifetime is
> limited within the same function.
From a quick look:
Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
- Arnaldo
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-timechart.c | 33 ++++++++++++++++++++++-----------
> 1 file changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index 7c4ace51302e260c..d56adb41dcf9bb69 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
> @@ -1582,6 +1582,8 @@ static void timechart__release(struct timechart *tchart)
> free(c);
> c = next_comm;
> }
> +
> + free(p);
> p = next_pid;
> }
>
> @@ -1820,6 +1822,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
> "syscalls:sys_exit_select",
> };
> unsigned int poll_events_nr = ARRAY_SIZE(poll_events);
> + int ret;
>
> rec_argc = common_args_nr +
> disk_events_nr * 4 +
> @@ -1838,7 +1841,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
>
> p = rec_argv;
> for (i = 0; i < common_args_nr; i++)
> - *p++ = strdup(common_args[i]);
> + *p++ = common_args[i];
>
> for (i = 0; i < disk_events_nr; i++) {
> if (!is_valid_tracepoint(disk_events[i])) {
> @@ -1847,7 +1850,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
> }
>
> *p++ = "-e";
> - *p++ = strdup(disk_events[i]);
> + *p++ = disk_events[i];
> *p++ = "--filter";
> *p++ = filter;
> }
> @@ -1858,7 +1861,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
> }
>
> *p++ = "-e";
> - *p++ = strdup(net_events[i]);
> + *p++ = net_events[i];
> *p++ = "--filter";
> *p++ = filter;
> }
> @@ -1869,7 +1872,7 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
> }
>
> *p++ = "-e";
> - *p++ = strdup(poll_events[i]);
> + *p++ = poll_events[i];
> *p++ = "--filter";
> *p++ = filter;
> }
> @@ -1877,7 +1880,11 @@ static int timechart__io_record(int argc, const char **argv, const char *output_
> for (i = 0; i < (unsigned int)argc; i++)
> *p++ = argv[i];
>
> - return cmd_record(rec_argc, rec_argv);
> + ret = cmd_record(rec_argc, rec_argv);
> +
> + free(rec_argv);
> + free(filter);
> + return ret;
> }
>
>
> @@ -1888,6 +1895,7 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
> const char **rec_argv;
> const char **p;
> unsigned int record_elems;
> + int ret;
>
> const char * const common_args[] = {
> "record", "-a", "-R", "-c", "1", "-o", output_data,
> @@ -1952,24 +1960,27 @@ static int timechart__record(struct timechart *tchart, int argc, const char **ar
>
> p = rec_argv;
> for (i = 0; i < common_args_nr; i++)
> - *p++ = strdup(common_args[i]);
> + *p++ = common_args[i];
>
> for (i = 0; i < backtrace_args_no; i++)
> - *p++ = strdup(backtrace_args[i]);
> + *p++ = backtrace_args[i];
>
> for (i = 0; i < tasks_args_nr; i++)
> - *p++ = strdup(tasks_args[i]);
> + *p++ = tasks_args[i];
>
> for (i = 0; i < power_args_nr; i++)
> - *p++ = strdup(power_args[i]);
> + *p++ = power_args[i];
>
> for (i = 0; i < old_power_args_nr; i++)
> - *p++ = strdup(old_power_args[i]);
> + *p++ = old_power_args[i];
>
> for (j = 0; j < (unsigned int)argc; j++)
> *p++ = argv[j];
>
> - return cmd_record(rec_argc, rec_argv);
> + ret = cmd_record(rec_argc, rec_argv);
> +
> + free(rec_argv);
> + return ret;
> }
>
> static int
> --
> 2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] perf timechart: Fix memory leaks
2026-06-05 23:19 ` [PATCH 0/4] perf timechart: Fix memory leaks Arnaldo Carvalho de Melo
@ 2026-06-05 23:28 ` Namhyung Kim
2026-06-05 23:41 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 11+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:28 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Jun 05, 2026 at 08:19:13PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Jun 05, 2026 at 04:11:51PM -0700, Namhyung Kim wrote:
> > Hello,
> >
> > This patchset fixes memory leaks in the perf timechart code.
> > I've extended and verified the timechart test with ASAN.
>
> Hi Namhyung,
>
> You submitted a series for timechart before, so I had to go on
> looking if this was a second version or a different series.
Right, this is a different series. I believe they are independent.
>
> I suggest you always have a version in the cover letter, i.e.:
>
> [PATCH v1 0/4] perf timechart: Fix memory leaks
>
> To avoid this brief distraction :-)
Ok, I'll do that if that helps. :)
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] perf timechart: Release event samples at the end
2026-06-05 23:21 ` Arnaldo Carvalho de Melo
@ 2026-06-05 23:31 ` Namhyung Kim
0 siblings, 0 replies; 11+ messages in thread
From: Namhyung Kim @ 2026-06-05 23:31 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Jun 05, 2026 at 08:21:49PM -0300, Arnaldo Carvalho de Melo wrote:
> On Fri, Jun 05, 2026 at 04:11:52PM -0700, Namhyung Kim wrote:
> > Add timechart__release() to free all data structures added during the
> > sample processing.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/builtin-timechart.c | 54 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 54 insertions(+)
> >
> > diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> > index 04dbb944a42720b4..7c4ace51302e260c 100644
> > --- a/tools/perf/builtin-timechart.c
> > +++ b/tools/perf/builtin-timechart.c
> > @@ -1548,6 +1548,59 @@ static void write_svg_file(struct timechart *tchart, const char *filename)
> > svg_close();
> > }
> >
> > +static void timechart__release(struct timechart *tchart)
> > +{
> > + struct per_pid *p = tchart->all_data;
> > + struct power_event *pwr = tchart->power_events;
> > + struct wake_event *we = tchart->wake_events;
> > +
> > + while (p) {
> > + struct per_pid *next_pid = p->next;
> > + struct per_pidcomm *c = p->all;
> > +
> > + while (c) {
> > + struct per_pidcomm *next_comm = c->next;
> > + struct cpu_sample *cs = c->samples;
> > + struct io_sample *ios = c->io_samples;
> > +
> > + while (cs) {
> > + struct cpu_sample *next = cs->next;
> > +
> > + free((char *)cs->backtrace);
>
> zfree()? This way if somebody has a
> pointer to cs and tries to use
> cs->backtrace after this free, it will
> segfault instead of using garbage or
> worse, setting some other unrelated
> struct to some cpu_sample field supposed
> value.
Ok, will do here and others below.
Thanks,
Namhyung
>
> > + free(cs);
> > + cs = next;
> > + }
> > +
> > + while (ios) {
> > + struct io_sample *next = ios->next;
> > +
> > + free(ios);
>
> io->next = NULL;
>
> > + ios = next;
> > + }
> > +
> > + free(c->comm);
>
> zfree
>
> > + free(c);
> > + c = next_comm;
> > + }
> > + p = next_pid;
> > + }
> > +
> > + while (pwr) {
> > + struct power_event *next = pwr->next;
> > +
> > + free(pwr);
> pwr->next = NULL;
> > + pwr = next;
> > + }
> > +
> > + while (we) {
> > + struct wake_event *next = we->next;
> > +
> > + free((char *)we->backtrace);
> zfree
> > + free(we);
> > + we = next;
> > + }
> > +}
> > +
> > static int process_header(struct perf_file_section *section __maybe_unused,
> > struct perf_header *ph,
> > int feat,
> > @@ -2079,6 +2132,7 @@ int cmd_timechart(int argc, const char **argv)
> >
> > ret = __cmd_timechart(&tchart, output_name);
> > out:
> > + timechart__release(&tchart);
> > zfree(&cpus_cstate_start_times);
> > zfree(&cpus_cstate_state);
> > zfree(&cpus_pstate_start_times);
> > --
> > 2.54.0.1032.g2f8565e1d1-goog
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/4] perf timechart: Fix memory leaks
2026-06-05 23:28 ` Namhyung Kim
@ 2026-06-05 23:41 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-06-05 23:41 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
Ingo Molnar, LKML, linux-perf-users
On Fri, Jun 05, 2026 at 04:28:13PM -0700, Namhyung Kim wrote:
> On Fri, Jun 05, 2026 at 08:19:13PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Fri, Jun 05, 2026 at 04:11:51PM -0700, Namhyung Kim wrote:
> > > This patchset fixes memory leaks in the perf timechart code.
> > > I've extended and verified the timechart test with ASAN.
> > You submitted a series for timechart before, so I had to go on
> > looking if this was a second version or a different series.
> Right, this is a different series. I believe they are independent.
Thanks for clarifying.
> > I suggest you always have a version in the cover letter, i.e.:
> > [PATCH v1 0/4] perf timechart: Fix memory leaks
> > To avoid this brief distraction :-)
> Ok, I'll do that if that helps. :)
It is a small detail, not having anything it may have been that the
submitter forgot to add something or that it is the first entry, having
v1 removes this ambiguity.
Thanks,
- Arnaldo
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-05 23:41 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-05 23:11 [PATCH 0/4] perf timechart: Fix memory leaks Namhyung Kim
2026-06-05 23:11 ` [PATCH 1/4] perf timechart: Release event samples at the end Namhyung Kim
2026-06-05 23:21 ` Arnaldo Carvalho de Melo
2026-06-05 23:31 ` Namhyung Kim
2026-06-05 23:11 ` [PATCH 2/4] perf timechart: Fix memory leaks during record Namhyung Kim
2026-06-05 23:23 ` Arnaldo Carvalho de Melo
2026-06-05 23:11 ` [PATCH 3/4] perf timechart: Fix memory leaks in draw_wakeups() Namhyung Kim
2026-06-05 23:11 ` [PATCH 4/4] perf test: Update perf timechart test Namhyung Kim
2026-06-05 23:19 ` [PATCH 0/4] perf timechart: Fix memory leaks Arnaldo Carvalho de Melo
2026-06-05 23:28 ` Namhyung Kim
2026-06-05 23:41 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox