On Wed, 27 May 2026 09:35:07 -0400
Steven Rostedt <[email protected]> wrote:

> On Wed, 27 May 2026 12:47:21 +0900
> Masami Hiramatsu (Google) <[email protected]> wrote:
> 
> > Yeah, for the persistent ring buffer, it does not happen.
> > But there seems RB_MISSED_EVENTS bit can be cleared in
> > "else" path (after applying 1-8 patches)?
> 
> Note, *only* the persistent ring buffer adds RB_MISSED_EVENTS to the pages
> in the write buffer. In the normal buffer, these bits are only set by this
> function. That is, they would not be set from the swap of pages.
> 

Ah, OK. For normal ring buffers, it is only set by reader.

> > 
> > ----------
> >     if (read || (len < (commit - read)) ||
> >         cpu_buffer->reader_page == cpu_buffer->commit_page ||
> >         force_memcpy) { // <-- persistent ring buffer sets force_memcpy = 
> > true.
> > [...]
> >     } else {
> >             /* update the entry counter */
> > [...]
> >             if (!missed_events && rb_data_page_commit(dpage) & 
> > RB_MISSED_EVENTS)
> >                     missed_events = -1;     
> >                     //^-- we check RB_MISSED_EVENTS bit on @dpage->commit 
> > and set missed_events = -1.
> > 
> >             /*
> >              * Use the real_end for the data size,
> >              * This gives us a chance to store the lost events
> >              * on the page.
> >              */
> >             if (reader->real_end)
> >                     local_set(&dpage->commit, reader->real_end);
> >                         // ^- only if @reader->real_end, RB_MISSED_EVENTS 
> > bit is dropped.
> 
> Because this isn't a persistent ring buffer (if it was, as you noted,
> force_memcpy would be true and we wouldn't enter the else path), the
> RB_MISSED_EVENTS bit in the commit would never be set here. It is *only* set
> by the verifier of the persistent ring buffer logic.

OK, I got it.

Thanks for confirmation!

> 
> >     }
> > 
> >     cpu_buffer->lost_events = 0;
> > 
> >     commit = rb_data_page_commit(dpage);
> >     /*
> >      * Set a flag in the commit field if we lost events
> >      */
> >     if (missed_events) {
> >             /*
> >              * If there is room at the end of the page to save the
> >              * missed events, then record it there.
> >              */
> >             if (missed_events > 0 &&
> >                 buffer->subbuf_size - commit >= sizeof(missed_events)) {
> >                     memcpy(&dpage->data[commit], &missed_events,
> >                            sizeof(missed_events));
> >                     local_add(RB_MISSED_STORED, &dpage->commit);
> >                     commit += sizeof(missed_events);
> >             }
> >             local_add(RB_MISSED_EVENTS, &dpage->commit); // <-- 
> > @dpage->commit is updated.
> >     }
> 
> And this is the first place it would get set.
> 
> But yeah, it is very confusing and needs better comments.

Yeah, the relationship between the persistent ring buffer (which is mapped)
and the RB_MISSED_EVENTS is a bit unclear.

Thanks, 


> 
> Thanks,
> 
> -- Steve


-- 
Masami Hiramatsu (Google) <[email protected]>

Reply via email to