Linux Trace Kernel
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Vincent Donnefort" <vdonnefort@google.com>
Cc: linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v8 1/3] tracing: Fix subbuf resize races with trace_pipe_raw readers
Date: Wed, 26 Aug 2026 09:59:46 +0000	[thread overview]
Message-ID: <20260826095947.4FF8B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260826094528.3738023-2-vdonnefort@google.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ring_buffer_read_page() incorrectly returns -1 for splice_read requests on subbuf order mismatch
--

commit c4d6ee6f55f23c8ec6da6f812ec3b722b2b7f6b6
Author: Vincent Donnefort <vdonnefort@google.com>

tracing: Fix subbuf resize races with trace_pipe_raw readers

This commit modifies ring_buffer_alloc_read_page() and
ring_buffer_read_page() to handle dynamic resizing of trace ring
buffers, allowing readers to avoid stale size values. It introduces
a manual data copy fallback in ring_buffer_read_page() when the
subbuffer orders do not match.

> 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
[ ... ]
> @@ -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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826094528.3738023-1-vdonnefort@google.com?part=1

  reply	other threads:[~2026-08-26  9:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-26  9:45 [PATCH v8 0/3] ring-buffer: Fixes for subbuf resizing and persistent buffers Vincent Donnefort
2026-08-26  9:45 ` [PATCH v8 1/3] tracing: Fix subbuf resize races with trace_pipe_raw readers Vincent Donnefort
2026-08-26  9:59   ` sashiko-bot [this message]
2026-08-26 14:37     ` Steven Rostedt
2026-08-26 16:24       ` Vincent Donnefort
2026-08-26 18:31         ` Steven Rostedt
2026-08-27  6:31           ` Vincent Donnefort
2026-08-27 13:15             ` Steven Rostedt
2026-08-27 16:21               ` Vincent Donnefort
2026-08-27 19:33                 ` Steven Rostedt
2026-08-28  8:24                   ` Vincent Donnefort
2026-08-28  8:36                     ` Steven Rostedt
2026-08-26  9:45 ` [PATCH v8 2/3] ring-buffer: Cap static ring buffer nr_pages Vincent Donnefort
2026-08-26  9:45 ` [PATCH v8 3/3] ring-buffer: Prevent truncation of nr_pages / nr_subbufs Vincent Donnefort
2026-08-26 10:02   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260826095947.4FF8B1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=vdonnefort@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox