[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <651c277f8bfc1892f99fd038236fafaec23e8203.camel@kernel.org>
Date: Mon, 26 Jan 2026 15:37:44 -0600
From: Tom Zanussi <zanussi@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>, LKML
<linux-kernel@...r.kernel.org>, Linux Trace Kernel
<linux-trace-kernel@...r.kernel.org>
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
<mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH] tracing: Up the hist stacktrace size from 16 to 31
Hi Steve,
On Fri, 2026-01-23 at 10:54 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@...dmis.org>
>
> Recording stacktraces is very useful, but the size of 16 deep is very
> restrictive. For example, in seeing where tasks schedule out in a non
> running state, the following can be used:
>
> ~# cd /sys/kernel/tracing
> ~# echo 'hist:keys=common_stacktrace:vals=hitcount if prev_state & 3' > events/sched/sched_switch/trigger
> ~# cat events/sched/sched_switch/hist
> [..]
> { common_stacktrace:
> __schedule+0xdc0/0x1860
> schedule+0x27/0xd0
> schedule_timeout+0xb5/0x100
> wait_for_completion+0x8a/0x140
> xfs_buf_iowait+0x20/0xd0 [xfs]
> xfs_buf_read_map+0x103/0x250 [xfs]
> xfs_trans_read_buf_map+0x161/0x310 [xfs]
> xfs_btree_read_buf_block+0xa0/0x120 [xfs]
> xfs_btree_lookup_get_block+0xa3/0x1e0 [xfs]
> xfs_btree_lookup+0xea/0x530 [xfs]
> xfs_alloc_fixup_trees+0x72/0x570 [xfs]
> xfs_alloc_ag_vextent_size+0x67f/0x800 [xfs]
> xfs_alloc_vextent_iterate_ags.constprop.0+0x52/0x230 [xfs]
> xfs_alloc_vextent_start_ag+0x9d/0x1b0 [xfs]
> xfs_bmap_btalloc+0x2af/0x680 [xfs]
> xfs_bmapi_allocate+0xdb/0x2c0 [xfs]
> } hitcount: 1
> [..]
>
> The above stops at 16 functions where knowing more would be useful. As the
> allocated storage for stacks is the same for strings, and that size is 256
> bytes, there is a lot of space not being used for stacktraces.
>
> 16 * 8 = 128
>
> Up the size to 31 (it requires the last slot to be zero, so it can't be 32).
>
Yeah, it can't be 32 because of the extra slot needed for saving the
number of elements. If the stacktrace doesn't take up the full 31
elements, it writes a 0 after the last one to mark the end; if it fills
it up, the 0 isn't written since the size of the array is known.
> Also change the BUILD_BUG_ON() to allow the size of the stacktrace storage
> to be equal to the max size. It already accounts for the empty slot via
> a "+ 1":
>
> BUILD_BUG_ON((HIST_STACKTRACE_DEPTH + 1) * sizeof(long) >= STR_VAR_LEN_MAX);
>
> Change that from ">=" to just ">", as now they are equal.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
The patch itself looks good to me, thanks!
Reviewed-by: Tom Zanussi <zanussi@...nel.org>
> ---
> kernel/trace/trace.h | 2 +-
> kernel/trace/trace_events_hist.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 8888fc9335b6..69e7defba6c6 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -128,7 +128,7 @@ enum trace_type {
>
> #define FAULT_STRING "(fault)"
>
> -#define HIST_STACKTRACE_DEPTH 16
> +#define HIST_STACKTRACE_DEPTH 31
> #define HIST_STACKTRACE_SIZE (HIST_STACKTRACE_DEPTH * sizeof(unsigned long))
> #define HIST_STACKTRACE_SKIP 5
>
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 24441b30a1a4..dfb7c4754bf8 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -3154,7 +3154,7 @@ static inline void __update_field_vars(struct tracing_map_elt *elt,
> u64 var_val;
>
> /* Make sure stacktrace can fit in the string variable length */
> - BUILD_BUG_ON((HIST_STACKTRACE_DEPTH + 1) * sizeof(long) >= STR_VAR_LEN_MAX);
> + BUILD_BUG_ON((HIST_STACKTRACE_DEPTH + 1) * sizeof(long) > STR_VAR_LEN_MAX);
>
> for (i = 0, j = field_var_str_start; i < n_field_vars; i++) {
> struct field_var *field_var = field_vars[i];
Powered by blists - more mailing lists