Re: [PATCH] tracing: Fix nr_subbufs initialization in simple_ring_buffer_init_mm()

[Vincent Donnefort <vdonnefort@google.com>]

On Wed, Mar 18, 2026 at 03:28:04PM -0400, Steven Rostedt wrote:
> On Wed, 18 Mar 2026 17:19:07 +0000
> David Carlier <devnexen@gmail.com> wrote:
> 
> > In simple_ring_buffer_init_mm(), meta->nr_subbufs is assigned from
> > cpu_buffer->nr_pages at line 398, but cpu_buffer was just zeroed by
> > memset() at line 390. The actual page count is only computed later in
> > the loop (lines 410-429), so nr_subbufs is always set to 0.
> 
> Did you use an AI agent to discover this? If so, you need to disclose that.
> (AI agents are typically the only one that uses line numbers to describe
> problems like this)
> 
> > 
> > This field is part of struct trace_buffer_meta (UAPI), exposed to
> > consumers who rely on it for buffer geometry calculations (e.g.,
> > data_len = subbuf_size * nr_subbufs). A value of 0 breaks ring buffer
> > consumption entirely.
> > 
> > Fix by using desc->nr_page_va directly, which holds the correct total
> > page count (reader + ring pages) and is available at this point. This
> > matches the UAPI documentation: "Number of subbufs in the ring-buffer,
> > including the reader."
> 
> As this will likely be pulled into mainline via the ARM64 tree, and they
> are currently the only ones actually using this code, this should go
> through them.
> 
> -- Steve
> 

I don't think it is fixing anything at the moment. It sets nr_subbufs for the
sack of completing the meta_page but this field isn't read by the kernel. It
doesn't need it because the reader is using the ring_buffer_desc.

Nonetheless it's probably worth to fix now, that will be less work if later we
e.g. allow remotes to be mapped by userspace. (Not something I have on my todo
list).

> 
> > 
> > Fixes: 34e5b958bdad ("tracing: Introduce simple_ring_buffer")
> > Signed-off-by: David Carlier <devnexen@gmail.com>
> > ---
> >  kernel/trace/simple_ring_buffer.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/trace/simple_ring_buffer.c b/kernel/trace/simple_ring_buffer.c
> > index 02af2297ae5a..e991a0d8def2 100644
> > --- a/kernel/trace/simple_ring_buffer.c
> > +++ b/kernel/trace/simple_ring_buffer.c
> > @@ -395,7 +395,7 @@ int simple_ring_buffer_init_mm(struct simple_rb_per_cpu *cpu_buffer,
> >  
> >  	memset(cpu_buffer->meta, 0, sizeof(*cpu_buffer->meta));
> >  	cpu_buffer->meta->meta_page_size = PAGE_SIZE;
> > -	cpu_buffer->meta->nr_subbufs = cpu_buffer->nr_pages;
> > +	cpu_buffer->meta->nr_subbufs = desc->nr_page_va;

I would just move this assignment later to make sure cpu_buffer->nr_pages is
aligned with the meta page, instead of relying on the ring_buffer_desc.

> >  
> >  	/* The reader page is not part of the ring initially */
> >  	page = load_page(desc->page_va[0]);
>