All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] perf timechart: Small optimization for backtrace
@ 2026-06-04 21:49 Namhyung Kim
  2026-06-04 21:49 ` [PATCH v2 1/4] perf timechart: Don't pass @event to cat_backtrace() Namhyung Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Namhyung Kim @ 2026-06-04 21:49 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,

I found an issue int timechart backtrace handling during the last review.
The goal is to reduce unnecessary work in generating backtrace string.

v2 changes)
 * fix memory leaks when backtrace is not used
 * copy backtrace when it's used twice

Thanks,
Namhyung


Namhyung Kim (4):
  perf timechart: Don't pass @event to cat_backtrace()
  perf timechart: Generate backtrace only if needed
  perf timechart: Remove unused backtrace in trace_handler
  perf timechart: Remove unnecessary copy of backtrace

 tools/perf/builtin-timechart.c | 107 ++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 55 deletions(-)

-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/4] perf timechart: Don't pass @event to cat_backtrace()
  2026-06-04 21:49 [PATCH v2 0/4] perf timechart: Small optimization for backtrace Namhyung Kim
@ 2026-06-04 21:49 ` Namhyung Kim
  2026-06-04 21:49 ` [PATCH v2 2/4] perf timechart: Generate backtrace only if needed Namhyung Kim
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2026-06-04 21:49 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 cat_backtrace() is only called from process_sample_event() which
means the event type is always PERF_RECORD_SAMPLE.  We don't need to
pass the event just because to print already known info.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-timechart.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 85a9ad0455aecccd..dafd361ecf9d8cd7 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -493,8 +493,7 @@ static void sched_switch(struct timechart *tchart, int cpu, u64 timestamp,
  * Returns a malloc'd backtrace string built via open_memstream, or NULL
  * on error.  Caller must free() the returned pointer.
  */
-static char *cat_backtrace(union perf_event *event,
-			   struct perf_sample *sample,
+static char *cat_backtrace(struct perf_sample *sample,
 			   struct machine *machine)
 {
 	struct addr_location al;
@@ -516,9 +515,8 @@ static char *cat_backtrace(union perf_event *event,
 		goto exit;
 
 	if (machine__resolve(machine, &al, sample) < 0) {
-		pr_err("problem processing %s (%u) event at offset %#" PRIx64 ", skipping it.\n",
-		       perf_event__name(event->header.type), event->header.type,
-		       sample->file_offset);
+		pr_err("problem processing SAMPLE (%u) event at offset %#" PRIx64 ", skipping it.\n",
+		       PERF_RECORD_SAMPLE, sample->file_offset);
 		goto exit;
 	}
 
@@ -578,7 +576,7 @@ typedef int (*tracepoint_handler)(struct timechart *tchart,
 				  const char *backtrace);
 
 static int process_sample_event(const struct perf_tool *tool,
-				union perf_event *event,
+				union perf_event *event __maybe_unused,
 				struct perf_sample *sample,
 				struct machine *machine)
 {
@@ -595,7 +593,7 @@ static int process_sample_event(const struct perf_tool *tool,
 
 	if (evsel->handler != NULL) {
 		tracepoint_handler f = evsel->handler;
-		char *backtrace = cat_backtrace(event, sample, machine);
+		char *backtrace = cat_backtrace(sample, machine);
 
 		ret = f(tchart, sample, backtrace);
 		free(backtrace);
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/4] perf timechart: Generate backtrace only if needed
  2026-06-04 21:49 [PATCH v2 0/4] perf timechart: Small optimization for backtrace Namhyung Kim
  2026-06-04 21:49 ` [PATCH v2 1/4] perf timechart: Don't pass @event to cat_backtrace() Namhyung Kim
@ 2026-06-04 21:49 ` Namhyung Kim
  2026-06-04 21:49 ` [PATCH v2 3/4] perf timechart: Remove unused backtrace in trace_handler Namhyung Kim
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2026-06-04 21:49 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 backtrace was used by sched-switch and sched-wakeup only.  No need
to call cat_backtrace() in the process_sample_event().  Let's pass NULL
backtrace and generate it from the sched events.

As it needs a pointer to the 'machine', let's save the session in the
timechart struct and use the host machine of the session instead of
passing the pointer to all handlers.  It should be fine to assume the
host machine as timechart command doesn't deal with guest machines and
there's no way to get tracepoints from the guest events.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-timechart.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index dafd361ecf9d8cd7..337ae547d008ea0b 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -56,6 +56,7 @@ struct timechart {
 	struct per_pid		*all_data;
 	struct power_event	*power_events;
 	struct wake_event	*wake_events;
+	struct perf_session	*session;
 	int			proc_num;
 	unsigned int		numcpus;
 	u64			min_freq,	/* Lowest CPU frequency seen */
@@ -578,7 +579,7 @@ typedef int (*tracepoint_handler)(struct timechart *tchart,
 static int process_sample_event(const struct perf_tool *tool,
 				union perf_event *event __maybe_unused,
 				struct perf_sample *sample,
-				struct machine *machine)
+				struct machine *machine __maybe_unused)
 {
 	struct timechart *tchart = container_of(tool, struct timechart, tool);
 	struct evsel *evsel = sample->evsel;
@@ -593,10 +594,8 @@ static int process_sample_event(const struct perf_tool *tool,
 
 	if (evsel->handler != NULL) {
 		tracepoint_handler f = evsel->handler;
-		char *backtrace = cat_backtrace(sample, machine);
 
-		ret = f(tchart, sample, backtrace);
-		free(backtrace);
+		ret = f(tchart, sample, NULL);
 	}
 
 	return ret;
@@ -656,7 +655,10 @@ process_sample_sched_wakeup(struct timechart *tchart,
 			 sample->file_offset, sample->cpu);
 		return -1;
 	}
+
+	backtrace = cat_backtrace(sample, &tchart->session->machines.host);
 	sched_wakeup(tchart, sample->cpu, sample->time, waker, wakee, flags, backtrace);
+	free((char *)backtrace);
 	return 0;
 }
 
@@ -675,8 +677,11 @@ process_sample_sched_switch(struct timechart *tchart,
 			 sample->file_offset, sample->cpu);
 		return -1;
 	}
+
+	backtrace = cat_backtrace(sample, &tchart->session->machines.host);
 	sched_switch(tchart, sample->cpu, sample->time, prev_pid, next_pid,
 		     prev_state, backtrace);
+	free((char *)backtrace);
 	return 0;
 }
 
@@ -1661,6 +1666,7 @@ static int __cmd_timechart(struct timechart *tchart, const char *output_name)
 	if (IS_ERR(session))
 		return PTR_ERR(session);
 
+	tchart->session = session;
 	symbol__init(perf_session__env(session));
 
 	(void)perf_header__process_sections(&session->header,
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/4] perf timechart: Remove unused backtrace in trace_handler
  2026-06-04 21:49 [PATCH v2 0/4] perf timechart: Small optimization for backtrace Namhyung Kim
  2026-06-04 21:49 ` [PATCH v2 1/4] perf timechart: Don't pass @event to cat_backtrace() Namhyung Kim
  2026-06-04 21:49 ` [PATCH v2 2/4] perf timechart: Generate backtrace only if needed Namhyung Kim
@ 2026-06-04 21:49 ` Namhyung Kim
  2026-06-04 21:49 ` [PATCH v2 4/4] perf timechart: Remove unnecessary copy of backtrace Namhyung Kim
  2026-06-04 22:38 ` [PATCH v2 0/4] perf timechart: Small optimization for backtrace Ian Rogers
  4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2026-06-04 21:49 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

Now the backtrace argument is not used in any handler.  Let's get rid of
it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-timechart.c | 64 ++++++++++++----------------------
 1 file changed, 23 insertions(+), 41 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 337ae547d008ea0b..4efac73a714c5e5f 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -573,8 +573,7 @@ static char *cat_backtrace(struct perf_sample *sample,
 }
 
 typedef int (*tracepoint_handler)(struct timechart *tchart,
-				  struct perf_sample *sample,
-				  const char *backtrace);
+				  struct perf_sample *sample);
 
 static int process_sample_event(const struct perf_tool *tool,
 				union perf_event *event __maybe_unused,
@@ -595,7 +594,7 @@ static int process_sample_event(const struct perf_tool *tool,
 	if (evsel->handler != NULL) {
 		tracepoint_handler f = evsel->handler;
 
-		ret = f(tchart, sample, NULL);
+		ret = f(tchart, sample);
 	}
 
 	return ret;
@@ -603,8 +602,7 @@ static int process_sample_event(const struct perf_tool *tool,
 
 static int
 process_sample_cpu_idle(struct timechart *tchart __maybe_unused,
-			struct perf_sample *sample,
-			const char *backtrace __maybe_unused)
+			struct perf_sample *sample)
 {
 	u32 state  = perf_sample__intval(sample, "state");
 	u32 cpu_id = perf_sample__intval(sample, "cpu_id");
@@ -624,8 +622,7 @@ process_sample_cpu_idle(struct timechart *tchart __maybe_unused,
 
 static int
 process_sample_cpu_frequency(struct timechart *tchart,
-			     struct perf_sample *sample,
-			     const char *backtrace __maybe_unused)
+			     struct perf_sample *sample)
 {
 	u32 state  = perf_sample__intval(sample, "state");
 	u32 cpu_id = perf_sample__intval(sample, "cpu_id");
@@ -642,12 +639,12 @@ process_sample_cpu_frequency(struct timechart *tchart,
 
 static int
 process_sample_sched_wakeup(struct timechart *tchart,
-			    struct perf_sample *sample,
-			    const char *backtrace)
+			    struct perf_sample *sample)
 {
 	u8 flags  = perf_sample__intval(sample, "common_flags");
 	int waker = perf_sample__intval(sample, "common_pid");
 	int wakee = perf_sample__intval(sample, "pid");
+	char *backtrace;
 
 	/* perf.data is untrusted input — CPU may be absent or corrupted */
 	if (sample->cpu >= MAX_CPUS) {
@@ -664,12 +661,12 @@ process_sample_sched_wakeup(struct timechart *tchart,
 
 static int
 process_sample_sched_switch(struct timechart *tchart,
-			    struct perf_sample *sample,
-			    const char *backtrace)
+			    struct perf_sample *sample)
 {
 	int prev_pid   = perf_sample__intval(sample, "prev_pid");
 	int next_pid   = perf_sample__intval(sample, "next_pid");
 	u64 prev_state = perf_sample__intval(sample, "prev_state");
+	char *backtrace;
 
 	/* perf.data is untrusted input — CPU may be absent or corrupted */
 	if (sample->cpu >= MAX_CPUS) {
@@ -688,8 +685,7 @@ process_sample_sched_switch(struct timechart *tchart,
 #ifdef SUPPORT_OLD_POWER_EVENTS
 static int
 process_sample_power_start(struct timechart *tchart __maybe_unused,
-			   struct perf_sample *sample,
-			   const char *backtrace __maybe_unused)
+			   struct perf_sample *sample)
 {
 	u64 cpu_id = perf_sample__intval(sample, "cpu_id");
 	u64 value  = perf_sample__intval(sample, "value");
@@ -706,8 +702,7 @@ process_sample_power_start(struct timechart *tchart __maybe_unused,
 
 static int
 process_sample_power_end(struct timechart *tchart,
-			 struct perf_sample *sample,
-			 const char *backtrace __maybe_unused)
+			 struct perf_sample *sample)
 {
 	/* perf.data is untrusted input — CPU may be absent or corrupted */
 	if (sample->cpu >= MAX_CPUS) {
@@ -721,8 +716,7 @@ process_sample_power_end(struct timechart *tchart,
 
 static int
 process_sample_power_frequency(struct timechart *tchart,
-			       struct perf_sample *sample,
-			       const char *backtrace __maybe_unused)
+			       struct perf_sample *sample)
 {
 	u64 cpu_id = perf_sample__intval(sample, "cpu_id");
 	u64 value  = perf_sample__intval(sample, "value");
@@ -895,8 +889,7 @@ static int pid_end_io_sample(struct timechart *tchart, int pid, int type,
 
 static int
 process_enter_read(struct timechart *tchart,
-		   struct perf_sample *sample,
-		   const char *backtrace __maybe_unused)
+		   struct perf_sample *sample)
 {
 	long fd = perf_sample__intval(sample, "fd");
 	return pid_begin_io_sample(tchart, sample->tid, IOTYPE_READ,
@@ -905,8 +898,7 @@ process_enter_read(struct timechart *tchart,
 
 static int
 process_exit_read(struct timechart *tchart,
-		  struct perf_sample *sample,
-		  const char *backtrace __maybe_unused)
+		  struct perf_sample *sample)
 {
 	long ret = perf_sample__intval(sample, "ret");
 	return pid_end_io_sample(tchart, sample->tid, IOTYPE_READ,
@@ -915,8 +907,7 @@ process_exit_read(struct timechart *tchart,
 
 static int
 process_enter_write(struct timechart *tchart,
-		    struct perf_sample *sample,
-		    const char *backtrace __maybe_unused)
+		    struct perf_sample *sample)
 {
 	long fd = perf_sample__intval(sample, "fd");
 	return pid_begin_io_sample(tchart, sample->tid, IOTYPE_WRITE,
@@ -925,8 +916,7 @@ process_enter_write(struct timechart *tchart,
 
 static int
 process_exit_write(struct timechart *tchart,
-		   struct perf_sample *sample,
-		   const char *backtrace __maybe_unused)
+		   struct perf_sample *sample)
 {
 	long ret = perf_sample__intval(sample, "ret");
 	return pid_end_io_sample(tchart, sample->tid, IOTYPE_WRITE,
@@ -935,8 +925,7 @@ process_exit_write(struct timechart *tchart,
 
 static int
 process_enter_sync(struct timechart *tchart,
-		   struct perf_sample *sample,
-		   const char *backtrace __maybe_unused)
+		   struct perf_sample *sample)
 {
 	long fd = perf_sample__intval(sample, "fd");
 	return pid_begin_io_sample(tchart, sample->tid, IOTYPE_SYNC,
@@ -945,8 +934,7 @@ process_enter_sync(struct timechart *tchart,
 
 static int
 process_exit_sync(struct timechart *tchart,
-		  struct perf_sample *sample,
-		  const char *backtrace __maybe_unused)
+		  struct perf_sample *sample)
 {
 	long ret = perf_sample__intval(sample, "ret");
 	return pid_end_io_sample(tchart, sample->tid, IOTYPE_SYNC,
@@ -955,8 +943,7 @@ process_exit_sync(struct timechart *tchart,
 
 static int
 process_enter_tx(struct timechart *tchart,
-		 struct perf_sample *sample,
-		 const char *backtrace __maybe_unused)
+		 struct perf_sample *sample)
 {
 	long fd = perf_sample__intval(sample, "fd");
 	return pid_begin_io_sample(tchart, sample->tid, IOTYPE_TX,
@@ -965,8 +952,7 @@ process_enter_tx(struct timechart *tchart,
 
 static int
 process_exit_tx(struct timechart *tchart,
-		struct perf_sample *sample,
-		const char *backtrace __maybe_unused)
+		struct perf_sample *sample)
 {
 	long ret = perf_sample__intval(sample, "ret");
 	return pid_end_io_sample(tchart, sample->tid, IOTYPE_TX,
@@ -975,8 +961,7 @@ process_exit_tx(struct timechart *tchart,
 
 static int
 process_enter_rx(struct timechart *tchart,
-		 struct perf_sample *sample,
-		 const char *backtrace __maybe_unused)
+		 struct perf_sample *sample)
 {
 	long fd = perf_sample__intval(sample, "fd");
 	return pid_begin_io_sample(tchart, sample->tid, IOTYPE_RX,
@@ -985,8 +970,7 @@ process_enter_rx(struct timechart *tchart,
 
 static int
 process_exit_rx(struct timechart *tchart,
-		struct perf_sample *sample,
-		const char *backtrace __maybe_unused)
+		struct perf_sample *sample)
 {
 	long ret = perf_sample__intval(sample, "ret");
 	return pid_end_io_sample(tchart, sample->tid, IOTYPE_RX,
@@ -995,8 +979,7 @@ process_exit_rx(struct timechart *tchart,
 
 static int
 process_enter_poll(struct timechart *tchart,
-		   struct perf_sample *sample,
-		   const char *backtrace __maybe_unused)
+		   struct perf_sample *sample)
 {
 	long fd = perf_sample__intval(sample, "fd");
 	return pid_begin_io_sample(tchart, sample->tid, IOTYPE_POLL,
@@ -1005,8 +988,7 @@ process_enter_poll(struct timechart *tchart,
 
 static int
 process_exit_poll(struct timechart *tchart,
-		  struct perf_sample *sample,
-		  const char *backtrace __maybe_unused)
+		  struct perf_sample *sample)
 {
 	long ret = perf_sample__intval(sample, "ret");
 	return pid_end_io_sample(tchart, sample->tid, IOTYPE_POLL,
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/4] perf timechart: Remove unnecessary copy of backtrace
  2026-06-04 21:49 [PATCH v2 0/4] perf timechart: Small optimization for backtrace Namhyung Kim
                   ` (2 preceding siblings ...)
  2026-06-04 21:49 ` [PATCH v2 3/4] perf timechart: Remove unused backtrace in trace_handler Namhyung Kim
@ 2026-06-04 21:49 ` Namhyung Kim
  2026-06-04 22:38 ` [PATCH v2 0/4] perf timechart: Small optimization for backtrace Ian Rogers
  4 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2026-06-04 21:49 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 pattern of strdup() and free() is found, and I think it just can
use the original backtrace directly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-timechart.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 4efac73a714c5e5f..04dbb944a42720b4 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -300,7 +300,7 @@ static void pid_put_sample(struct timechart *tchart, int pid, int type,
 	sample->type = type;
 	sample->next = c->samples;
 	sample->cpu = cpu;
-	sample->backtrace = backtrace ? strdup(backtrace) : NULL;
+	sample->backtrace = backtrace;
 	c->samples = sample;
 
 	if (sample->type == TYPE_RUNNING && end > start && start > 0) {
@@ -429,12 +429,14 @@ static void sched_wakeup(struct timechart *tchart, int cpu, u64 timestamp,
 	struct per_pid *p;
 	struct wake_event *we = zalloc(sizeof(*we));
 
-	if (!we)
+	if (!we) {
+		free((char *)backtrace);
 		return;
+	}
 
 	we->time = timestamp;
 	we->waker = waker;
-	we->backtrace = backtrace ? strdup(backtrace) : NULL;
+	we->backtrace = backtrace;
 
 	if ((flags & TRACE_FLAG_HARDIRQ) || (flags & TRACE_FLAG_SOFTIRQ))
 		we->waker = -1;
@@ -461,20 +463,28 @@ static void sched_switch(struct timechart *tchart, int cpu, u64 timestamp,
 			 const char *backtrace)
 {
 	struct per_pid *p = NULL, *prev_p;
+	bool backtrace_used = false;
 
 	prev_p = find_create_pid(tchart, prev_pid);
 
 	p = find_create_pid(tchart, next_pid);
 
-	if (prev_p->current && prev_p->current->state != TYPE_NONE)
+	if (prev_p->current && prev_p->current->state != TYPE_NONE) {
 		pid_put_sample(tchart, prev_pid, TYPE_RUNNING, cpu,
 			       prev_p->current->state_since, timestamp,
 			       backtrace);
+		backtrace_used = true;
+	}
 	if (p && p->current) {
-		if (p->current->state != TYPE_NONE)
+		if (p->current->state != TYPE_NONE) {
+			if (backtrace && backtrace_used)
+				backtrace = strdup(backtrace);
+
 			pid_put_sample(tchart, next_pid, p->current->state, cpu,
 				       p->current->state_since, timestamp,
 				       backtrace);
+			backtrace_used = true;
+		}
 
 		p->current->state_since = timestamp;
 		p->current->state = TYPE_RUNNING;
@@ -488,6 +498,9 @@ static void sched_switch(struct timechart *tchart, int cpu, u64 timestamp,
 		if (prev_state == 0)
 			prev_p->current->state = TYPE_WAITING;
 	}
+
+	if (!backtrace_used)
+		free((char *)backtrace);
 }
 
 /*
@@ -655,7 +668,6 @@ process_sample_sched_wakeup(struct timechart *tchart,
 
 	backtrace = cat_backtrace(sample, &tchart->session->machines.host);
 	sched_wakeup(tchart, sample->cpu, sample->time, waker, wakee, flags, backtrace);
-	free((char *)backtrace);
 	return 0;
 }
 
@@ -678,7 +690,6 @@ process_sample_sched_switch(struct timechart *tchart,
 	backtrace = cat_backtrace(sample, &tchart->session->machines.host);
 	sched_switch(tchart, sample->cpu, sample->time, prev_pid, next_pid,
 		     prev_state, backtrace);
-	free((char *)backtrace);
 	return 0;
 }
 
-- 
2.54.0.1032.g2f8565e1d1-goog


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 0/4] perf timechart: Small optimization for backtrace
  2026-06-04 21:49 [PATCH v2 0/4] perf timechart: Small optimization for backtrace Namhyung Kim
                   ` (3 preceding siblings ...)
  2026-06-04 21:49 ` [PATCH v2 4/4] perf timechart: Remove unnecessary copy of backtrace Namhyung Kim
@ 2026-06-04 22:38 ` Ian Rogers
  2026-06-04 23:34   ` Namhyung Kim
  4 siblings, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2026-06-04 22:38 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Thu, Jun 4, 2026 at 2:49 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> I found an issue int timechart backtrace handling during the last review.
> The goal is to reduce unnecessary work in generating backtrace string.
>
> v2 changes)
>  * fix memory leaks when backtrace is not used
>  * copy backtrace when it's used twice

Hi Namhyung,

thanks for cleaning this up and the fixes! Running a build of:
$ make -C tools/perf O=/tmp/perf DEBUG=1
EXTRA_CFLAGS="-fno-omit-frame-pointer -O0 -g -fsanitize=address" clean
all

I get an error but the test harness makes it look like the test is
passing (I added set -x to the test and removed the "> /dev/null
2>&1"):
```
$ sudo /tmp/perf/perf test -vv timechart
136: perf timechart tests
136: perf timechart tests:
---- start ----
test child forked, pid 937043
+ err=0
++ mktemp /tmp/__perf_timechart_test.perf.data.XXXXX
+ perfdata=/tmp/__perf_timechart_test.perf.data.cXMvK
++ mktemp /tmp/__perf_timechart_test.output.XXXXX.svg
+ output=/tmp/__perf_timechart_test.output.xZXfy.svg
+ trap trap_cleanup EXIT TERM INT
+ perf check feature -q libtraceevent
+ test_timechart
+ echo 'Basic perf timechart test'
Basic perf timechart test
+ perf timechart record -o /tmp/__perf_timechart_test.perf.data.cXMvK true
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 4.135 MB
/tmp/__perf_timechart_test.perf.data.cXMvK (748 samples) ]

=================================================================
==937049==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 136 byte(s) in 1 object(s) allocated from:
   #0 0x7fd211119a23 in calloc
../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:74
   #1 0x555c44044416 in timechart__record ~/linux/tools/perf/builtin-timech
art.c:1895
   #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
c:2073
   #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
   #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
98
   #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
   #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
   #7 0x7fd206e29f76 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
   #8 0x7fffcf8a352a  ([stack]+0x2052a)

Direct leak of 10 byte(s) in 2 object(s) allocated from:
   #0 0x7fd211113e20 in strdup
../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
   #1 0x555c440444a1 in timechart__record ~/linux/tools/perf/builtin-timech
art.c:1902
   #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
c:2073
   #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
   #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
98
   #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
   #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
   #7 0x7fd206e29f76 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
   #8 0x7fffcf8a352a  ([stack]+0x2052a)

Indirect leak of 44 byte(s) in 4 object(s) allocated from:
   #0 0x7fd211113e20 in strdup
../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
   #1 0x555c440445ef in timechart__record ~/linux/tools/perf/builtin-timech
art.c:1908
   #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
c:2073
   #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
   #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
98
   #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
   #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
   #7 0x7fd206e29f76 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
   #8 0x7fffcf8a352a  ([stack]+0x2052a)

Indirect leak of 41 byte(s) in 4 object(s) allocated from:
   #0 0x7fd211113e20 in strdup
../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
   #1 0x555c44044696 in timechart__record ~/linux/tools/perf/builtin-timech
art.c:1911
   #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
c:2073
   #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
   #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
98
   #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
   #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
   #7 0x7fd206e29f76 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
   #8 0x7fffcf8a352a  ([stack]+0x2052a)

Indirect leak of 11 byte(s) in 4 object(s) allocated from:
   #0 0x7fd211113e20 in strdup
../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
   #1 0x555c440444a1 in timechart__record ~/linux/tools/perf/builtin-timech
art.c:1902
   #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
c:2073
   #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
   #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
98
   #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
   #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
   #7 0x7fd206e29f76 in __libc_start_call_main
../sysdeps/nptl/libc_start_call_main.h:58
   #8 0x7fffcf8a352a  ([stack]+0x2052a)

SUMMARY: AddressSanitizer: 242 byte(s) leaked in 15 allocation(s).
+ echo 'Basic perf timechart test [Skipped: perf timechart record
failed (permissions/events?)]'
Basic perf timechart test [Skipped: perf timechart record failed
(permissions/events?)]
+ return
+ cleanup
+ rm -f /tmp/__perf_timechart_test.perf.data.cXMvK
+ rm -f /tmp/__perf_timechart_test.output.xZXfy.svg
+ trap - EXIT TERM INT
+ exit 0
---- end(0) ----
136: perf timechart tests
: Ok
```

It was this test that I was trying to make leak sanitizer clean when
doing my fixes.

Thanks,
Ian

> Thanks,
> Namhyung
>
>
> Namhyung Kim (4):
>   perf timechart: Don't pass @event to cat_backtrace()
>   perf timechart: Generate backtrace only if needed
>   perf timechart: Remove unused backtrace in trace_handler
>   perf timechart: Remove unnecessary copy of backtrace
>
>  tools/perf/builtin-timechart.c | 107 ++++++++++++++++-----------------
>  1 file changed, 52 insertions(+), 55 deletions(-)
>
> --
> 2.54.0.1032.g2f8565e1d1-goog
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 0/4] perf timechart: Small optimization for backtrace
  2026-06-04 22:38 ` [PATCH v2 0/4] perf timechart: Small optimization for backtrace Ian Rogers
@ 2026-06-04 23:34   ` Namhyung Kim
  0 siblings, 0 replies; 7+ messages in thread
From: Namhyung Kim @ 2026-06-04 23:34 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

Hi Ian,

On Thu, Jun 04, 2026 at 03:38:05PM -0700, Ian Rogers wrote:
> On Thu, Jun 4, 2026 at 2:49 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > I found an issue int timechart backtrace handling during the last review.
> > The goal is to reduce unnecessary work in generating backtrace string.
> >
> > v2 changes)
> >  * fix memory leaks when backtrace is not used
> >  * copy backtrace when it's used twice
> 
> Hi Namhyung,
> 
> thanks for cleaning this up and the fixes! Running a build of:
> $ make -C tools/perf O=/tmp/perf DEBUG=1
> EXTRA_CFLAGS="-fno-omit-frame-pointer -O0 -g -fsanitize=address" clean
> all
> 
> I get an error but the test harness makes it look like the test is
> passing (I added set -x to the test and removed the "> /dev/null
> 2>&1"):
> ```
> $ sudo /tmp/perf/perf test -vv timechart
> 136: perf timechart tests
> 136: perf timechart tests:
> ---- start ----
> test child forked, pid 937043
> + err=0
> ++ mktemp /tmp/__perf_timechart_test.perf.data.XXXXX
> + perfdata=/tmp/__perf_timechart_test.perf.data.cXMvK
> ++ mktemp /tmp/__perf_timechart_test.output.XXXXX.svg
> + output=/tmp/__perf_timechart_test.output.xZXfy.svg
> + trap trap_cleanup EXIT TERM INT
> + perf check feature -q libtraceevent
> + test_timechart
> + echo 'Basic perf timechart test'
> Basic perf timechart test
> + perf timechart record -o /tmp/__perf_timechart_test.perf.data.cXMvK true
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 4.135 MB
> /tmp/__perf_timechart_test.perf.data.cXMvK (748 samples) ]
> 
> =================================================================
> ==937049==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 136 byte(s) in 1 object(s) allocated from:
>    #0 0x7fd211119a23 in calloc
> ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:74
>    #1 0x555c44044416 in timechart__record ~/linux/tools/perf/builtin-timech
> art.c:1895
>    #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
> c:2073
>    #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
>    #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
> 98
>    #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
>    #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
>    #7 0x7fd206e29f76 in __libc_start_call_main
> ../sysdeps/nptl/libc_start_call_main.h:58
>    #8 0x7fffcf8a352a  ([stack]+0x2052a)
> 
> Direct leak of 10 byte(s) in 2 object(s) allocated from:
>    #0 0x7fd211113e20 in strdup
> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
>    #1 0x555c440444a1 in timechart__record ~/linux/tools/perf/builtin-timech
> art.c:1902
>    #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
> c:2073
>    #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
>    #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
> 98
>    #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
>    #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
>    #7 0x7fd206e29f76 in __libc_start_call_main
> ../sysdeps/nptl/libc_start_call_main.h:58
>    #8 0x7fffcf8a352a  ([stack]+0x2052a)
> 
> Indirect leak of 44 byte(s) in 4 object(s) allocated from:
>    #0 0x7fd211113e20 in strdup
> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
>    #1 0x555c440445ef in timechart__record ~/linux/tools/perf/builtin-timech
> art.c:1908
>    #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
> c:2073
>    #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
>    #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
> 98
>    #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
>    #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
>    #7 0x7fd206e29f76 in __libc_start_call_main
> ../sysdeps/nptl/libc_start_call_main.h:58
>    #8 0x7fffcf8a352a  ([stack]+0x2052a)
> 
> Indirect leak of 41 byte(s) in 4 object(s) allocated from:
>    #0 0x7fd211113e20 in strdup
> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
>    #1 0x555c44044696 in timechart__record ~/linux/tools/perf/builtin-timech
> art.c:1911
>    #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
> c:2073
>    #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
>    #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
> 98
>    #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
>    #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
>    #7 0x7fd206e29f76 in __libc_start_call_main
> ../sysdeps/nptl/libc_start_call_main.h:58
>    #8 0x7fffcf8a352a  ([stack]+0x2052a)
> 
> Indirect leak of 11 byte(s) in 4 object(s) allocated from:
>    #0 0x7fd211113e20 in strdup
> ../../../../src/libsanitizer/asan/asan_interceptors.cpp:593
>    #1 0x555c440444a1 in timechart__record ~/linux/tools/perf/builtin-timech
> art.c:1902
>    #2 0x555c44046c19 in cmd_timechart ~/linux/tools/perf/builtin-timechart.
> c:2073
>    #3 0x555c440828a4 in run_builtin ~/linux/tools/perf/perf.c:348
>    #4 0x555c44082d42 in handle_internal_command ~/linux/tools/perf/perf.c:3
> 98
>    #5 0x555c44083110 in run_argv ~/linux/tools/perf/perf.c:442
>    #6 0x555c44083667 in main ~/linux/tools/perf/perf.c:549
>    #7 0x7fd206e29f76 in __libc_start_call_main
> ../sysdeps/nptl/libc_start_call_main.h:58
>    #8 0x7fffcf8a352a  ([stack]+0x2052a)
> 
> SUMMARY: AddressSanitizer: 242 byte(s) leaked in 15 allocation(s).
> + echo 'Basic perf timechart test [Skipped: perf timechart record
> failed (permissions/events?)]'
> Basic perf timechart test [Skipped: perf timechart record failed
> (permissions/events?)]
> + return
> + cleanup
> + rm -f /tmp/__perf_timechart_test.perf.data.cXMvK
> + rm -f /tmp/__perf_timechart_test.output.xZXfy.svg
> + trap - EXIT TERM INT
> + exit 0
> ---- end(0) ----
> 136: perf timechart tests
> : Ok
> ```
> 
> It was this test that I was trying to make leak sanitizer clean when
> doing my fixes.

I think these leaks are pre-existing.  I don't see where it frees cpu
samples and other events.  I'll take a look and send a fix separately.

Thanks,
Namhyung


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-06-04 23:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04 21:49 [PATCH v2 0/4] perf timechart: Small optimization for backtrace Namhyung Kim
2026-06-04 21:49 ` [PATCH v2 1/4] perf timechart: Don't pass @event to cat_backtrace() Namhyung Kim
2026-06-04 21:49 ` [PATCH v2 2/4] perf timechart: Generate backtrace only if needed Namhyung Kim
2026-06-04 21:49 ` [PATCH v2 3/4] perf timechart: Remove unused backtrace in trace_handler Namhyung Kim
2026-06-04 21:49 ` [PATCH v2 4/4] perf timechart: Remove unnecessary copy of backtrace Namhyung Kim
2026-06-04 22:38 ` [PATCH v2 0/4] perf timechart: Small optimization for backtrace Ian Rogers
2026-06-04 23:34   ` Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.