On Tue, 26 May 2026 13:41:16 -0400 Steven Rostedt <[email protected]> wrote:
> On Tue, 26 May 2026 14:06:09 +0900 > Masami Hiramatsu (Google) <[email protected]> wrote: > > > > @@ -7204,10 +7209,12 @@ int ring_buffer_read_page(struct trace_buffer > > > *buffer, > > > * 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 > > > + /* > > > + * If there is room at the end of the page to save the > > > * missed events, then record it there. > > > */ > > > - if (buffer->subbuf_size - commit >= sizeof(missed_events)) { > > > + 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); > > > > After this line, we "add" RB_MISSED_EVENTS instead of set. > > In this case, does it clear the RB_MISSED_EVENTS bit because > > it already sets RB_MISSED_EVENTS. > > > > commit += sizeof(missed_events); > > } > > local_add(RB_MISSED_EVENTS, &bpage->commit); > > ^^^ here. > > Perhaps this needs to be commented better. > > The answer to your question is "No". The reason is that this is a *copy* of > the page we are reading. As persistent pages are always assigned to > specific memory, it can never leave the buffer even for the splice system > call. It is always copied to a new page. > > The new page doesn't have these bits set and needs to set them depending on > what was found when reading the page from the buffer. > > Now if this was a normal ring buffer where it did a zero copy from the > buffer itself by swapping pages with the passed in page, if the bit was set > before, then adding would cause a problem. But normal ring buffer pages > never set these bits while in the buffer. They are only set by this function. 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)? ---------- 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. } 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. } ---------- Thanks, > > -- Steve -- Masami Hiramatsu (Google) <[email protected]>
