All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: "Paul E. McKenney" <paulmck@kernel.org>
Cc: rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@meta.com,  rostedt@goodmis.org,
	Sunho Park <shpark061104@gmail.com>,
	 syzbot+d4faf7db59e11f6fd1ab@syzkaller.appspotmail.com,
	 Zqiang <qiang.zhang@linux.dev>
Subject: Re: [PATCH 09/19] srcu: Fix WARN_ON() for rcu_segcblist_n_cbs() in cleanup_srcu_struct()
Date: Wed, 30 Sep 2026 17:27:19 -0700	[thread overview]
Message-ID: <ar2o5yyB-o12pAos@google.com> (raw)
In-Reply-To: <20260919003521.3134552-9-paulmck@kernel.org>

On Fri, Sep 18, 2026, Paul E. McKenney wrote:
> From: Sunho Park <shpark061104@gmail.com>
> 
> The WARN_ON() added by commit 78a38cbf6f20 ("srcu: Queue sdp->work
> when the delay timer is successfully deleted") uses rcu_segcblist_n_cbs()
> to detect callbacks that srcu_barrier() failed to wait for. However, the
> ->len counter is decremented only at the end of srcu_invoke_callbacks(),
> after the invoking loop has finished. Since srcu_barrier() can return
> right after the barrier callback is invoked, cleanup_srcu_struct() can see
> a non-zero n_cbs even though the cblist is already physically empty,
> falsely triggering the WARN_ON() together with a still-pending delay_work
> timer.
> 
> This can be triggered as follows, as seen in the syzbot report against
> kvm_destroy_vm() -> cleanup_srcu_struct(&kvm->srcu):
> 
> 1. call_srcu(&kvm->srcu, &bus->rcu, __free_bus) starts SRCU grace
>    period GP1.
> 
> 2. GP1 ends: a delay timer is armed and sdp->work is queued, but
>    sdp->work has not run yet.
> 
> 3. Another call_srcu(&kvm->srcu, &bus->rcu, __free_bus) call invokes
>    srcu_segcblist_advance(), which moves the GP1 callback to
>    RCU_DONE_TAIL, and starts SRCU grace period GP2.
> 
> 4. srcu_barrier() is called. It queues its barrier callback after the
>    GP2 callback and waits for srcu_invoke_callbacks() to invoke it.
> 
> 5. GP2 ends: another delay timer is armed, and the sdp->work queued in
>    step 2 begins to run. Its srcu_invoke_callbacks() call invokes
>    srcu_segcblist_advance() again, moving the GP2 and barrier callbacks
>    to RCU_DONE_TAIL as well, and then invokes all of them. However,
>    rcu_segcblist_add_len(), which updates srcu_cblist's ->len, has not
>    run yet at this point.
> 
> 6. srcu_barrier() returns once its callback has been invoked, and
>    cleanup_srcu_struct() starts running. It finds the delay timer
>    armed in step 5 still pending and srcu_cblist's ->len still
>    non-zero (because step 5 has not reached rcu_segcblist_add_len()
>    yet), and WARN_ON() fires even though every callback has actually
>    been invoked.
> 
> Use rcu_segcblist_empty(), which checks the actual head of the cblist,
> instead of rcu_segcblist_n_cbs(), which checks the racy ->len counter.
> Callbacks that have genuinely not been invoked yet still leave the list
> non-empty, so the WARN_ON() still catches callers that skip srcu_barrier()
> or queue callbacks after it.
> 
> Link: https://lore.kernel.org/rcu/e6350377085ddd85d6ef00d8e9a67bd50c762d3c@linux.dev/T/#t
> Reported-by: syzbot+d4faf7db59e11f6fd1ab@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d4faf7db59e11f6fd1ab
> Fixes: 78a38cbf6f20 ("srcu: Queue sdp->work when the delay timer is successfully deleted")
> Suggested-by: Zqiang <qiang.zhang@linux.dev>
> Signed-off-by: Sunho Park <shpark061104@gmail.com>
> Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> ---

Is this going to be grabbed for 7.3?  It shows up relatively frequently in KVM.
Not so much that it's truly problematic/alarming, but enough that it definitely
stands out.

Tested-by: Sean Christopherson <seanjc@google.com>

  reply	other threads:[~2026-10-01  0:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19  0:35 [PATCH 0/19] Add atomic SRCU Paul E. McKenney
2026-09-19  0:35 ` [PATCH 01/19] srcutiny: Make a Tiny SRCU grace period imply an RCU grace period Paul E. McKenney
2026-09-19  0:35 ` [PATCH 02/19] srcutree: Suppress srcu_advance_state() mutex_lock in atomic Paul E. McKenney
2026-09-19  0:35 ` [PATCH 03/19] srcutree: Suppress to-big transition for atomic SRCU Paul E. McKenney
2026-09-19  0:35 ` [PATCH 04/19] srcutree: Add an atomic Tree SRCU Paul E. McKenney
2026-09-19  0:35 ` [PATCH 05/19] srcutiny: Add an atomic Tiny SRCU Paul E. McKenney
2026-09-19  0:35 ` [PATCH 06/19] rcutorture: Add support for testing synchronize_srcu_atomic() Paul E. McKenney
2026-09-19  0:35 ` [PATCH 07/19] srcutree: Disable preemption across synchronize_srcu_atomic() Paul E. McKenney
2026-09-19  0:35 ` [PATCH 08/19] srcu: Use IRQ_WORK_INIT_HARD for srcu's irq_work Paul E. McKenney
2026-09-19  0:35 ` [PATCH 09/19] srcu: Fix WARN_ON() for rcu_segcblist_n_cbs() in cleanup_srcu_struct() Paul E. McKenney
2026-10-01  0:27   ` Sean Christopherson [this message]
2026-10-01  0:57     ` Paul E. McKenney
2026-10-02  0:27       ` Paul E. McKenney
2026-09-19  0:35 ` [PATCH 10/19] srcutree: Warn if Tiny SRCU readers are preempted Paul E. McKenney
2026-09-19  0:35 ` [PATCH 11/19] srcutree: Explicitly note DEFINE_SRCU() needs for srcu_barrier() Paul E. McKenney
2026-09-19  0:35 ` [PATCH 12/19] rcutorture: Add atomic-SRCU support to torture.sh Paul E. McKenney
2026-09-19  0:35 ` [PATCH 13/19] srcutree: Add reader-free fastpath to synchronize_srcu_atomic() Paul E. McKenney
2026-09-19  0:35 ` [PATCH 14/19] srcutree: Skip callback scheduling for atomic SRCU grace periods Paul E. McKenney
2026-09-19  0:35 ` [PATCH 15/19] srcutree: Remove srcu_barrier() sleep for atomic SRCU Paul E. McKenney
2026-09-19  0:35 ` [PATCH 16/19] srcu: Restrict atomic-SRCU non_block annotation to task context Paul E. McKenney
2026-09-19  0:35 ` [PATCH 17/19] srcutree: Make init_srcu_struct_atomic() prevent transition to big Paul E. McKenney
2026-09-19  0:35 ` [PATCH 18/19] srcutree: Don't transition atomic SRCU to big in srcu_gp_end() Paul E. McKenney
2026-09-19  0:35 ` [PATCH 19/19] srcutree: Skip torture to-big transition for atomic SRCU Paul E. McKenney

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=ar2o5yyB-o12pAos@google.com \
    --to=seanjc@google.com \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=qiang.zhang@linux.dev \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=shpark061104@gmail.com \
    --cc=syzbot+d4faf7db59e11f6fd1ab@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 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.