[PATCH v2] ring-buffer: Handle a reader taking the new head in rb_handle_head_page()

[PATCH v2] ring-buffer: Handle a reader taking the new head in rb_handle_head_page()

[Breno Leitao]

The following warning occasionally fires on a KASAN-enabled debug
kernel in Meta's fleet:

  WARNING: kernel/trace/ring_buffer.c:3793 at rb_move_tail+0xf78/0x1050
  Call trace:
   rb_move_tail+0xf78/0x1050 (P)
   __rb_reserve_next+0x648/0xac0
   ring_buffer_lock_reserve+0xd00/0xf00
   trace_buffer_lock_reserve+0x28/0x188
   trace_graph_return+0x220/0x3e8
   ftrace_return_to_handler+0x2bc/0x5a8
   return_to_handler+0x20/0x48
   __arm64_sys_sched_yield+0x18/0x30 (F)

rb_handle_head_page() moves the head page forward in two steps: it marks
the pointer to the old head page UPDATE, then marks the pointer to the
page after it (new_head) HEAD. The second cmpxchg is assumed to return
only HEAD or NORMAL, but it can also return RB_PAGE_MOVED:

  CPU 0 (writer)                        CPU 1 (reader)
  ------------------------------------  --------------------------------
  rb_handle_head_page()
    old head ptr: HEAD -> UPDATE
    new_head = page after old head
    <interrupt: nested write>
      sees UPDATE, marks new_head
      HEAD and moves the tail page
    </interrupt>
                                        rb_get_reader_page()
                                          swaps new_head out for the
                                          reader page
    cmpxchg(new_head|NORMAL ->
            new_head|HEAD) -> MOVED
    RB_WARN_ON()

A nested writer that interrupts the outer one after the second cmpxchg
still sees the old head pointer UPDATE and repeats the second step. If
the reader swaps new_head out just before that cmpxchg, it returns
RB_PAGE_MOVED too.

RB_WARN_ON() permanently disables recording on the whole ring buffer,
and the outer writer returns without clearing its UPDATE flag.

In both cases another writer has already moved the head off the old
head page, as when the cmpxchg returns HEAD. Treat RB_PAGE_MOVED the
same way and fall through: the outer writer still clears its UPDATE
flag, and the caller updates the tail page as usual.

Fixes: 77ae365eca89 ("ring-buffer: make lockless")
Cc: stable@vger.kernel.org
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- Fall through on RB_PAGE_MOVED instead of returning 1, so a nested
  writer still moves the tail page (Vincent)
- Drop the duplicated UPDATE clear (Vincent)
- Document RB_PAGE_MOVED in the "Valid returns" comment (Vincent)
- Commit message improvements
- Link to v1: https://patch.msgid.link/20261102-ring-buffer-head-moved-v1-1-165973790be4@debian.org
---
 kernel/trace/ring_buffer.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1fd28a72e6ee13..44782b71664722 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3783,10 +3783,13 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 	 *            1) We really set it.
 	 *            2) A bunch of interrupts came in and moved
 	 *               the page forward again.
+	 *  MOVED  - another writer set it, then a reader on
+	 *           another CPU swapped it for its reader page.
 	 */
 	switch (ret) {
 	case RB_PAGE_HEAD:
 	case RB_PAGE_NORMAL:
+	case RB_PAGE_MOVED:
 		/* OK */
 		break;
 	default:

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261102-ring-buffer-head-moved-f9caf5da9444

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH v2] ring-buffer: Handle a reader taking the new head in rb_handle_head_page()

[Steven Rostedt]

On Thu, 05 Nov 2026 03:12:42 -0800
Breno Leitao <leitao@debian.org> wrote:

> The following warning occasionally fires on a KASAN-enabled debug
> kernel in Meta's fleet:
> 
>   WARNING: kernel/trace/ring_buffer.c:3793 at rb_move_tail+0xf78/0x1050
>   Call trace:
>    rb_move_tail+0xf78/0x1050 (P)
>    __rb_reserve_next+0x648/0xac0
>    ring_buffer_lock_reserve+0xd00/0xf00
>    trace_buffer_lock_reserve+0x28/0x188
>    trace_graph_return+0x220/0x3e8
>    ftrace_return_to_handler+0x2bc/0x5a8
>    return_to_handler+0x20/0x48
>    __arm64_sys_sched_yield+0x18/0x30 (F)
> 
> rb_handle_head_page() moves the head page forward in two steps: it marks
> the pointer to the old head page UPDATE, then marks the pointer to the
> page after it (new_head) HEAD. The second cmpxchg is assumed to return
> only HEAD or NORMAL, but it can also return RB_PAGE_MOVED:
> 
>   CPU 0 (writer)                        CPU 1 (reader)
>   ------------------------------------  --------------------------------
>   rb_handle_head_page()
>     old head ptr: HEAD -> UPDATE
>     new_head = page after old head
>     <interrupt: nested write>
>       sees UPDATE, marks new_head
>       HEAD and moves the tail page

Wait? The interrupt should never change UPDATE to HEAD.

Something else is broken here. Because only the one that set HEAD to update
is allowed to set it back to NORMAL.

>     </interrupt>
>                                         rb_get_reader_page()
>                                           swaps new_head out for the
>                                           reader page

				This should not happen. The entire point of
				setting the pointer to UPDATE is to prevent
				swaps from happening when any commit is in
				progress.

The rb_handle_head_page() even states this:

	/*
	 * If this was the outer most commit (the one that
	 * changed the original pointer from HEAD to UPDATE),
	 * then it is up to us to reset it to NORMAL.
	 */
	if (type == RB_PAGE_HEAD) {
		ret = rb_head_page_set_normal(cpu_buffer, next_page,
					      tail_page,
					      RB_PAGE_UPDATE);
		if (RB_WARN_ON(cpu_buffer,
			       ret != RB_PAGE_UPDATE))
			return -1;
	}



>     cmpxchg(new_head|NORMAL ->
>             new_head|HEAD) -> MOVED
>     RB_WARN_ON()
> 
> A nested writer that interrupts the outer one after the second cmpxchg
> still sees the old head pointer UPDATE and repeats the second step. If
> the reader swaps new_head out just before that cmpxchg, it returns
> RB_PAGE_MOVED too.
> 
> RB_WARN_ON() permanently disables recording on the whole ring buffer,
> and the outer writer returns without clearing its UPDATE flag.
> 
> In both cases another writer has already moved the head off the old
> head page, as when the cmpxchg returns HEAD. Treat RB_PAGE_MOVED the
> same way and fall through: the outer writer still clears its UPDATE
> flag, and the caller updates the tail page as usual.
> 
> Fixes: 77ae365eca89 ("ring-buffer: make lockless")
> Cc: stable@vger.kernel.org
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v2:
> - Fall through on RB_PAGE_MOVED instead of returning 1, so a nested
>   writer still moves the tail page (Vincent)
> - Drop the duplicated UPDATE clear (Vincent)
> - Document RB_PAGE_MOVED in the "Valid returns" comment (Vincent)
> - Commit message improvements
> - Link to v1: https://patch.msgid.link/20261102-ring-buffer-head-moved-v1-1-165973790be4@debian.org

From your v1 comment:

> A nested writer and a consuming reader can race so that the second
> cmpxchg of a head page move in rb_handle_head_page() returns
> RB_PAGE_MOVED. That trips RB_WARN_ON(), which disables the ring buffer
> and leaves the old head pointer flagged UPDATE. The patch handles the
> return like the RB_PAGE_MOVED case of the first cmpxchg: clear the
> UPDATE flag if this writer set it, and retry.
> 
> Testing, on arm64 KVM guests under virtme-ng (32 vCPUs, KASAN config,
> next-20261030; kernel/trace/ring_buffer.c is identical in v7.4-rc1):
> - Setup: function_graph tracer filtered to "*sched*", buffer_size_kb=7
>   (two pages per CPU), overwrite on, a trace_pipe_raw reader on every
>   CPU plus a trace_pipe reader, and stress-ng --yield/--timer/--futex.

OK, I think I see the issue here. I believe the reader is finding the wrong
head page. Making the buffer really small can make this scenario happen
more often.

The problem is where the reader thinks the "head_page" is. When the reader
swaps out the page, it first looks for the "head_page". The head page
always has flags set in the pointer to it, so before the swap, the reader
will search for the head page.

It starts with the cpu_buffer->head_page pointer which is where it found it
last. It then searches for the head page and when it finds it, it tries the
swap. The swap will only succeed if the pointer is HEAD.

The issue I think you hit was that we have two HEAD pages, when interrupts
come in. That is, we can get a state of:

  --> subbuf --UPDATE--> subbuf --> subbuf --HEAD--> subbuf -->

With the expectation that the reader will never be between those two
pointers and it will always find the UPDATE before it sees the new HEAD. But
since only the reader moves the cpu_buffer->head_page around and the writer
does not, we can have a stale pointer of this:


  --> subbuf --UPDATE--> subbuf --> subbuf --HEAD--> subbuf -->
                                      ^
                                      |
                            cpu_buffer->head_page

And when the reader looks for the new head, it will find this:


  --> subbuf --UPDATE--> subbuf --> subbuf --HEAD--> subbuf -->
                                                       ^
                                                       |
                                             cpu_buffer->head_page

And possibly swap out the head page before it is allowed to.

To prevent this, I believe having the writer move this pointer will keep it
from being stale and ending up between the two.

Can you try this?

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index c64e001c3fc0..f0bb2d23d818 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3721,6 +3721,14 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 		local_sub(rb_page_commit(next_page), &cpu_buffer->entries_bytes);
 		local_inc(&cpu_buffer->pages_lost);
 
+		/*
+		 * To prevent a stale head_page from being between
+		 * the UPDATE and new HEAD pages, set it here to have
+		 * the reader check always find the update before
+		 * finding the HEAD page.
+		 */
+		cpu_buffer->head_page = next_page;
+
 		if (cpu_buffer->ring_meta)
 			rb_update_meta_head(cpu_buffer, next_page);
 		/*

We may need to add memory barriers here, but for now, I think this could
fix it. As it should prevent the search from ever starting in between the
two heads.

-- Steve

Re: [PATCH v2] ring-buffer: Handle a reader taking the new head in rb_handle_head_page()

[Steven Rostedt]

On Thu, 5 Nov 2026 15:57:29 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> We may need to add memory barriers here, but for now, I think this could
> fix it. As it should prevent the search from ever starting in between the
> two heads.

Yeah, I think this will do it. Please test, I'll go write up a formal patch:

-- Steve

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index c64e001c3fc0..0d37f8b529d5 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1494,7 +1494,13 @@ rb_set_head_page(struct ring_buffer_per_cpu *cpu_buffer)
 	if (RB_WARN_ON(cpu_buffer, rb_list_head(list->prev->next) != list))
 		return NULL;
 
-	page = head = cpu_buffer->head_page;
+	/*
+	 * Make sure that the start of the search of the head_page does not
+	 * start between the writer adding the UPDATE and the new HEAD that it
+	 * moves forward.
+	 */
+	/* Sync with the writer moving the HEAD in rb_handle_head_page() */
+	page = head = smp_load_acquire(&cpu_buffer->head_page);
 	/*
 	 * It is possible that the writer moves the header behind
 	 * where we started, and we miss in one loop.
@@ -3721,6 +3727,15 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 		local_sub(rb_page_commit(next_page), &cpu_buffer->entries_bytes);
 		local_inc(&cpu_buffer->pages_lost);
 
+		/*
+		 * To prevent a stale head_page from being between
+		 * the UPDATE and new HEAD pages, set it here to have
+		 * the reader check always find the update before
+		 * finding the HEAD page.
+		 */
+		/* Sync with the reader setting the head page */
+		smp_store_release(&cpu_buffer->head_page, next_page);
+
 		if (cpu_buffer->ring_meta)
 			rb_update_meta_head(cpu_buffer, next_page);
 		/*
-- 
2.53.0

Re: [PATCH v2] ring-buffer: Handle a reader taking the new head in rb_handle_head_page()

[Vincent Donnefort]

On Thu, Nov 05, 2026 at 03:57:29PM -0500, Steven Rostedt wrote:
> On Thu, 05 Nov 2026 03:12:42 -0800
> Breno Leitao <leitao@debian.org> wrote:
> 
> > The following warning occasionally fires on a KASAN-enabled debug
> > kernel in Meta's fleet:
> > 
> >   WARNING: kernel/trace/ring_buffer.c:3793 at rb_move_tail+0xf78/0x1050
> >   Call trace:
> >    rb_move_tail+0xf78/0x1050 (P)
> >    __rb_reserve_next+0x648/0xac0
> >    ring_buffer_lock_reserve+0xd00/0xf00
> >    trace_buffer_lock_reserve+0x28/0x188
> >    trace_graph_return+0x220/0x3e8
> >    ftrace_return_to_handler+0x2bc/0x5a8
> >    return_to_handler+0x20/0x48
> >    __arm64_sys_sched_yield+0x18/0x30 (F)
> > 
> > rb_handle_head_page() moves the head page forward in two steps: it marks
> > the pointer to the old head page UPDATE, then marks the pointer to the
> > page after it (new_head) HEAD. The second cmpxchg is assumed to return
> > only HEAD or NORMAL, but it can also return RB_PAGE_MOVED:
> > 
> >   CPU 0 (writer)                        CPU 1 (reader)
> >   ------------------------------------  --------------------------------
> >   rb_handle_head_page()
> >     old head ptr: HEAD -> UPDATE
> >     new_head = page after old head
> >     <interrupt: nested write>
> >       sees UPDATE, marks new_head
> >       HEAD and moves the tail page
> 
> Wait? The interrupt should never change UPDATE to HEAD.
> 
> Something else is broken here. Because only the one that set HEAD to update
> is allowed to set it back to NORMAL.
> 
> >     </interrupt>
> >                                         rb_get_reader_page()
> >                                           swaps new_head out for the
> >                                           reader page
> 
> 				This should not happen. The entire point of
> 				setting the pointer to UPDATE is to prevent
> 				swaps from happening when any commit is in
> 				progress.
> 
> The rb_handle_head_page() even states this:
> 
> 	/*
> 	 * If this was the outer most commit (the one that
> 	 * changed the original pointer from HEAD to UPDATE),
> 	 * then it is up to us to reset it to NORMAL.
> 	 */
> 	if (type == RB_PAGE_HEAD) {
> 		ret = rb_head_page_set_normal(cpu_buffer, next_page,
> 					      tail_page,
> 					      RB_PAGE_UPDATE);
> 		if (RB_WARN_ON(cpu_buffer,
> 			       ret != RB_PAGE_UPDATE))
> 			return -1;
> 	}
> 
> 
> 
> >     cmpxchg(new_head|NORMAL ->
> >             new_head|HEAD) -> MOVED
> >     RB_WARN_ON()
> > 
> > A nested writer that interrupts the outer one after the second cmpxchg
> > still sees the old head pointer UPDATE and repeats the second step. If
> > the reader swaps new_head out just before that cmpxchg, it returns
> > RB_PAGE_MOVED too.
> > 
> > RB_WARN_ON() permanently disables recording on the whole ring buffer,
> > and the outer writer returns without clearing its UPDATE flag.
> > 
> > In both cases another writer has already moved the head off the old
> > head page, as when the cmpxchg returns HEAD. Treat RB_PAGE_MOVED the
> > same way and fall through: the outer writer still clears its UPDATE
> > flag, and the caller updates the tail page as usual.
> > 
> > Fixes: 77ae365eca89 ("ring-buffer: make lockless")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> > Changes in v2:
> > - Fall through on RB_PAGE_MOVED instead of returning 1, so a nested
> >   writer still moves the tail page (Vincent)
> > - Drop the duplicated UPDATE clear (Vincent)
> > - Document RB_PAGE_MOVED in the "Valid returns" comment (Vincent)
> > - Commit message improvements
> > - Link to v1: https://patch.msgid.link/20261102-ring-buffer-head-moved-v1-1-165973790be4@debian.org
> 
> From your v1 comment:
> 
> > A nested writer and a consuming reader can race so that the second
> > cmpxchg of a head page move in rb_handle_head_page() returns
> > RB_PAGE_MOVED. That trips RB_WARN_ON(), which disables the ring buffer
> > and leaves the old head pointer flagged UPDATE. The patch handles the
> > return like the RB_PAGE_MOVED case of the first cmpxchg: clear the
> > UPDATE flag if this writer set it, and retry.
> > 
> > Testing, on arm64 KVM guests under virtme-ng (32 vCPUs, KASAN config,
> > next-20261030; kernel/trace/ring_buffer.c is identical in v7.4-rc1):
> > - Setup: function_graph tracer filtered to "*sched*", buffer_size_kb=7
> >   (two pages per CPU), overwrite on, a trace_pipe_raw reader on every
> >   CPU plus a trace_pipe reader, and stress-ng --yield/--timer/--futex.
> 
> OK, I think I see the issue here. I believe the reader is finding the wrong
> head page. Making the buffer really small can make this scenario happen
> more often.
> 
> The problem is where the reader thinks the "head_page" is. When the reader
> swaps out the page, it first looks for the "head_page". The head page
> always has flags set in the pointer to it, so before the swap, the reader
> will search for the head page.
> 
> It starts with the cpu_buffer->head_page pointer which is where it found it
> last. It then searches for the head page and when it finds it, it tries the
> swap. The swap will only succeed if the pointer is HEAD.
> 
> The issue I think you hit was that we have two HEAD pages, when interrupts
> come in. That is, we can get a state of:
> 
>   --> subbuf --UPDATE--> subbuf --> subbuf --HEAD--> subbuf -->

How we can end-up in this situation? The cmpxch will only swap HEAD for UPDATE. 

> 
> With the expectation that the reader will never be between those two
> pointers and it will always find the UPDATE before it sees the new HEAD. But
> since only the reader moves the cpu_buffer->head_page around and the writer
> does not, we can have a stale pointer of this:
> 
> 
>   --> subbuf --UPDATE--> subbuf --> subbuf --HEAD--> subbuf -->
>                                       ^
>                                       |
>                             cpu_buffer->head_page
> 
> And when the reader looks for the new head, it will find this:
> 
> 
>   --> subbuf --UPDATE--> subbuf --> subbuf --HEAD--> subbuf -->
>                                                        ^
>                                                        |
>                                              cpu_buffer->head_page
> 
> And possibly swap out the head page before it is allowed to.
> 
> To prevent this, I believe having the writer move this pointer will keep it
> from being stale and ending up between the two.
> 
> Can you try this?
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index c64e001c3fc0..f0bb2d23d818 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -3721,6 +3721,14 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
>  		local_sub(rb_page_commit(next_page), &cpu_buffer->entries_bytes);
>  		local_inc(&cpu_buffer->pages_lost);
>  
> +		/*
> +		 * To prevent a stale head_page from being between
> +		 * the UPDATE and new HEAD pages, set it here to have
> +		 * the reader check always find the update before
> +		 * finding the HEAD page.
> +		 */
> +		cpu_buffer->head_page = next_page;
> +
>  		if (cpu_buffer->ring_meta)
>  			rb_update_meta_head(cpu_buffer, next_page);
>  		/*
> 
> We may need to add memory barriers here, but for now, I think this could
> fix it. As it should prevent the search from ever starting in between the
> two heads.
> 
> -- Steve
> 

-- 
Vincent