Thread (15 messages) flat view 15 messages, 3 authors, 13d ago

Re: [PATCH v8 1/3] tracing: Fix subbuf resize races with trace_pipe_raw readers

From: Vincent Donnefort <hidden>
Date: 2026-08-26 16:24:42
Also in: sashiko-reviews

On Wed, Aug 26, 2026 at 10:37:36AM -0400, Steven Rostedt wrote:
On Wed, 26 Aug 2026 09:59:46 +0000
sashiko-bot@kernel.org wrote:
quoted
quoted
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c  
[ ... ]
quoted
@@ -7183,7 +7218,8 @@ int ring_buffer_read_page(struct trace_buffer *buffer,
 	 */
 	if (read || (len < (size - read)) ||
 	    cpu_buffer->reader_page == cpu_buffer->commit_page ||
-	    rb_is_static(cpu_buffer)) {
+	    rb_is_static(cpu_buffer) ||
+	    data_page->order != reader->order) {
 		struct buffer_data_page *rpage = cpu_buffer->reader_page->page;
 		unsigned int rpos = read;
 		unsigned int pos = 0;  
[Severity: High]
Does this new condition break splice reads when the buffer order changes?

When tracing_buffers_splice_read() allocates a read page with the new order
and calls ring_buffer_read_page() with full=1, it will now enter this manual
copy block because the subbuffer orders no longer match.

However, just a few lines down inside this block in ring_buffer_read_page(),
there is the following check:

        if (full &&
            (!read || (len < (size - read)) ||
             cpu_buffer->reader_page == cpu_buffer->commit_page))
                return -1;

Since the reader page typically hasn't been partially read yet (read == 0),
and full == 1 from splice, this check evaluates to true and immediately
returns -1 instead of copying the data.

Because tracing_buffers_splice_read() loops and ultimately returns -EAGAIN
on failure, will this cause an infinite loop of -EAGAIN for userspace tools
trying to read across buffer resizes?

This is a legitimate concern. But with an easy solution. Perhaps just add:

	if (full &&
	    (!read || (len < (size - read)) ||
-	     cpu_buffer->reader_page == cpu_buffer->commit_page))
+	     cpu_buffer->reader_page == cpu_buffer->commit_page) &&
+	    data_page->order == buffer->subbuf_order)
		return -1;

If the user is changing the buffer size at the same time as reading raw
pages, they get what they deserve! I just don't want to let the kernel go
into an infinite loop.

-- Steve
Sounds good. Do you prefer to get a v9 or you fold this into the existing commit?

-- 
Vincent
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help