Re: call_rcu data race patch

From: Paul E. McKenney
Date: Mon Sep 27 2021 - 12:10:49 EST


On Mon, Sep 27, 2021 at 05:38:42PM +0200, Guillaume Morin wrote:
> On 22 Sep 12:24, Paul E. McKenney wrote:
> > On Wed, Sep 22, 2021 at 09:14:07PM +0200, Guillaume Morin wrote:
> > > I am little afraid of jinxing it :) but so far so good. I have the a
> > > patched kernel running on a few machines (including my most "reliable
> > > crasher") and they've been stable so far.
> > >
> > > It's definitely too early to declare victory though. I will keep you
> > > posted.
> >
> > Here is hoping! ;-)
>
> Things are still stable. So I am pretty optimistic. How are you planning
> to proceeed?

Very good! Would you be willing to give me your Tested-by?

> The first patch is already in your rcu tree and my gut feeling is that
> it is the one that fixes the issue but you're the expert here... Though
> I think it should be probably fast tracked and marked for stable?
>
> Are you planning on committing the 2nd patch to your tree?

This is the second patch, correct? (Too many patches!)

If so, I add your Tested-by and fill out the commit log. It would be
slated for the v5.17 merge window by default, that is, not the upcoming
merge window but the one after that. Please let me know if you need
it sooner.

Thanx, Paul

------------------------------------------------------------------------

commit 1a792b59071b697defd4ccdc8b951cce49de9d2f
Author: Paul E. McKenney <paulmck@xxxxxxxxxx>
Date: Fri Sep 17 15:04:48 2021 -0700

EXP rcu: Tighten rcu_advance_cbs_nowake() checks

This is an experimental shot-in-the-dark debugging patch.

Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 6a1e9d3374db..6d692a591f66 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1590,10 +1590,14 @@ static void __maybe_unused rcu_advance_cbs_nowake(struct rcu_node *rnp,
struct rcu_data *rdp)
{
rcu_lockdep_assert_cblist_protected(rdp);
- if (!rcu_seq_state(rcu_seq_current(&rnp->gp_seq)) ||
+ // Don't do anything unless the current grace period is guaranteed
+ // not to end. This means a grace period in progress and at least
+ // one holdout CPU.
+ if (!rcu_seq_state(rcu_seq_current(&rnp->gp_seq)) || !READ_ONCE(rnp->qsmask) ||
!raw_spin_trylock_rcu_node(rnp))
return;
- WARN_ON_ONCE(rcu_advance_cbs(rnp, rdp));
+ if (rcu_seq_state(rcu_seq_current(&rnp->gp_seq)) && READ_ONCE(rnp->qsmask))
+ WARN_ON_ONCE(rcu_advance_cbs(rnp, rdp));
raw_spin_unlock_rcu_node(rnp);
}