At CPU offline time, we make sure to flush any pending wakeup for the
nocb_gp kthread linked to the outgoing CPU.
Now we are making sure of that twice:
1) From rcu_report_dead() when the outgoing CPU makes the very last
local cleanups by itself before switching offline.
2) From rcutree_dead_cpu(). Here the offlining CPU has gone and is truly
now offline. Another CPU takes care of post-portem cleaning up and
check if the offline CPU had pending wakeup.
Both ways are fine but we have to choose one or the other because we
don't need to repeat that action. Simply benefit from cache locality
and keep only the first solution.
Signed-off-by: Frederic Weisbecker <[email protected]>
---
kernel/rcu/tree.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 28f1093027b9..a6b448e6e059 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2469,9 +2469,6 @@ int rcutree_dead_cpu(unsigned int cpu)
WRITE_ONCE(rcu_state.n_online_cpus, rcu_state.n_online_cpus - 1);
/* Adjust any no-longer-needed kthreads. */
rcu_boost_kthread_setaffinity(rnp, -1);
- /* Do any needed no-CB deferred wakeups from this CPU. */
- do_nocb_deferred_wakeup(per_cpu_ptr(&rcu_data, cpu));
-
// Stop-machine done, so allow nohz_full to disable tick.
tick_dep_clear(TICK_DEP_BIT_RCU);
return 0;
--
2.25.1
On Wed, May 19, 2021 at 02:09:29AM +0200, Frederic Weisbecker wrote:
> At CPU offline time, we make sure to flush any pending wakeup for the
> nocb_gp kthread linked to the outgoing CPU.
>
> Now we are making sure of that twice:
>
> 1) From rcu_report_dead() when the outgoing CPU makes the very last
> local cleanups by itself before switching offline.
>
> 2) From rcutree_dead_cpu(). Here the offlining CPU has gone and is truly
> now offline. Another CPU takes care of post-portem cleaning up and
> check if the offline CPU had pending wakeup.
>
> Both ways are fine but we have to choose one or the other because we
> don't need to repeat that action. Simply benefit from cache locality
> and keep only the first solution.
But between those two calls, the CPU takes a full pass through the
scheduler and heads into the idle loop. What if there is a call_rcu()
along the way, and if this was the last online CPU in its rcuog kthread's
group of CPUs? Wouldn't that callback be stranded until one of those
CPUs came back online?
Thanx, Paul
> Signed-off-by: Frederic Weisbecker <[email protected]>
> ---
> kernel/rcu/tree.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 28f1093027b9..a6b448e6e059 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2469,9 +2469,6 @@ int rcutree_dead_cpu(unsigned int cpu)
> WRITE_ONCE(rcu_state.n_online_cpus, rcu_state.n_online_cpus - 1);
> /* Adjust any no-longer-needed kthreads. */
> rcu_boost_kthread_setaffinity(rnp, -1);
> - /* Do any needed no-CB deferred wakeups from this CPU. */
> - do_nocb_deferred_wakeup(per_cpu_ptr(&rcu_data, cpu));
> -
> // Stop-machine done, so allow nohz_full to disable tick.
> tick_dep_clear(TICK_DEP_BIT_RCU);
> return 0;
> --
> 2.25.1
>
On Wed, May 19, 2021 at 08:59:05AM -0700, Paul E. McKenney wrote:
> On Wed, May 19, 2021 at 02:09:29AM +0200, Frederic Weisbecker wrote:
> > At CPU offline time, we make sure to flush any pending wakeup for the
> > nocb_gp kthread linked to the outgoing CPU.
> >
> > Now we are making sure of that twice:
> >
> > 1) From rcu_report_dead() when the outgoing CPU makes the very last
> > local cleanups by itself before switching offline.
> >
> > 2) From rcutree_dead_cpu(). Here the offlining CPU has gone and is truly
> > now offline. Another CPU takes care of post-portem cleaning up and
> > check if the offline CPU had pending wakeup.
> >
> > Both ways are fine but we have to choose one or the other because we
> > don't need to repeat that action. Simply benefit from cache locality
> > and keep only the first solution.
>
> But between those two calls, the CPU takes a full pass through the
> scheduler and heads into the idle loop. What if there is a call_rcu()
> along the way, and if this was the last online CPU in its rcuog kthread's
> group of CPUs? Wouldn't that callback be stranded until one of those
> CPUs came back online?
Nope, rcu_report_dead() is called from the idle path right before
arch_cpu_idle_dead(). There should be no call to the scheduler until the
CPU comes back online.
Thanks!
On Thu, May 20, 2021 at 02:25:53AM +0200, Frederic Weisbecker wrote:
> On Wed, May 19, 2021 at 08:59:05AM -0700, Paul E. McKenney wrote:
> > On Wed, May 19, 2021 at 02:09:29AM +0200, Frederic Weisbecker wrote:
> > > At CPU offline time, we make sure to flush any pending wakeup for the
> > > nocb_gp kthread linked to the outgoing CPU.
> > >
> > > Now we are making sure of that twice:
> > >
> > > 1) From rcu_report_dead() when the outgoing CPU makes the very last
> > > local cleanups by itself before switching offline.
> > >
> > > 2) From rcutree_dead_cpu(). Here the offlining CPU has gone and is truly
> > > now offline. Another CPU takes care of post-portem cleaning up and
> > > check if the offline CPU had pending wakeup.
> > >
> > > Both ways are fine but we have to choose one or the other because we
> > > don't need to repeat that action. Simply benefit from cache locality
> > > and keep only the first solution.
> >
> > But between those two calls, the CPU takes a full pass through the
> > scheduler and heads into the idle loop. What if there is a call_rcu()
> > along the way, and if this was the last online CPU in its rcuog kthread's
> > group of CPUs? Wouldn't that callback be stranded until one of those
> > CPUs came back online?
>
> Nope, rcu_report_dead() is called from the idle path right before
> arch_cpu_idle_dead(). There should be no call to the scheduler until the
> CPU comes back online.
You are of course correct. I have pulled this one in as well, thank you!
Thanx, Paul