On Tue, 08 Sep 2026 10:21:40 +0900 Masami Hiramatsu (Google) <[email protected]> wrote: > > Kernel command line parameter: > > - ftrace_stackmap.bits=N: set map capacity (2^N unique stacks, > > range 10-18, default 14) > > Ah, this kernel cmdline parameter is also be a separated patch, > because this is not fundamentary needed.
Right, it is not needed for the basic functionality. In v7 the core patch hardcodes the default capacity, and a later patch adds the early_param along with the [10, 18] clamp and the memory-footprint documentation that goes with it. > It is OK to use an official address for SoB, but to make sure this > address work, please at least Cc to this address. [email protected] is on the Cc list of this series and receives the list traffic; it is a working address. It will stay on Cc for v7 and any follow-up. > > + if (!smap) { > > + seq_puts(m, "stackmap not initialized\n"); > > + return 0; > > + } > > + > > You also need down_read(&smap->reader_sem) here for serializing. [...] > > + seq_printf(m, "success_rate: %llu%%\n", successes); > > and up_read(&smap->reader_sem) too. Correct, this is a real hole. Reset clears next_elt and the per-CPU successes/drops counters under the write side of reader_sem, so an unserialized stat read can straddle it and mix pre- and post-reset values -- for instance a non-zero entries count next to counters that have already been zeroed. v7 takes the rwsem for read around the whole sampling and formatting block. > > + * At bits=18 this caps at ~135 MB. The file is mode 0440 > > + * (TRACE_MODE_READ), so only privileged users can open it. > > Hmm, this is too huge to make a copy inside the kernel. > If the stackmap is only increasing, and can avoid resetting by > reader_sem, what about rewriting this as a raw-output mode of > seq_file? > You can use seq_write() to seq_file buffer. Agreed on both counts: the copy is far too large, and seq_file with seq_write() removes the need for it entirely. The element pool only grows and slots are never recycled, so an iterator can walk the table in index order and emit each populated entry through seq_write() as it goes, holding reader_sem for read across each pass the way the text export already does. That drops the per-open cost to the seq_file buffer, and the header layout and version stay as they are: open() counts the populated entries once to fill nr_stacks, which is a plain memory scan of the table rather than a copy of it. One caveat on relying on reader_sem alone: seq_file releases it between read() calls, so a reset landing between two reads of the same fd would otherwise splice two generations of the map into a single output stream. For the text export that is merely confusing output, but a binary consumer would silently parse it as one coherent dump. So the reworked export also carries a generation counter, bumped by reset; open() records it along with the entry count, and each pass revalidates it and fails the read if it changed. A reader that raced a reset gets an error and can retry instead of receiving a spliced dump. Pengfei
