From: Steven Rostedt <rostedt@goodmis.org>
To: Li Pengfei <ljdlns1987@gmail.com>
Cc: mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
mark.rutland@arm.com, corbet@lwn.net, skhan@linuxfoundation.org,
linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org,
zhangbo56@xiaomi.com
Subject: Re: [RFC PATCH v7 00/10] trace: stack trace deduplication for ftrace ring buffer
Date: Tue, 3 Nov 2026 19:44:29 -0500 [thread overview]
Message-ID: <20261103194429.739feafc@gandalf.local.home> (raw)
In-Reply-To: <20261013020321.1898006-1-lipengfei28@xiaomi.com>
On Tue, 13 Oct 2026 10:03:21 +0800
Li Pengfei <ljdlns1987@gmail.com> wrote:
> Hi Steven, Masami,
>
> Open question 2 in the v7 cover letter was whether the map generation
> should be part of the ABI. I think it has to be, and I would rather
> settle the record format with you before sending a v8.
>
> The problem is that reset clears the map and resets the element cursor to
> zero, so a new stack takes slot 42 immediately afterwards. A stack_id 42
> recorded before the reset is still sitting in the ring buffer, and
> resolving it now yields the new occupant of that slot. Userspace cannot
> tell that apart from a correct lookup - it gets a plausible stack, not an
> error.
>
> I first tried to fix it in the table, storing the generation per slot and
> comparing it with the current one on lookup. That does not work: the
> comparison tells you which generation the slot now holds, not which
> generation the caller's id came from, and the case that matters is
> exactly when those differ and the slot has already been refilled. The
> identity has to travel with the record.
>
> So for v8:
>
> struct stack_id_entry {
> struct trace_entry ent;
> u64 stack_cookie; /* gen << 32 | id */
> };
>
> A single opaque 64-bit cookie, high half generation, low half slot. An id
> is still only valid within its own generation; the cookie makes crossing
> that boundary detectable.
>
> stack_map_bin reports the generation of the dump in the header word that
> is currently reserved (ABI version 1 -> 2), so a dump and a record can be
> paired. Text output splits the cookie rather than printing it raw:
>
> <stack_id 42 gen 3>
>
> Cost, against the workload measured in the v7 cover letter:
>
> full stack, ~20 frames trace_entry + size + 8*20 = 172 B
> v7, 4-byte id trace_entry + 4 = 12 B
> v8, 8-byte cookie trace_entry + 8 = 16 B
This is reasonable.
>
> Retention on that workload drops from about 2.17x to about 2.0x. I will
> re-measure rather than quote that from arithmetic.
>
> Two alternatives I rejected, in case you would have picked one:
>
> Packing the generation into the existing 32-bit id keeps the record at 4
> bytes, but with FTRACE_STACKMAP_BITS_MAX at 18 the slot index needs 19
> bits, leaving 12 bits of generation in a signed field. 4096 resets wrap,
> and a wrapped generation passes the check - the same failure, only
> rarer. It also ties the id layout to the capacity limit.
>
> Two separate u32 fields come to the same 16 bytes and would make the
> format file self-describing, so an event filter could match the slot
> directly. I went with the cookie because an opaque value says the whole
> thing is the identifier - two fields invite userspace to read the slot
> number on its own, which is exactly what is not meaningful without its
> generation. If the filtering matters more, that is an easy change.
>
> One consequence I had not expected. If a trace spans several resets,
> dumping stack_map_bin before each one keeps the whole timeline resolvable
> afterwards: every record says which generation it belongs to and every
> dump header says which generation it is. Without the tag, saving several
> dumps is pointless, since nothing says which dump a given record should
> be read against.
>
> Does that format look right?
Yeah, the cookie + generation looks like the best way forward.
Thanks,
-- Steve
>
> v7 split the series into 10 focused patches following Masami's review of
> v6, and the design is otherwise unchanged.
prev parent reply other threads:[~2026-11-04 0:44 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-12 8:37 Li Pengfei
2026-09-12 8:37 ` [RFC PATCH v7 01/10] trace: add lock-free stackmap for stack trace deduplication Li Pengfei
2026-11-03 22:49 ` Steven Rostedt
2026-09-12 8:37 ` [RFC PATCH v7 02/10] trace: use the stackmap from the ftrace stack recording path Li Pengfei
2026-11-04 0:34 ` Steven Rostedt
2026-09-12 8:37 ` [RFC PATCH v7 03/10] trace: add stackmap statistics interface Li Pengfei
2026-11-04 0:36 ` Steven Rostedt
2026-09-12 8:37 ` [RFC PATCH v7 04/10] trace: add stackmap binary export Li Pengfei
2026-09-12 8:37 ` [RFC PATCH v7 05/10] trace: make the stackmap capacity settable on the kernel command line Li Pengfei
[not found] ` <20260912085841.48E7B1F00893@smtp.kernel.org>
2026-09-14 6:08 ` Li Pengfei
2026-09-12 8:37 ` [RFC PATCH v7 06/10] Documentation: tracing: document the ftrace stackmap Li Pengfei
2026-09-12 8:37 ` [RFC PATCH v7 07/10] tools/tracing: add a parser for the stackmap binary export Li Pengfei
2026-09-12 8:37 ` [RFC PATCH v7 08/10] selftests/ftrace: add a stackmap basic functionality test Li Pengfei
[not found] ` <20260912090213.C77861F00893@smtp.kernel.org>
2026-09-14 6:08 ` Li Pengfei
2026-09-12 8:37 ` [RFC PATCH v7 09/10] selftests/ftrace: add a stackmap reset and binary ABI test Li Pengfei
2026-09-12 8:37 ` [RFC PATCH v7 10/10] selftests/ftrace: add a stackmap instance gating test Li Pengfei
2026-10-13 2:03 ` [RFC PATCH v7 00/10] trace: stack trace deduplication for ftrace ring buffer Li Pengfei
2026-11-04 0:44 ` Steven Rostedt [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261103194429.739feafc@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=corbet@lwn.net \
--cc=linux-doc@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=ljdlns1987@gmail.com \
--cc=mark.rutland@arm.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=skhan@linuxfoundation.org \
--cc=zhangbo56@xiaomi.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®