[PATCH] ring-buffer: Let the reader spin on head page update

[PATCH] ring-buffer: Let the reader spin on head page update

[Vincent Donnefort]

When a writer hits the head page, it needs to move it. The update
happens in two steps: first it swaps the HEAD link for UPDATE and then
swaps the following from NORMAL to HEAD. However, if a stale
cpu_buffer->head_page sits past the UPDATE link, rb_set_head_page() will
find HEAD directly and allow swapping.

This is where things gets hairy. If a outer update is interrupted before
setting HEAD, this nested writer finishes the update first. Another CPU
running trace_pipe can swap that newly installed head page with its
reader page. Now back to the outer writer. It resumes and tries to swap
the following link from NORMAL to HEAD. It sees PAGE_MOVED which is
fatal for the ring buffer.

To prevent the race, mark the newly moved head links with HEAD_UPDATE
(HEAD | UPDATE) and only convert the final link to PAGE_HEAD when the
outer writer exists rb_handle_head_page(). This effectively keeps the
reader spinning.

Fixes: 77ae365eca89 ("ring-buffer: make lockless")
Reported-by: Breno Leitao <leitao@debian.org>
Closes: https://lore.kernel.org/all/20261105-ring-buffer-head-moved-v2-1-05da40cebccc@debian.org/
Cc: stable@vger.kernel.org
Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
---
 kernel/trace/ring_buffer.c | 44 +++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1fd28a72e6ee..73f31e6719f9 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1306,6 +1306,7 @@ EXPORT_SYMBOL_GPL(ring_buffer_normalize_time_stamp);
 #define RB_PAGE_NORMAL		0UL
 #define RB_PAGE_HEAD		1UL
 #define RB_PAGE_UPDATE		2UL
+#define RB_PAGE_HEAD_UPDATE	(RB_PAGE_HEAD | RB_PAGE_UPDATE)
 
 
 #define RB_FLAG_MASK		3UL
@@ -1455,6 +1456,15 @@ static int rb_head_page_set_head(struct ring_buffer_per_cpu *cpu_buffer,
 				old_flag, RB_PAGE_HEAD);
 }
 
+static int rb_head_page_set_head_update(struct ring_buffer_per_cpu *cpu_buffer,
+					struct buffer_page *head,
+					struct buffer_page *prev,
+					int old_flag)
+{
+	return rb_head_page_set(cpu_buffer, head, prev,
+				old_flag, RB_PAGE_HEAD_UPDATE);
+}
+
 static int rb_head_page_set_normal(struct ring_buffer_per_cpu *cpu_buffer,
 				   struct buffer_page *head,
 				   struct buffer_page *prev,
@@ -3699,6 +3709,11 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 	type = rb_head_page_set_update(cpu_buffer, next_page, tail_page,
 				       RB_PAGE_HEAD);
 
+	/* We have interrupted an update */
+	if (type == RB_PAGE_HEAD_UPDATE)
+		type = rb_head_page_set_update(cpu_buffer, next_page, tail_page,
+					       RB_PAGE_HEAD_UPDATE);
+
 	/*
 	 * type can be one of four:
 	 *  NORMAL - an interrupt already moved it for us
@@ -3711,6 +3726,7 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 	 */
 
 	switch (type) {
+	case RB_PAGE_HEAD_UPDATE:
 	case RB_PAGE_HEAD:
 		/*
 		 * We changed the head to UPDATE, thus
@@ -3773,8 +3789,8 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 	new_head = next_page;
 	rb_inc_page(&new_head);
 
-	ret = rb_head_page_set_head(cpu_buffer, new_head, next_page,
-				    RB_PAGE_NORMAL);
+	ret = rb_head_page_set_head_update(cpu_buffer, new_head, next_page,
+					   RB_PAGE_NORMAL);
 
 	/*
 	 * Valid returns are:
@@ -3785,7 +3801,7 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 	 *               the page forward again.
 	 */
 	switch (ret) {
-	case RB_PAGE_HEAD:
+	case RB_PAGE_HEAD_UPDATE:
 	case RB_PAGE_NORMAL:
 		/* OK */
 		break;
@@ -3816,7 +3832,7 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 		    buffer_tail_page != next_page)
 			rb_head_page_set_normal(cpu_buffer, new_head,
 						next_page,
-						RB_PAGE_HEAD);
+						RB_PAGE_HEAD_UPDATE);
 	}
 
 	/*
@@ -3824,7 +3840,7 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 	 * changed the original pointer from HEAD to UPDATE),
 	 * then it is up to us to reset it to NORMAL.
 	 */
-	if (type == RB_PAGE_HEAD) {
+	if (type == RB_PAGE_HEAD || type == RB_PAGE_HEAD_UPDATE) {
 		ret = rb_head_page_set_normal(cpu_buffer, next_page,
 					      tail_page,
 					      RB_PAGE_UPDATE);
@@ -3833,6 +3849,24 @@ rb_handle_head_page(struct ring_buffer_per_cpu *cpu_buffer,
 			return -1;
 	}
 
+	if (type == RB_PAGE_HEAD) {
+		struct buffer_page *prev_page;
+
+		do {
+			/* Interrupts might have moved the head further  */
+			while (!rb_is_head_page(new_head, new_head->list.prev))
+				rb_inc_page(&new_head);
+
+			prev_page = new_head;
+			rb_dec_page(&prev_page);
+
+			/* Try to release the reader */
+			ret = rb_head_page_set_head(cpu_buffer, new_head, prev_page,
+						    RB_PAGE_HEAD_UPDATE);
+
+		} while (ret != (RB_PAGE_HEAD_UPDATE));
+	}
+
 	return 0;
 }
 

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.56.0.542.g3ef89b7020-goog