* [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic
@ 2021-12-13 7:00 Neeraj Upadhyay
2021-12-13 8:09 ` Woodhouse, David
2021-12-13 13:06 ` Frederic Weisbecker
0 siblings, 2 replies; 5+ messages in thread
From: Neeraj Upadhyay @ 2021-12-13 7:00 UTC (permalink / raw)
To: paulmck, dwmw, josh, rostedt, mathieu.desnoyers, jiangshanlai, joel
Cc: rcu, linux-kernel, urezki, frederic, boqun.feng, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic
2021-12-13 7:00 [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic Neeraj Upadhyay
@ 2021-12-13 8:09 ` Woodhouse, David
2021-12-13 19:32 ` Paul E. McKenney
2021-12-13 13:06 ` Frederic Weisbecker
1 sibling, 1 reply; 5+ messages in thread
From: Woodhouse, David @ 2021-12-13 8:09 UTC (permalink / raw)
To: mathieu.desnoyers, quic_neeraju, paulmck, jiangshanlai, rostedt,
josh, joel
Cc: urezki, frederic, rcu, linux-kernel, boqun.feng
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic
2021-12-13 7:00 [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic Neeraj Upadhyay
2021-12-13 8:09 ` Woodhouse, David
@ 2021-12-13 13:06 ` Frederic Weisbecker
1 sibling, 0 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2021-12-13 13:06 UTC (permalink / raw)
To: Neeraj Upadhyay
Cc: paulmck, dwmw, josh, rostedt, mathieu.desnoyers, jiangshanlai,
joel, rcu, linux-kernel, urezki, boqun.feng
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic
2021-12-13 8:09 ` Woodhouse, David
@ 2021-12-13 19:32 ` Paul E. McKenney
2021-12-13 21:21 ` Frederic Weisbecker
0 siblings, 1 reply; 5+ messages in thread
From: Paul E. McKenney @ 2021-12-13 19:32 UTC (permalink / raw)
To: Woodhouse, David
Cc: mathieu.desnoyers, quic_neeraju, jiangshanlai, rostedt, josh,
joel, urezki, frederic, rcu, linux-kernel, boqun.feng
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic
2021-12-13 19:32 ` Paul E. McKenney
@ 2021-12-13 21:21 ` Frederic Weisbecker
0 siblings, 0 replies; 5+ messages in thread
From: Frederic Weisbecker @ 2021-12-13 21:21 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Woodhouse, David, mathieu.desnoyers, quic_neeraju, jiangshanlai,
rostedt, josh, joel, urezki, rcu, linux-kernel, boqun.feng
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!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-13 21:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-13 7:00 [PATCH] rcu: Make rcu_state.n_online_cpus updates atomic Neeraj Upadhyay
2021-12-13 8:09 ` Woodhouse, David
2021-12-13 19:32 ` Paul E. McKenney
2021-12-13 21:21 ` Frederic Weisbecker
2021-12-13 13:06 ` Frederic Weisbecker
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).