Re: [PATCH v20 02/10] ring-buffer: Skip invalid sub-buffers when rewinding persistent ring buffer

[Steven Rostedt]

On Thu, 21 May 2026 10:17:42 -0400
Steven Rostedt <rost...@goodmis.org> wrote:

> > > +         local_set(&bpage->entries, 0);  
> > [ ... ]  
> > > @@ -1915,25 +1936,29 @@ static void rb_meta_validate_events(struct 
> > > ring_buffer_per_cpu *cpu_buffer)
> > >   orig_head = head_page = cpu_buffer->head_page;
> > >   orig_reader = cpu_buffer->reader_page;
> > >  
> > > - /* Do the reader page first */
> > > - ret = rb_validate_buffer(orig_reader->page, cpu_buffer->cpu, meta);
> > > + /* Do the head page first */
> > > + ret = rb_validate_buffer(head_page, cpu_buffer->cpu, meta, 0, 0);
> > > + if (ret < 0) {
> > > +         pr_info("Ring buffer meta [%d] invalid head page detected\n",
> > > +                 cpu_buffer->cpu);
> > > +         goto skip_rewind;
> > > + }
> > > + ts = head_page->page->time_stamp;
> > > +
> > > + /* Do the reader page - reader must be previous to head. */
> > > + ret = rb_validate_buffer(orig_reader, cpu_buffer->cpu, meta, 0, ts);  
> 
> > If rb_validate_buffer() for the head page fails, we take the goto 
> > skip_rewind
> > path. Since skip_rewind jumps past this orig_reader validation, and the
> > iteration loop explicitly skips orig_reader:
> >     /* Iterate until finding the commit page */
> >     for (i = 0; i < meta->nr_subbufs + 1; i++, rb_inc_page(&head_page)) {
> >         /* The original reader page has already been checked/counted. */
> >         if (head_page == orig_reader)
> >             continue;
> > does this mean the reader page escapes validation entirely, introducing a
> > regression?  
> 
> This is a bug and needs to be fixed. If the head page is invalid it
> shouldn't skip the rewind but instead jump to the "invalid:" label.

Hmm, no it shouldn't jump to invalid, it just needs to be discarded, and
then we should skip rewind. I've changed this patch with the following:

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 7291744d0216..c10cf4ba91d6 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1926,6 +1926,7 @@ static void rb_meta_validate_events(struct 
ring_buffer_per_cpu *cpu_buffer)
        unsigned long entry_bytes = 0;
        unsigned long entries = 0;
        int discarded = 0;
+       bool skip = false;
        int ret;
        u64 ts;
        int i;
@@ -1941,9 +1942,12 @@ static void rb_meta_validate_events(struct 
ring_buffer_per_cpu *cpu_buffer)
        if (ret < 0) {
                pr_info("Ring buffer meta [%d] invalid head page detected\n",
                        cpu_buffer->cpu);
-               goto skip_rewind;
+               /* Don't bother rewinding */
+               skip = true;
+               ts = 0;
+       } else {
+               ts = head_page->page->time_stamp;
        }
-       ts = head_page->page->time_stamp;
 
        /* Do the reader page - reader must be previous to head. */
        ret = rb_validate_buffer(orig_reader, cpu_buffer->cpu, meta, 0, ts);
@@ -1957,6 +1961,9 @@ static void rb_meta_validate_events(struct 
ring_buffer_per_cpu *cpu_buffer)
                ts = orig_reader->page->time_stamp;
        }
 
+       if (skip)
+               goto skip_rewind;
+
        /*
         * Try to rewind the head so that we can read the pages which are 
already
         * read in the previous boot.

-- Steve