[PATCH v1 1/4] ring-buffer: Skip remote reader page swap when on tail

[Vincent Donnefort]

When the reader page is also the tail, swap should not be permitted or
it would insert the tail behind the head, breaking the integrity of the
ring buffer. While __rb_get_reader_page() does correctly prevent this
situation, the simple_ring_buffer implementation misses it.

Once the reader catches up with the tail, make
simple_ring_buffer_swap_reader_page() return 0. The remote reader now
can expect an unchanged reader. When it is the case, there's nothing
more to read.

Also, when the simple_ring_buffer writer moves the tail to a new page,
it resets the new tail counters and more specifically page::commit. As
moving the tail unblocks page swap, order the page reset before the
tail_page update. This ensures the reader can't swap the reader page
with the new tail and observe a stale commit value.

Cc: stable@xxxxxxxxxxxxxxx
Fixes: 2e67fabd8b77 ("ring-buffer: Introduce ring-buffer remotes")
Fixes: 34e5b958bdad ("tracing: Introduce simple_ring_buffer")
Signed-off-by: Vincent Donnefort <vdonnefort@xxxxxxxxxx>
---
 kernel/trace/ring_buffer.c        |  3 ++-
 kernel/trace/simple_ring_buffer.c | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 1fd28a72e6ee..3c7aa982d43e 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -5848,7 +5848,8 @@ __rb_get_reader_page_from_remote(struct ring_buffer_per_cpu *cpu_buffer)
 
 	new_reader = cpu_buffer->subbuf_ids[cpu_buffer->meta_page->reader.id];
 
-	WARN_ON_ONCE(prev_reader == new_reader);
+	if (prev_reader == new_reader)
+		return NULL;
 
 	prev_head = new_reader;  /* New reader was also the previous head */
 	new_head = prev_head;
diff --git a/kernel/trace/simple_ring_buffer.c b/kernel/trace/simple_ring_buffer.c
index abfaf9bee12e..d5c84af01925 100644
--- a/kernel/trace/simple_ring_buffer.c
+++ b/kernel/trace/simple_ring_buffer.c
@@ -139,6 +139,13 @@ int simple_ring_buffer_swap_reader_page(struct simple_rb_per_cpu *cpu_buffer)
 
 	reader = cpu_buffer->reader_page;
 
+	/*
+	 * The reader is already the tail page. See simple_rb_move_tail() for
+	 * memory ordering.
+	 */
+	if (reader == smp_load_acquire(&cpu_buffer->tail_page))
+		return 0;
+
 	do {
 		/* Run after the writer to find the head */
 		ret = simple_rb_find_head(cpu_buffer);
@@ -196,7 +203,13 @@ static struct simple_buffer_page *simple_rb_move_tail(struct simple_rb_per_cpu *
 	}
 
 	simple_bpage_reset(new_tail);
-	cpu_buffer->tail_page = new_tail;
+
+	/*
+	 * Setting a new tail may unblock the swap in
+	 * simple_ring_buffer_swap_reader_page(). Ensure the reader observes the
+	 * page::commit reset.
+	 */
+	smp_store_release(&cpu_buffer->tail_page, new_tail);
 
 	simple_rb_meta_inc(cpu_buffer->meta->pages_touched, 1);
 
-- 
2.56.0.611.g1030f0b940-goog