The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [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