Hi Steve, On Fri, 2026-01-23 at 10:54 -0500, Steven Rostedt wrote: > From: Steven Rostedt <[email protected]> > > 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) <[email protected]> The patch itself looks good to me, thanks! Reviewed-by: Tom Zanussi <[email protected]> > --- > 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];
