Re: [PATCHv2 1/3] rcu: Keep qsmaskinitnext fresh when rcutree_online_cpu()

[Frederic Weisbecker <frederic@kernel.org>]

On Thu, Sep 15, 2022 at 01:58:23PM +0800, Pingfan Liu wrote:
> rcutree_online_cpu() is concurrent, so there is the following race
> scene:
> 
>         CPU 1                               CPU2
> mask_old = rcu_rnp_online_cpus(rnp);
> ...
> 
>                                    mask_new = rcu_rnp_online_cpus(rnp);
>                                    ...
>                                    set_cpus_allowed_ptr(t, cm);
> 
> set_cpus_allowed_ptr(t, cm);

Just for clarity, may I suggest:


          CPU 0                                    CPU 1
          -----                                    -----
       rcutree_online_cpu()
           rcu_boost_kthread_setaffinity()
               mask = rcu_rnp_online_cpus(rnp);
                                                rcu_cpu_starting()
                                                    WRITE_ONCE(rnp->qsmaskinitnext, rnp->qsmaskinitnext | mask);
                                                rcutree_online_cpu()
                                                    rcu_boost_kthread_setaffinity()
                                                        mask = rcu_rnp_online_cpus(rnp);
                                                        set_cpus_allowed_ptr(t, cm);
               set_cpus_allowed_ptr(t, cm);


> @@ -1233,6 +1233,11 @@ static void rcu_boost_kthread_setaffinity(struct rcu_node *rnp, int outgoingcpu)
>  	if (!zalloc_cpumask_var(&cm, GFP_KERNEL))
>  		return;
>  	mutex_lock(&rnp->boost_kthread_mutex);
> +	/*
> +	 * Relying on the lock to serialize, so when onlining, the latest
> +	 * qsmaskinitnext is for cpus_ptr.
> +	 */

Given how tricky the situation is, I suggest to also add more details to the
comment:

        /*
         * Rely on the boost mutex to serialize ->qsmaskinitnext and
         * set_cpus_allowed_ptr(), so when concurrently onlining, racing against
	 * rcu_cpu_starting() makes sure that the latest update eventually
	 * wins with the right affinity setting.
         */

Thanks!

> +	mask = rcu_rnp_online_cpus(rnp);
>  	for_each_leaf_node_possible_cpu(rnp, cpu)
>  		if ((mask & leaf_node_cpu_bit(rnp, cpu)) &&
>  		    cpu != outgoingcpu)
> -- 
> 2.31.1
>