Re: [PATCH] tracing: Up the hist stacktrace size from 16 to 31

[Tom Zanussi <zanussi@kernel.org>]

Hi Steve,

On Fri, 2026-01-23 at 10:54 -0500, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.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@goodmis.org>

The patch itself looks good to me, thanks!

Reviewed-by: Tom Zanussi <zanussi@kernel.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];