[PATCH] rcu: Make rcu_state.n_online_cpus updates atomic

[PATCH] rcu: Make rcu_state.n_online_cpus updates atomic

[Neeraj Upadhyay]

To support onlining multiple CPUs concurrently,
change rcu_state.n_online_cpus updates to be atomic.
Note, it's ok for rcu_blocking_is_gp() to do a
atomic_read(&rcu_state.n_online_cpus), as the
value of .n_online_cpus switches from 1->2, in
rcutree_prepare_cpu(), which runs before the new
CPU comes online. Similarly 2->1 transition happens
from rcutree_dead_cpu(), which executes after the
CPU is offlined, and runs on the last online CPU.

Signed-off-by: Neeraj Upadhyay <quic_neeraju@quicinc.com>
---
 kernel/rcu/tree.c | 6 +++---
 kernel/rcu/tree.h | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 3da7826865f7..c1db01c4ea39 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2460,7 +2460,7 @@ int rcutree_dead_cpu(unsigned int cpu)
 	if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
 		return 0;
 
-	WRITE_ONCE(rcu_state.n_online_cpus, rcu_state.n_online_cpus - 1);
+	atomic_dec(&rcu_state.n_online_cpus);
 	/* Adjust any no-longer-needed kthreads. */
 	rcu_boost_kthread_setaffinity(rnp, -1);
 	// Stop-machine done, so allow nohz_full to disable tick.
@@ -3740,7 +3740,7 @@ static int rcu_blocking_is_gp(void)
 	 * in the code, without the need for additional memory barriers.
 	 * Those memory barriers are provided by CPU-hotplug code.
 	 */
-	ret = READ_ONCE(rcu_state.n_online_cpus) <= 1;
+	ret = atomic_read(&rcu_state.n_online_cpus) <= 1;
 	preempt_enable();
 	return ret;
 }
@@ -4199,7 +4199,7 @@ int rcutree_prepare_cpu(unsigned int cpu)
 	raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 	rcu_spawn_one_boost_kthread(rnp);
 	rcu_spawn_cpu_nocb_kthread(cpu);
-	WRITE_ONCE(rcu_state.n_online_cpus, rcu_state.n_online_cpus + 1);
+	atomic_inc(&rcu_state.n_online_cpus);
 
 	return 0;
 }
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 4b889081f4f4..f1017e7e1e9e 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -299,7 +299,7 @@ struct rcu_state {
 						/* Hierarchy levels (+1 to */
 						/*  shut bogus gcc warning) */
 	int ncpus;				/* # CPUs seen so far. */
-	int n_online_cpus;			/* # CPUs online for RCU. */
+	atomic_t n_online_cpus;			/* # CPUs online for RCU. */
 
 	/* The following fields are guarded by the root rcu_node's lock. */
 
-- 
2.17.1

Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic

[Woodhouse, David]

On Mon, 2021-12-13 at 12:30 +0530, Neeraj Upadhyay wrote:
> To support onlining multiple CPUs concurrently,
> change rcu_state.n_online_cpus updates to be atomic.
> Note, it's ok for rcu_blocking_is_gp() to do a
> atomic_read(&rcu_state.n_online_cpus), as the
> value of .n_online_cpus switches from 1->2, in
> rcutree_prepare_cpu(), which runs before the new
> CPU comes online. Similarly 2->1 transition happens
> from rcutree_dead_cpu(), which executes after the
> CPU is offlined, and runs on the last online CPU.
> 
> Signed-off-by: Neeraj Upadhyay <quic_neeraju@quicinc.com>

In my parallel-bringup series, the prepare stages are still being
executed in series on the BSP, so I don't think this patch is needed
yet. I'm not sure we'd ever end up with the prepare stages being done
in parallel — the most I see us doing is onlining a single *batch* of
CPUs at a time, much like bringup_nonboot_cpus() does.

But this patch certainly doesn't *hurt*.

Acked-by: David Woodhouse <dwmw@amazon.co.uk>





Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.

Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic

[Paul E. McKenney]

On Mon, Dec 13, 2021 at 08:09:22AM +0000, Woodhouse, David wrote:
> On Mon, 2021-12-13 at 12:30 +0530, Neeraj Upadhyay wrote:
> > To support onlining multiple CPUs concurrently,
> > change rcu_state.n_online_cpus updates to be atomic.
> > Note, it's ok for rcu_blocking_is_gp() to do a
> > atomic_read(&rcu_state.n_online_cpus), as the
> > value of .n_online_cpus switches from 1->2, in
> > rcutree_prepare_cpu(), which runs before the new
> > CPU comes online. Similarly 2->1 transition happens
> > from rcutree_dead_cpu(), which executes after the
> > CPU is offlined, and runs on the last online CPU.
> > 
> > Signed-off-by: Neeraj Upadhyay <quic_neeraju@quicinc.com>
> 
> In my parallel-bringup series, the prepare stages are still being
> executed in series on the BSP, so I don't think this patch is needed
> yet. I'm not sure we'd ever end up with the prepare stages being done
> in parallel — the most I see us doing is onlining a single *batch* of
> CPUs at a time, much like bringup_nonboot_cpus() does.
> 
> But this patch certainly doesn't *hurt*.
> 
> Acked-by: David Woodhouse <dwmw@amazon.co.uk>

Queued for further review and testing.

To Frederic's point, this won't go to mainline unless it is actually
needed, but it will at least be pulled into a branch in -rcu marked with
a tag for future reference.

							Thanx, Paul

Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic

[Frederic Weisbecker]

On Mon, Dec 13, 2021 at 11:32:56AM -0800, Paul E. McKenney wrote:
> On Mon, Dec 13, 2021 at 08:09:22AM +0000, Woodhouse, David wrote:
> > On Mon, 2021-12-13 at 12:30 +0530, Neeraj Upadhyay wrote:
> > > To support onlining multiple CPUs concurrently,
> > > change rcu_state.n_online_cpus updates to be atomic.
> > > Note, it's ok for rcu_blocking_is_gp() to do a
> > > atomic_read(&rcu_state.n_online_cpus), as the
> > > value of .n_online_cpus switches from 1->2, in
> > > rcutree_prepare_cpu(), which runs before the new
> > > CPU comes online. Similarly 2->1 transition happens
> > > from rcutree_dead_cpu(), which executes after the
> > > CPU is offlined, and runs on the last online CPU.
> > > 
> > > Signed-off-by: Neeraj Upadhyay <quic_neeraju@quicinc.com>
> > 
> > In my parallel-bringup series, the prepare stages are still being
> > executed in series on the BSP, so I don't think this patch is needed
> > yet. I'm not sure we'd ever end up with the prepare stages being done
> > in parallel — the most I see us doing is onlining a single *batch* of
> > CPUs at a time, much like bringup_nonboot_cpus() does.
> > 
> > But this patch certainly doesn't *hurt*.
> > 
> > Acked-by: David Woodhouse <dwmw@amazon.co.uk>
> 
> Queued for further review and testing.
> 
> To Frederic's point, this won't go to mainline unless it is actually
> needed, but it will at least be pulled into a branch in -rcu marked with
> a tag for future reference.

Ok, sounds reasonable then.

Thanks!

Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic

[Frederic Weisbecker]

On Mon, Dec 13, 2021 at 12:30:59PM +0530, Neeraj Upadhyay wrote:
> To support onlining multiple CPUs concurrently,
> change rcu_state.n_online_cpus updates to be atomic.
> Note, it's ok for rcu_blocking_is_gp() to do a
> atomic_read(&rcu_state.n_online_cpus), as the
> value of .n_online_cpus switches from 1->2, in
> rcutree_prepare_cpu(), which runs before the new
> CPU comes online. Similarly 2->1 transition happens
> from rcutree_dead_cpu(), which executes after the
> CPU is offlined, and runs on the last online CPU.
> 
> Signed-off-by: Neeraj Upadhyay <quic_neeraju@quicinc.com>

That's a step but I can imagine much more complications to handle while looking
at rcutree_dead_cpu() VS rcutree_dead_cpu() (or other hotplug operations)
inside the same rnp calling rcu_boost_kthread_setaffinity() concurrently
or more generally rcu_boost_kthread_setaffinity() against concurrent onlining/offlining.

This function fetches the online CPUs to decide the affinity of boosting.
This can go quite wrong if CPUs can be concurrently onlined/offlined.

And I don't know how such problems are going to be solved in the future
but some new CPU hotplug concurrency primitives will be needed...

That's one more reason why I think it is a bit early to handle this wide problem...

Thanks.