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 <[email protected]>
---
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
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 <[email protected]>
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 <[email protected]>
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.
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 <[email protected]>
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.
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 <[email protected]>
>
> 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 <[email protected]>
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
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 <[email protected]>
> >
> > 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 <[email protected]>
>
> 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!