mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Vincent Donnefort <vdonnefort@google.com>
To: Shuai Liu <847807867@qq.com>
Cc: rostedt@goodmis.org, mhiramat@kernel.org,
	mathieu.desnoyers@efficios.com,
	linux-trace-kernel@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzbot+008851bf625629489e75@syzkaller.appspotmail.com,
	sashiko-bot@kernel.org
Subject: Re: [PATCH v2] ring-buffer: Reset per-CPU buffer state in ring_buffer_subbuf_order_set()
Date: Mon, 9 Nov 2026 08:58:06 +0000	[thread overview]
Message-ID: <avGLHp0ubTUz5D9x@google.com> (raw)
In-Reply-To: <tencent_0A65695A966979B026705C7531C809277707@qq.com>

On Mon, Nov 09, 2026 at 11:06:53AM +0800, Shuai Liu wrote:
> ring_buffer_subbuf_order_set() replaces every page of a ring buffer
> with newly allocated empty pages when the sub-buffer order changes.
> However, it does not reset the per-CPU accounting of the discarded
> pages. The stale counters (pages_read, pages_touched, pages_lost,
> entries, ...) continue to describe pages that no longer exist.
> 
> A full waiter (e.g. poll on per_cpu/*/trace_pipe_raw with
> buffer_percent set) then causes ring_buffer_nr_dirty_pages() to read
> the stale accounting, and the invariant pages_read <= pages_touched -
> pages_lost + 1 can be violated, which trips the WARN_ON_ONCE() there:
> 
>   WARNING: kernel/trace/ring_buffer.c:866 at ring_buffer_nr_dirty_pages
> 
> Since the old pages are gone, the buffer state after swapping in the
> new pages is equivalent to a freshly allocated buffer. Replace the
> manual head/tail/commit page assignment and the rb_head_page_activate()
> call with rb_reset_cpu(), which performs the same page setup and also
> resets all per-CPU accounting.
> 
> Verified on QEMU (x86_64, v7.4-rc2): enable sched_switch events,
> partially consume trace_pipe, set buffer_subbuf_size_kb to 32, then
> poll trace_pipe_raw with buffer_percent=50 while writing events. The
> unpatched kernel triggers the WARN above, while the patched kernel
> does not, and the per-CPU stats report a clean buffer after the order
> change.
> 
> Fixes: 8e7b58c27b3c ("ring-buffer: Just update the subbuffers when changing their allocation order")
> Reported-by: syzbot+008851bf625629489e75@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=008851bf625629489e75
> Link: https://lore.kernel.org/linux-trace-kernel/6af009ed.4dd4714a.f8d02.000d.GAE@google.com/
> Signed-off-by: Shuai Liu <847807867@qq.com>

Please, ratelimit posting and do not use in-reply-to.

> ---
> 
> Changes in v2:
>   - Fix the Fixes tag: the stale per-CPU accounting was introduced by
>     8e7b58c27b3c ("ring-buffer: Just update the subbuffers when changing
>     their allocation order"), not 2808e31ec12e5 which only added the
>     interface stub (Sashiko)
> 
>  kernel/trace/ring_buffer.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 75cd8df23c1b9..2726eb8cb3eeb 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -7548,10 +7548,6 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
>  		list_del_init(&cpu_buffer->new_pages);
>  		cpu_buffer->cnt++;
>  
> -		cpu_buffer->head_page
> -			= list_entry(cpu_buffer->pages, struct buffer_page, list);
> -		cpu_buffer->tail_page = cpu_buffer->commit_page = cpu_buffer->head_page;
> -
>  		cpu_buffer->nr_pages = cpu_buffer->nr_pages_to_update;
>  		cpu_buffer->nr_pages_to_update = 0;
>  
> @@ -7560,7 +7556,13 @@ int ring_buffer_subbuf_order_set(struct trace_buffer *buffer, int order)
>  		cpu_buffer->free_page.data = NULL;
>  		arch_spin_unlock(&cpu_buffer->lock);
>  
> -		rb_head_page_activate(cpu_buffer);
> +		/*
> +		 * The installed pages are freshly allocated and empty.
> +		 * Reset the stale accounting of the discarded pages,
> +		 * which can otherwise trip the WARN_ON_ONCE() in
> +		 * ring_buffer_nr_dirty_pages().

This comment isn't necessary.

> +		 */
> +		rb_reset_cpu(cpu_buffer);
>  
>  		raw_spin_unlock_irqrestore(&cpu_buffer->reader_lock, flags);
>  
> -- 
> 2.43.0
> 

-- 
Vincent

      reply	other threads:[~2026-11-09  8:58 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <sashiko-outbox-204360@kernel.org>
2026-11-09  3:06 ` Shuai Liu
2026-11-09  8:58   ` Vincent Donnefort [this message]

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=avGLHp0ubTUz5D9x@google.com \
    --to=vdonnefort@google.com \
    --cc=847807867@qq.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=sashiko-bot@kernel.org \
    --cc=syzbot+008851bf625629489e75@syzkaller.appspotmail.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

all inboxes | Powered by JetHome®