On Fri, Sep 08, 2023 at 10:35:59PM +0200, Frederic Weisbecker wrote:
> rcu_report_dead() has to be called locally by the CPU that is going to
> exit the RCU state machine. Passing a cpu argument here is error-prone
> and leaves the possibility for a racy remote call.
>
> Use local access instead.
>
> Signed-off-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
I was going to ask for an assertion for "cpu" in cpu_die_early(), but
given that its value comes from smp_processor_id() just a few lines
earlier, there isn't a whole lot of point to that. So:
Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> ---
> arch/arm64/kernel/smp.c | 2 +-
> include/linux/rcupdate.h | 2 +-
> kernel/cpu.c | 2 +-
> kernel/rcu/tree.c | 4 ++--
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index edd63894d61e..ce672cb69f1c 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -401,7 +401,7 @@ void __noreturn cpu_die_early(void)
>
> /* Mark this CPU absent */
> set_cpu_present(cpu, 0);
> - rcu_report_dead(cpu);
> + rcu_report_dead();
>
> if (IS_ENABLED(CONFIG_HOTPLUG_CPU)) {
> update_cpu_boot_status(CPU_KILL_ME);
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 5e5f920ade90..aa351ddcbe8d 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -122,7 +122,7 @@ static inline void call_rcu_hurry(struct rcu_head *head, rcu_callback_t func)
> void rcu_init(void);
> extern int rcu_scheduler_active;
> void rcu_sched_clock_irq(int user);
> -void rcu_report_dead(unsigned int cpu);
> +void rcu_report_dead(void);
> void rcutree_migrate_callbacks(int cpu);
>
> #ifdef CONFIG_TASKS_RCU_GENERIC
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 88a7ede322bd..86f08eafbd9f 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1368,7 +1368,7 @@ void cpuhp_report_idle_dead(void)
> struct cpuhp_cpu_state *st = this_cpu_ptr(&cpuhp_state);
>
> BUG_ON(st->state != CPUHP_AP_OFFLINE);
> - rcu_report_dead(smp_processor_id());
> + rcu_report_dead();
> st->state = CPUHP_AP_IDLE_DEAD;
> /*
> * We cannot call complete after rcu_report_dead() so we delegate it
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 8b5ebef32e17..289c51417cbc 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -4551,11 +4551,11 @@ void rcu_cpu_starting(unsigned int cpu)
> * from the outgoing CPU rather than from the cpuhp_step mechanism.
> * This is because this function must be invoked at a precise location.
> */
> -void rcu_report_dead(unsigned int cpu)
> +void rcu_report_dead(void)
> {
> unsigned long flags;
> unsigned long mask;
> - struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
> + struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
> struct rcu_node *rnp = rdp->mynode; /* Outgoing CPU's rdp & rnp. */
>
> /*
> --
> 2.41.0
>
