On Fri, 6 Mar 2026 17:46:09 +0900 Masami Hiramatsu (Google) <[email protected]> wrote:
> On Thu, 5 Mar 2026 13:03:48 -0500 > Steven Rostedt <[email protected]> wrote: > > > On Thu, 26 Feb 2026 22:38:43 +0900 > > "Masami Hiramatsu (Google)" <[email protected]> wrote: > > > > > From: Masami Hiramatsu (Google) <[email protected]> > > > > > > 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: [email protected] > > > > 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;
