mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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.


      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®