From: Pengfei Li <[email protected]>
On Tue, 14 Jul 2026 17:11:44 -0400 Steven Rostedt wrote:
> smap->entries = vcalloc(smap->map_size, sizeof(*smap->entries));
> Make the error paths have: ... goto fail; ...
Will switch both entries and elts to vcalloc(), and collapse the
error paths into a single goto-fail ladder (freeing in reverse
alloc order, including the drops percpu).
> Do not add anonymous blocks in functions.
Will move the cpu declaration to the top of ftrace_stackmap_reset().
> Really should have ftrace_stackmap_bin_entry have a flexible
> array: u64 ips[];
Agreed - will add the u64 ips[] flexible member and use
struct_size(e, ips, nr) in stackmap_bin_open(). On-disk layout is
unchanged, so the bin format stays compatible.
I'll also fold in two issues I found locally: a missing lock on one
init failure path, and TRACE_STACK_ID not being handled in the
function_graph output (it falls through to print_graph_comment()
instead of being punted like TRACE_STACK).
One design point I'd like your steer on before respinning: reset
checks tracer_tracing_is_on() once under trace_types_lock, but
traceon triggers (ftrace_traceon / traceon_trigger) can re-enable
tracing without that lock during reset's clear phase. As far as I
can tell this is a semantic-contract issue, not a memory-safety
one: the map is protected by the resetting flag (get_id bails with
-EINVAL and synchronize_rcu() drains in-flight callers), and the
ring buffer pages aren't freed by reset, so the worst case is a
non-empty / inconsistent buffer after reset rather than corruption.
So I'm leaning toward documenting reset as best-effort ("stop
tracing, including traceon triggers, before reset") rather than
adding machinery to block the window. Does that match your view, or
is there a ring-buffer-state hazard I'm missing that would justify
blocking it explicitly?
On the element pool: your `-l '*lock*'` run is a good illustration -
the ~326K-line stack_map dump is the 16K-entry pool (bits=14)
filling up quickly under broad function tracing. I'm leaning toward
keeping eager allocation for v5 (it keeps the hot path free of an
allocation-failure path), but I'm happy to switch to lazy allocation
on the first 'echo 1 > options/stackmap' if you'd rather not pay the
~8MB resident cost when the option is never enabled. I'd keep the
stack_map_bin interface as-is.
Thanks,
Pengfei
