Re: [PATCH v5 2/3] ring-buffer: Handle RB_MISSED_* flags on commit field correctly

[Steven Rostedt]

On Fri, 6 Mar 2026 17:46:09 +0900
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> wrote:

> On Thu, 5 Mar 2026 13:03:48 -0500
> Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> 
> > On Thu, 26 Feb 2026 22:38:43 +0900
> > "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote:
> >   
> > > From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > > 
> > > Since the MSBs of rb_data_page::commit are used for storing
> > > RB_MISSED_EVENTS and RB_MISSED_STORED, we need to mask out those bits
> > > when it is used for finding the size of data pages.
> > > 
> > > Fixes: 5f3b6e839f3c ("ring-buffer: Validate boot range memory events")
> > > Fixes: 5b7be9c709e1 ("ring-buffer: Add test to validate the time stamp deltas")
> > > Cc: stable@xxxxxxxxxxxxxxx  
> > 
> > This is unneeded for the current way things work.
> > 
> > The missed events flags are added when a page is read, so the commits in
> > the write buffer should never have those flags set. If they did, the ring
> > buffer code itself would break.  
> 
> Hmm, but commit ca296d32ece3 ("tracing: ring_buffer: Rewind persistent 
> ring buffer on reboot") may change it. Maybe we should treat it while
> unwinding it?

Might change what?

> 
> > 
> > But as patch 3 is adding a flag, you should likely merge this and patch 3
> > together, as the only way that flag would get set is if the validator set
> > it on a previous boot. And then this would be needed for subsequent boots
> > that did not reset the buffer.  
> 
> It is OK to combine these 2 patches. But my question is that when the flag
> must be checked and when it must be ignored. Since the flags are encoded
> to commit, if that is used for limiting or indexing inside the page,
> we must mask the flag or check the max size to avoid accessing outside of
> the subpage.
> 
> > 
> > Hmm, I don't think we even need to do that! Because if it is set, it would
> > simply warn again that a page is invalid, and I think we *want* that! As it
> > would preserve that pages were invalid and not be cleared with a simple
> > reboot.  
> 
> OK, then I don't mark it, but just invalidate the subpage.

No, I mean you can mark it, but then just have the validator skip it.
Something like:

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 156ed19fb569..c98ab86cf384 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1950,6 +1951,10 @@ static void rb_meta_validate_events(struct ring_buffer_per_cpu *cpu_buffer)
 	rb_dec_page(&head_page);
 	for (i = 0; i < meta->nr_subbufs + 1; i++, rb_dec_page(&head_page)) {
 
+		/* Skip if this buffer was flagged as bad before */
+		if (local_read(&head_page->page->commit) ==  RB_MISSED_EVENTS)
+			continue;
+
 		/* Rewind until tail (writer) page. */
 		if (head_page == cpu_buffer->tail_page)
 			break;