by Paul E. McKenney

[permalink] [raw]

Subject: Re: [PATCH 07/10] rcu: Conditionally build CPU-hotplug teardown callbacks

On Fri, Sep 08, 2023 at 10:36:00PM +0200, Frederic Weisbecker wrote:
> Among the three CPU-hotplug teardown RCU callbacks, two of them early
> exit if CONFIG_HOTPLUG_CPU=n, and one is left unchanged. In any case
> all of them have an implementation when CONFIG_HOTPLUG_CPU=n.
>
> Align instead with the common way to deal with CPU-hotplug teardown
> callbacks and provide a proper stub when they are not supported.
>
> Signed-off-by: Frederic Weisbecker <[email protected]>

Good eyes!

Reviewed-by: Paul E. McKenney <[email protected]>

> ---
> include/linux/rcutree.h | 11 +++-
> kernel/rcu/tree.c | 114 +++++++++++++++++++---------------------
> 2 files changed, 63 insertions(+), 62 deletions(-)
>
> diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> index af6ddbd291eb..7d75066c72aa 100644
> --- a/include/linux/rcutree.h
> +++ b/include/linux/rcutree.h
> @@ -109,9 +109,16 @@ void rcu_all_qs(void);
> /* RCUtree hotplug events */
> int rcutree_prepare_cpu(unsigned int cpu);
> int rcutree_online_cpu(unsigned int cpu);
> -int rcutree_offline_cpu(unsigned int cpu);
> +void rcu_cpu_starting(unsigned int cpu);
> +
> +#ifdef CONFIG_HOTPLUG_CPU
> int rcutree_dead_cpu(unsigned int cpu);
> int rcutree_dying_cpu(unsigned int cpu);
> -void rcu_cpu_starting(unsigned int cpu);
> +int rcutree_offline_cpu(unsigned int cpu);
> +#else
> +#define rcutree_dead_cpu NULL
> +#define rcutree_dying_cpu NULL
> +#define rcutree_offline_cpu NULL
> +#endif
>
> #endif /* __LINUX_RCUTREE_H */
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 289c51417cbc..875f241db508 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -4228,25 +4228,6 @@ static bool rcu_init_invoked(void)
> return !!rcu_state.n_online_cpus;
> }
>
> -/*
> - * Near the end of the offline process. Trace the fact that this CPU
> - * is going offline.
> - */
> -int rcutree_dying_cpu(unsigned int cpu)
> -{
> - bool blkd;
> - struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> - struct rcu_node *rnp = rdp->mynode;
> -
> - if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))
> - return 0;
> -
> - blkd = !!(READ_ONCE(rnp->qsmask) & rdp->grpmask);
> - trace_rcu_grace_period(rcu_state.name, READ_ONCE(rnp->gp_seq),
> - blkd ? TPS("cpuofl-bgp") : TPS("cpuofl"));
> - return 0;
> -}
> -
> /*
> * All CPUs for the specified rcu_node structure have gone offline,
> * and all tasks that were preempted within an RCU read-side critical
> @@ -4292,23 +4273,6 @@ static void rcu_cleanup_dead_rnp(struct rcu_node *rnp_leaf)
> }
> }
>
> -/*
> - * The CPU has been completely removed, and some other CPU is reporting
> - * this fact from process context. Do the remainder of the cleanup.
> - * There can only be one CPU hotplug operation at a time, so no need for
> - * explicit locking.
> - */
> -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);
> - // Stop-machine done, so allow nohz_full to disable tick.
> - tick_dep_clear(TICK_DEP_BIT_RCU);
> - return 0;
> -}
> -
> /*
> * Propagate ->qsinitmask bits up the rcu_node tree to account for the
> * first CPU in a given leaf rcu_node structure coming online. The caller
> @@ -4461,29 +4425,6 @@ int rcutree_online_cpu(unsigned int cpu)
> return 0;
> }
>
> -/*
> - * Near the beginning of the process. The CPU is still very much alive
> - * with pretty much all services enabled.
> - */
> -int rcutree_offline_cpu(unsigned int cpu)
> -{
> - unsigned long flags;
> - struct rcu_data *rdp;
> - struct rcu_node *rnp;
> -
> - rdp = per_cpu_ptr(&rcu_data, cpu);
> - rnp = rdp->mynode;
> - raw_spin_lock_irqsave_rcu_node(rnp, flags);
> - rnp->ffmask &= ~rdp->grpmask;
> - raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> -
> - rcutree_affinity_setting(cpu, cpu);
> -
> - // nohz_full CPUs need the tick for stop-machine to work quickly
> - tick_dep_set(TICK_DEP_BIT_RCU);
> - return 0;
> -}
> -
> /*
> * Mark the specified CPU as being online so that subsequent grace periods
> * (both expedited and normal) will wait on it. Note that this means that
> @@ -4637,7 +4578,60 @@ void rcutree_migrate_callbacks(int cpu)
> cpu, rcu_segcblist_n_cbs(&rdp->cblist),
> rcu_segcblist_first_cb(&rdp->cblist));
> }
> -#endif
> +
> +/*
> + * The CPU has been completely removed, and some other CPU is reporting
> + * this fact from process context. Do the remainder of the cleanup.
> + * There can only be one CPU hotplug operation at a time, so no need for
> + * explicit locking.
> + */
> +int rcutree_dead_cpu(unsigned int cpu)
> +{
> + WRITE_ONCE(rcu_state.n_online_cpus, rcu_state.n_online_cpus - 1);
> + // Stop-machine done, so allow nohz_full to disable tick.
> + tick_dep_clear(TICK_DEP_BIT_RCU);
> + return 0;
> +}
> +
> +/*
> + * Near the end of the offline process. Trace the fact that this CPU
> + * is going offline.
> + */
> +int rcutree_dying_cpu(unsigned int cpu)
> +{
> + bool blkd;
> + struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> + struct rcu_node *rnp = rdp->mynode;
> +
> + blkd = !!(READ_ONCE(rnp->qsmask) & rdp->grpmask);
> + trace_rcu_grace_period(rcu_state.name, READ_ONCE(rnp->gp_seq),
> + blkd ? TPS("cpuofl-bgp") : TPS("cpuofl"));
> + return 0;
> +}
> +
> +/*
> + * Near the beginning of the process. The CPU is still very much alive
> + * with pretty much all services enabled.
> + */
> +int rcutree_offline_cpu(unsigned int cpu)
> +{
> + unsigned long flags;
> + struct rcu_data *rdp;
> + struct rcu_node *rnp;
> +
> + rdp = per_cpu_ptr(&rcu_data, cpu);
> + rnp = rdp->mynode;
> + raw_spin_lock_irqsave_rcu_node(rnp, flags);
> + rnp->ffmask &= ~rdp->grpmask;
> + raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
> +
> + rcutree_affinity_setting(cpu, cpu);
> +
> + // nohz_full CPUs need the tick for stop-machine to work quickly
> + tick_dep_set(TICK_DEP_BIT_RCU);
> + return 0;
> +}
> +#endif /* #ifdef CONFIG_HOTPLUG_CPU */
>
> /*
> * On non-huge systems, use expedited RCU grace periods to make suspend
> --
> 2.41.0
>