Re: [PATCH v19 0/7] ring-buffer: Making persistent ring buffers robust

Re: [PATCH v19 0/7] ring-buffer: Making persistent ring buffers robust

[Masami Hiramatsu]

On Sat, 2 May 2026 18:17:06 -0400
Steven Rostedt <rostedt@kernel.org> wrote:

> On Sat, 2 May 2026 15:23:04 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > Hi Masami,
> > 
> > I applied your patches and enabled your ptracingtest code. I noticed
> > that when there's dropped pages, the trace output is not in order:
> > 
> >  # trace-cmd start -B ptracingtest -e all -v -e '*lock*'
> >  # taskset -c 5 echo c > /proc/sysrq-trigger
> > 
> > On reboot, I ran:
> > 
> >  # trace-cmd show -B ptracingtest > /tmp/trace.out
> > 
> > Then executed the attached perl program:
> > 
> >   # ./read-ts.pl < /tmp/trace.out
> > 
> > And it errors our:
> > 
> >  30.212495 < 30.213534
> >            <...>-1048    [005] d....    30.212495: irq_enable: caller=irqentry_exit+0xf5/0x710 parent=0x0
> > 
> > That is, I think the zero timestamps may be messing with the order.
> > 
> 
> Ah, I think I found the problem. The iterator needs the same logic you
> added for the consuming read:
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 7bfbed0ac90c..90a7fa772fe3 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -6105,12 +6105,14 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
>  	struct ring_buffer_per_cpu *cpu_buffer;
>  	struct ring_buffer_event *event;
>  	int nr_loops = 0;
> +	int max_loops;
>  
>  	if (ts)
>  		*ts = 0;
>  
>  	cpu_buffer = iter->cpu_buffer;
>  	buffer = cpu_buffer->buffer;
> +	max_loops = cpu_buffer->ring_meta ? cpu_buffer->nr_pages : 3;
>  
>  	/*
>  	 * Check if someone performed a consuming read to the buffer
> @@ -6133,7 +6135,7 @@ rb_iter_peek(struct ring_buffer_iter *iter, u64 *ts)
>  	 * the ring buffer with an active write as the consumer is.
>  	 * Do not warn if the three failures is reached.
>  	 */
> -	if (++nr_loops > 3)
> +	if (++nr_loops > max_loops)
>  		return NULL;
>  
>  	if (rb_per_cpu_empty(cpu_buffer))
> 
> 
> I'll test this some more, and make a proper patch.

Ah, indeed. Thanks for fixing!

BTW, shouldn't we unify common logic of those functions?

Thank you,

> 
> -- Steve
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH v19 0/7] ring-buffer: Making persistent ring buffers robust

[Steven Rostedt]

On Thu, 7 May 2026 13:14:16 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > I'll test this some more, and make a proper patch.  
> 
> Ah, indeed. Thanks for fixing!
> 
> BTW, shouldn't we unify common logic of those functions?

Hmm, there's not much common between the two. One is a consuming read and
the other is a non-consuming read that needs to test for a bunch of race
conditions.

If you see something that can be shared, I'm all for it.

-- Steve

Re: [PATCH v19 0/7] ring-buffer: Making persistent ring buffers robust

[Masami Hiramatsu]

On Mon, 11 May 2026 12:29:43 -0400
Steven Rostedt <rostedt@kernel.org> wrote:

> On Thu, 7 May 2026 13:14:16 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > > I'll test this some more, and make a proper patch.  
> > 
> > Ah, indeed. Thanks for fixing!
> > 
> > BTW, shouldn't we unify common logic of those functions?
> 
> Hmm, there's not much common between the two. One is a consuming read and
> the other is a non-consuming read that needs to test for a bunch of race
> conditions.
> 
> If you see something that can be shared, I'm all for it.

Maybe we can introduce a common inline function to calculate
max_loop, or at least replacing "3" with a common macro.

Thank you,


> 
> -- Steve


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>