Re: [PATCH v21 8/9] ring-buffer: Show persistent buffer dropped events in trace file

[Google]

On Wed, 27 May 2026 09:35:07 -0400
Steven Rostedt <rostedt@xxxxxxxxxx> wrote:

> On Wed, 27 May 2026 12:47:21 +0900
> Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> 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) <mhiramat@xxxxxxxxxx>