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

[Breno Leitao <leitao@debian.org>]

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>