Re: [PATCH v20 08/10] ring-buffer: Have dropped subbuffers be persistent across reboots

[Google]

On Wed, 20 May 2026 14:49:46 -0400
Steven Rostedt <rostedt@xxxxxxxxxx> wrote:

> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
> 
> When the persistent ring buffer detects a corrupted subbuffer, it will
> zero its size and report dropped pages in the dmesg, then it continues
> normally.
> 
> But if a reboot happens without clearing or restarting tracing on the
> persistent ring buffer, the next boot will show no pages are dropped.
> 
> If the persistent ring buffer is still the same, then it should still
> report dropped pages so the user knows that the buffer has missing events.
> 
> Add the RB_MISSED_EVENTS flag to the commit value of the subbuffer so that
> the next boot will still show that pages were dropped.
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>

Thanks!

> Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> ---
>  kernel/trace/ring_buffer.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index bda53a2d2159..ae5c645b59c9 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1915,7 +1915,7 @@ static int __rb_validate_buffer(struct buffer_page *bpage, int cpu,
>  	 * Even after clearing these bits, a commit value greater than the
>  	 * subbuf_size is considered invalid.
>  	 */
> -	tail = rb_data_page_size(dpage);
> +	tail = rb_data_page_commit(dpage);
>  	if (tail <= meta->subbuf_size - BUF_PAGE_HDR_SIZE)
>  		ret = rb_read_data_buffer(dpage, tail, cpu, &ts, &delta);
>  	else
> @@ -1929,7 +1929,7 @@ static int __rb_validate_buffer(struct buffer_page *bpage, int cpu,
>  	 */
>  	if (ret < 0 || (prev_ts && prev_ts > ts) || (next_ts && ts > next_ts)) {
>  		local_set(&bpage->entries, 0);
> -		local_set(&dpage->commit, 0);
> +		local_set(&dpage->commit, RB_MISSED_EVENTS);
>  		dpage->time_stamp = prev_ts ? prev_ts : next_ts;
>  		ret = -1;
>  	} else {
> @@ -3444,7 +3444,7 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
>  	 * is a mb(), which will synchronize with the rmb here.
>  	 * (see rb_tail_page_update() and __rb_reserve_next())
>  	 */
> -	commit = rb_page_commit(iter_head_page);
> +	commit = rb_page_size(iter_head_page);
>  	smp_rmb();
>  
>  	/* An event needs to be at least 8 bytes in size */
> @@ -3473,7 +3473,7 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
>  
>  	/* Make sure the page didn't change since we read this */
>  	if (iter->page_stamp != iter_head_page->page->time_stamp ||
> -	    commit > rb_page_commit(iter_head_page))
> +	    commit > rb_page_size(iter_head_page))
>  		goto reset;
>  
>  	iter->next_event = iter->head + length;
> @@ -5643,7 +5643,7 @@ int ring_buffer_iter_empty(struct ring_buffer_iter *iter)
>  	 * (see rb_tail_page_update())
>  	 */
>  	smp_rmb();
> -	commit = rb_page_commit(commit_page);
> +	commit = rb_page_size(commit_page);
>  	/* We want to make sure that the commit page doesn't change */
>  	smp_rmb();
>  
> @@ -5836,6 +5836,7 @@ __rb_get_reader_page(struct ring_buffer_per_cpu *cpu_buffer)
>  	 */
>  	local_set(&cpu_buffer->reader_page->write, 0);
>  	local_set(&cpu_buffer->reader_page->entries, 0);
> +	rb_init_data_page(cpu_buffer->reader_page->page);
>  	cpu_buffer->reader_page->real_end = 0;
>  
>   spin:
> -- 
> 2.53.0
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>