[v1] tracing: Fix nr_subbufs initialization in simple_ring_buffer_init_mm()

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

Posted by David Carlier 2 weeks, 4 days ago

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.

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."

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;
 
 	/* The reader page is not part of the ring initially */
 	page = load_page(desc->page_va[0]);
-- 
2.53.0

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

Posted by Steven Rostedt 2 weeks, 4 days ago

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


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

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

Posted by Vincent Donnefort 2 weeks, 4 days ago

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]);
>

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

Posted by Steven Rostedt 2 weeks, 3 days ago

On Thu, 19 Mar 2026 09:26:52 +0000
Vincent Donnefort <vdonnefort@google.com> wrote:

> 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).

Or add a comment to why it was set to zero.

> 
> >   
> > > 
> > > 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.

I was thinking the same thing.

-- Steve

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

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

Posted by David CARLIER 2 weeks, 4 days ago

Indeed for drivers or deep kernel parts, my agent AI helps me find
(easy) bugs that I can fix.
Ok I ll repost it appropriately later. Cheers.

On Wed, 18 Mar 2026 at 19:27, Steven Rostedt <rostedt@goodmis.org> 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
>
>
> >
> > 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;
> >
> >       /* The reader page is not part of the ring initially */
> >       page = load_page(desc->page_va[0]);
>

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

Posted by Marc Zyngier 2 weeks, 4 days ago

On Wed, 18 Mar 2026 19:34:48 +0000,
David CARLIER <devnexen@gmail.com> wrote:
> 
> Indeed for drivers or deep kernel parts, my agent AI helps me find
> (easy) bugs that I can fix.
> Ok I ll repost it appropriately later. Cheers.

Please also consider writing a synthetic commit message. Nobody cares
about line numbers, and we'd be better off with something that explain
in simple terms what is wrong with the current code and how this is
addressed.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

Posted by David Carlier 2 weeks, 4 days ago

nr_subbufs in the ring buffer metadata is always initialized to zero
because it is assigned from cpu_buffer->nr_pages before the page
initialization loop has run. While nr_subbufs is not currently read
by the kernel, it should reflect the actual buffer geometry in the
meta page for correctness.

Move the assignment after the page loop so that cpu_buffer->nr_pages
holds the final count. Bug discover by Opus AI agent.

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..f731f14d0ff7 100644
--- a/kernel/trace/simple_ring_buffer.c
+++ b/kernel/trace/simple_ring_buffer.c
@@ -395,7 +395,6 @@ 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;
 
 	/* The reader page is not part of the ring initially */
 	page = load_page(desc->page_va[0]);
@@ -437,6 +436,7 @@ int simple_ring_buffer_init_mm(struct simple_rb_per_cpu *cpu_buffer,
 		return ret;
 	}
 
+	cpu_buffer->meta->nr_subbufs = cpu_buffer->nr_pages;
 	/* Close the ring */
 	bpage->link.next = &cpu_buffer->tail_page->link;
 	cpu_buffer->tail_page->link.prev = &bpage->link;
-- 
2.53.0