On Fri, Feb 11, 2022 at 04:11:48PM +0100, Frederic Weisbecker wrote:
> On Fri, Feb 11, 2022 at 03:57:57PM +0100, Frederic Weisbecker wrote:
> > On Fri, Feb 04, 2022 at 03:07:59PM -0800, Paul E. McKenney wrote:
> > > From: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > >
> > > As we handle parallel CPU bringup, we will need to take care to avoid
> > > spawning multiple boost threads, or race conditions when setting their
> > > affinity. Spotted by Paul McKenney.
> > >
> > > Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
> > > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> >
> > Reviewed-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
> >
> > Speaking of, we have:
> >
> > rcu_init()
> > for_each_online_cpu(cpu) // should be boot CPU only at this stage ?
> > rcutree_prepare_cpu(cpu)
> > rcu_spawn_one_boost_kthread(cpu)
> >
> >
> > early_initcall()
> > rcu_spawn_gp_kthread()
> > rcu_spawn_boost_kthreads()
> > rcu_for_each_leaf_node(rnp)
> > rcu_rnp_online_cpus(rnp) // as above, only boot CPU at this stage.
> > rcu_spawn_one_boost_kthread(cpu)
> >
> > cpu_up()
> > rcutree_prepare_cpu(cpu)
> > rcu_spawn_one_boost_kthread(cpu)
> >
> >
> > My guess is that we could remove rcu_spawn_boost_kthreads() and simplify
> > rcu_init(). Something like this (untested yet):
If you also add a WARN_ON() for more than one CPU being online at
rcu_init() time, I am good with this approach. The main danger that
the pre-rcu_init() portion of the boot time becomes the long straw in
the eternal quest to make systems boot faster, but it is not hard to
put it back. Hence the WARN_ON() to make it clear that adjustment
is needed. ;-)
Thanx, Paul
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 86eec6a0f1a1..da8ac2b6f8cc 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -4526,7 +4526,6 @@ static int __init rcu_spawn_gp_kthread(void)
> > raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> > wake_up_process(t);
> > rcu_spawn_nocb_kthreads();
> > - rcu_spawn_boost_kthreads();
> > rcu_spawn_core_kthreads();
> > return 0;
> > }
> > @@ -4813,7 +4812,7 @@ static void __init kfree_rcu_batch_init(void)
> >
> > void __init rcu_init(void)
> > {
> > - int cpu;
> > + int cpu = smp_processor_id();
> >
> > rcu_early_boot_tests();
> >
> > @@ -4833,11 +4832,10 @@ void __init rcu_init(void)
> > * or the scheduler are operational.
> > */
> > pm_notifier(rcu_pm_notify, 0);
> > - for_each_online_cpu(cpu) {
> > - rcutree_prepare_cpu(cpu);
> > - rcu_cpu_starting(cpu);
> > - rcutree_online_cpu(cpu);
> > - }
> > +
> > + rcutree_prepare_cpu(cpu);
> > + rcu_cpu_starting(cpu);
> > + rcutree_online_cpu(cpu);
> >
> > /* Create workqueue for Tree SRCU and for expedited GPs. */
> > rcu_gp_wq = alloc_workqueue("rcu_gp", WQ_MEM_RECLAIM, 0);
> > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> > index 6082dd23408f..90925a589774 100644
> > --- a/kernel/rcu/tree_plugin.h
> > +++ b/kernel/rcu/tree_plugin.h
> > @@ -1226,18 +1226,6 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
> > free_cpumask_var(cm);
> > }
> >
> > -/*
> > - * Spawn boost kthreads -- called as soon as the scheduler is running.
> > - */
> > -static void __init rcu_spawn_boost_kthreads(void)
> > -{
> > - struct rcu_node *rnp;
> > -
> > - rcu_for_each_leaf_node(rnp)
> > - if (rcu_rnp_online_cpus(rnp))
> > - rcu_spawn_one_boost_kthread(rnp);
> > -}
> > -
> > #else /* #ifdef CONFIG_RCU_BOOST */
> >
> > static void rcu_initiate_boost(struct rcu_node *rnp, unsigned long flags)
> > @@ -1263,10 +1251,6 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
> > {
> > }
> >
> > -static void __init rcu_spawn_boost_kthreads(void)
> > -{
> > -}
> > -
> > #endif /* #else #ifdef CONFIG_RCU_BOOST */
> >
> > /*
>
> nocb kthread creation is similar but it depends on the gp kthread.
> So we can't rely on rcu_init() -> rcu_prepare_cpu() and we must keep
> the early_initcall() -> rcu_spawn_gp_kthread().
>
> That would become (untested again):
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index da8ac2b6f8cc..9284625a9a50 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -4525,7 +4525,7 @@ static int __init rcu_spawn_gp_kthread(void)
> smp_store_release(&rcu_state.gp_kthread, t); /* ^^^ */
> raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> wake_up_process(t);
> - rcu_spawn_nocb_kthreads();
> + rcu_spawn_cpu_nocb_kthread(smp_processor_id());
> rcu_spawn_core_kthreads();
> return 0;
> }
> diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
> index 636d0546a4e9..711f6eb7f7e1 100644
> --- a/kernel/rcu/tree_nocb.h
> +++ b/kernel/rcu/tree_nocb.h
> @@ -1277,22 +1277,6 @@ static void rcu_spawn_cpu_nocb_kthread(int cpu)
> WRITE_ONCE(rdp->nocb_gp_kthread, rdp_gp->nocb_gp_kthread);
> }
>
> -/*
> - * Once the scheduler is running, spawn rcuo kthreads for all online
> - * no-CBs CPUs. This assumes that the early_initcall()s happen before
> - * non-boot CPUs come online -- if this changes, we will need to add
> - * some mutual exclusion.
> - */
> -static void __init rcu_spawn_nocb_kthreads(void)
> -{
> - int cpu;
> -
> - if (rcu_nocb_is_setup) {
> - for_each_online_cpu(cpu)
> - rcu_spawn_cpu_nocb_kthread(cpu);
> - }
> -}
> -
> /* How many CB CPU IDs per GP kthread? Default of -1 for sqrt(nr_cpu_ids). */
> static int rcu_nocb_gp_stride = -1;
> module_param(rcu_nocb_gp_stride, int, 0444);
> @@ -1549,10 +1533,6 @@ static void rcu_spawn_cpu_nocb_kthread(int cpu)
> {
> }
>
> -static void __init rcu_spawn_nocb_kthreads(void)
> -{
> -}
> -
> static void show_rcu_nocb_state(struct rcu_data *rdp)
> {
> }
