On Tue, Sep 20, 2022 at 12:31:43PM +0200, Frederic Weisbecker wrote: > 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); > > OK. > > @@ -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. > */ > Appreciate for your re-phrase. I will use it in the next version Thanks, Pingfan
