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
>
