On Mon, 9 Mar 2026 08:53:17 +0900
Masami Hiramatsu (Google) <[email protected]> wrote:
> > > @@ -1827,11 +1833,6 @@ static bool rb_cpu_meta_valid(struct
> > > ring_buffer_cpu_meta *meta, int cpu,
> > > return false;
> > > }
> > >
> > > - if ((unsigned)local_read(&subbuf->commit) > subbuf_size) {
> > > - pr_info("Ring buffer boot meta [%d] buffer invalid
> > > commit\n", cpu);
> > > - return false;
> > > - }
> >
> > This should still be checked, although it doesn't need to fail the loop
> > but instead continue to the next buffer.
>
> We already have another check of the data in the loop in
> rb_meta_validate_events() so data corruption should be
> handled there.
Hmm, OK.
>
> >
> > Also, I mentioned that if the commit == RB_MISSED_EVENTS, then we know
> > the sub buffer was corrupted and should be skipped.
>
> Yes, if RB_MISSED_EVENTS bit is set, the commit field is out of range.
> That is checked in rb_validate_buffer().
>
> >
> > And honestly, the commit should never be greater than the subbuf_size,
> > even if corrupted. As we are only worried about corruption due to cache
> > not writing out. That should not corrupt the commit size (now we can
> > ignore the flags and use page size instead).
>
> Hmm, but if the kernel crash and reboot when it sets RB_MISSED_EVENTS,
> we will see the bit is set and commit size is different.
The RB_MISSED_EVENTS is only set on the reader page.
If the kernel crashes no boot up while reading the validated buffer,
then that's a bit more than what this is supposed to handle.
>
> Note, I think the reader_page RB_MISSED_EVENTS flag is not cleared after
> read. commit ca296d32ece3 ("tracing: ring_buffer: Rewind persistent
> ring buffer on reboot") drops clearing commit field for unwinding the
> buffer.
But that should be fine, as it's only read only. Once tracing is
started, it should be reset.
-- Steve
