All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: Vincent Donnefort <vdonnefort@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	 Masami Hiramatsu <mhiramat@kernel.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	 linux-kernel@vger.kernel.org,
	linux-trace-kernel@vger.kernel.org,
	 Steven Rostedt <srostedt@redhat.com>,
	kernel-team@meta.com, stable@vger.kernel.org
Subject: Re: [PATCH] ring-buffer: Handle a reader taking the new head in rb_handle_head_page()
Date: Thu, 5 Nov 2026 02:32:51 -0800	[thread overview]
Message-ID: <auxXYbBvbupetclu@gmail.com> (raw)
In-Reply-To: <auxJP5fkEQOkvicE@google.com>

On Thu, Nov 05, 2026 at 09:15:43AM +0000, Vincent Donnefort wrote:
> On Mon, Nov 02, 2026 at 06:30:11AM -0800, Breno Leitao wrote:
> > I am occasionally seeing the following  KASAN warning when running
> 
> a KASAN warning?

Sorry. It is a regular WARN from RB_WARN_ON(), on a kernel that happened
to have KASAN enabled. I will fix the wording.

> > I've understood that rb_handle_head_page() moves the head page forward
> 
> Drop "I've understood". 

Ack!

> Besides, I am not sure about returning 1. If the nested writer interrupts
> between rb_head_page_set_head() and rb_head_page_set_normal(), it can also get
> RB_PAGE_MOVED and skipping rb_tail_page_update() means the retry will observe
> the old tail.
> 
> All in all, I wonder if it shouldn't simply be: 
> 
>   case RB_PAGE_MOVED:
>      break;
> 

Good point, that handles both writers. The outer one falls through to
the existing UPDATE clear, and rb_tail_page_update() finds the tail
already moved by the interrupt, as when the cmpxchg returns HEAD. The
nested one moves the tail to next_page, which is right because the outer
writer already did the accounting for that page. And the duplication
goes away.

> Additionally, the comment with "Valid returns" must be amended.

Will do.

My reproducer used a two-page buffer, where this retry is harmless, so
I will re-test v2 with a bigger buffer before sending it.

Thanks for the review,
--breno

      reply	other threads:[~2026-11-05 10:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02 14:30 [PATCH] ring-buffer: Handle a reader taking the new head in rb_handle_head_page() Breno Leitao
2026-11-02 14:52 ` sashiko-bot
2026-11-05  9:15 ` Vincent Donnefort
2026-11-05 10:32   ` Breno Leitao [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=auxXYbBvbupetclu@gmail.com \
    --to=leitao@debian.org \
    --cc=kernel-team@meta.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=srostedt@redhat.com \
    --cc=stable@vger.kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.