All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vincent Donnefort <vdonnefort@google.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: mhiramat@kernel.org, mathieu.desnoyers@efficios.com,
	linux-trace-kernel@vger.kernel.org, maz@kernel.org,
	oliver.upton@linux.dev, joey.gouly@arm.com,
	suzuki.poulose@arm.com, yuzenghui@huawei.com,
	kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	jstultz@google.com, qperret@google.com, will@kernel.org,
	aneesh.kumar@kernel.org, kernel-team@android.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 04/24] tracing: Add reset to trace remotes
Date: Wed, 10 Sep 2025 10:45:05 +0100	[thread overview]
Message-ID: <aMFIoY5yfQa2Mzgk@google.com> (raw)
In-Reply-To: <20250909145236.69192cd0@gandalf.local.home>

On Tue, Sep 09, 2025 at 02:52:36PM -0400, Steven Rostedt wrote:
> On Tue, 9 Sep 2025 14:39:48 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > But anyway, I think it should work for the remote buffers too. Let me go
> > and fix the current iterator.
> 
> I thought it was broken but it isn't ;-) I did it properly, I just didn't
> look deep enough.
> 
> So yeah, look at the rb_iter_head_event() code. The ring buffer iterator
> has a copy of the event. It has:
> 
> 	if ((iter->head + length) > commit || length > iter->event_size)
> 		/* Writer corrupted the read? */
> 		goto reset;
> 
> 	memcpy(iter->event, event, length);
> 	/*
> 	 * If the page stamp is still the same after this rmb() then the
> 	 * event was safely copied without the writer entering the page.
> 	 */
> 	smp_rmb();
> 
> 	/* Make sure the page didn't change since we read this */
> 	if (iter->page_stamp != iter_head_page->page->time_stamp ||
> 	    commit > rb_page_commit(iter_head_page))
> 		goto reset;
> 
> It first checks before copying that the data it's about to copy hasn't been
> touched by the writer.
> 
> It then copies the event into the iter->event temp buffer.
> 
> Then it checks again that the data hasn't been touched. If it has, then
> consider the data corrupt and end the iteration. This is how the "trace"
> file works. I believe you could do the same for the remote code.
> 
> If we are gonna keep the "trace" file, let's make sure it's fully
> implemented.

I was more worry about the ring-buffer page order that can be reshuffled on each
swap_reader_page(), making the page links useless in the kernel. Ideally, the
meta-page would keep the page ID order somewhere.

Alternatively, we could walk all the buffer pages to read the timestamp and
re-create the order but that sounds quite cumbersome.

> 
> -- Steve

  reply	other threads:[~2025-09-10  9:45 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-21  8:13 [PATCH v6 00/24] Tracefs support for pKVM Vincent Donnefort
2025-08-21  8:13 ` [PATCH v6 01/24] ring-buffer: Add page statistics to the meta-page Vincent Donnefort
2025-08-21  8:13 ` [PATCH v6 02/24] ring-buffer: Introduce ring-buffer remotes Vincent Donnefort
2025-09-08 23:13   ` Steven Rostedt
2025-08-21  8:13 ` [PATCH v6 03/24] tracing: Introduce trace remotes Vincent Donnefort
2025-09-08 23:36   ` Steven Rostedt
2025-09-09 12:08     ` Vincent Donnefort
2025-09-09 13:38       ` Steven Rostedt
2025-09-09 16:10         ` Vincent Donnefort
2025-09-09 18:19           ` Steven Rostedt
2025-08-21  8:13 ` [PATCH v6 04/24] tracing: Add reset to " Vincent Donnefort
2025-09-08 23:37   ` Steven Rostedt
2025-09-09 12:10     ` Vincent Donnefort
2025-09-09 13:40       ` Steven Rostedt
2025-09-09 16:14         ` Vincent Donnefort
2025-09-09 18:39           ` Steven Rostedt
2025-09-09 18:52             ` Steven Rostedt
2025-09-10  9:45               ` Vincent Donnefort [this message]
2025-09-10 16:45                 ` Steven Rostedt
2025-08-21  8:13 ` [PATCH v6 05/24] tracing: Add init callback " Vincent Donnefort
2025-08-21  8:13 ` [PATCH v6 06/24] tracing: Add events " Vincent Donnefort
2025-09-09 21:47   ` Steven Rostedt
2025-09-09 22:24     ` Steven Rostedt
2025-08-21  8:13 ` [PATCH v6 07/24] tracing: Add events/ root files " Vincent Donnefort
2025-09-09 21:52   ` Steven Rostedt
2025-08-21  8:13 ` [PATCH v6 08/24] tracing: Add helpers to create trace remote events Vincent Donnefort
2025-08-21  8:13 ` [PATCH v6 09/24] ring-buffer: Export buffer_data_page and macros Vincent Donnefort
2025-08-21  8:13 ` [PATCH v6 10/24] tracing: Introduce simple_ring_buffer Vincent Donnefort
2025-09-09 22:26   ` Steven Rostedt
2025-09-10  9:47     ` Vincent Donnefort
2025-08-21  8:13 ` [PATCH v6 11/24] tracing: Add a trace remote module for testing Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 12/24] tracing: selftests: Add trace remote tests Vincent Donnefort
2025-09-03  4:58   ` Masami Hiramatsu
2025-08-21  8:14 ` [PATCH v6 13/24] tracing: load/unload page callbacks for simple_ring_buffer Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 14/24] tracing: Check for undefined symbols in simple_ring_buffer Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 15/24] KVM: arm64: Support unaligned fixmap in the pKVM hyp Vincent Donnefort
2025-09-10 16:47   ` Steven Rostedt
2025-09-10 17:06     ` Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 16/24] KVM: arm64: Add clock support for " Vincent Donnefort
2025-09-12 13:48   ` Will Deacon
2025-08-21  8:14 ` [PATCH v6 17/24] KVM: arm64: Add tracing capability " Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 18/24] KVM: arm64: Add trace remote " Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 19/24] KVM: arm64: Sync boot clock with " Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 20/24] KVM: arm64: Add trace reset to " Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 21/24] KVM: arm64: Add event support to the pKVM hyp and trace remote Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 22/24] KVM: arm64: Add hyp_enter/hyp_exit events to pKVM hyp Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 23/24] KVM: arm64: Add selftest event support " Vincent Donnefort
2025-08-21  8:14 ` [PATCH v6 24/24] tracing: selftests: Add pKVM trace remote tests Vincent Donnefort

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=aMFIoY5yfQa2Mzgk@google.com \
    --to=vdonnefort@google.com \
    --cc=aneesh.kumar@kernel.org \
    --cc=joey.gouly@arm.com \
    --cc=jstultz@google.com \
    --cc=kernel-team@android.com \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=maz@kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=oliver.upton@linux.dev \
    --cc=qperret@google.com \
    --cc=rostedt@goodmis.org \
    --cc=suzuki.poulose@arm.com \
    --cc=will@kernel.org \
    --cc=yuzenghui@huawei.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.