On Fri, Sep 18, 2020 at 09:00:03AM +0200, Thomas Gleixner wrote: > >> +void migrate_disable(void) > >> +{ > >> + unsigned long flags; > >> + > >> + if (!current->migration_ctrl.disable_cnt) { > >> + raw_spin_lock_irqsave(¤t->pi_lock, flags); > >> + current->migration_ctrl.disable_cnt++; > >> + raw_spin_unlock_irqrestore(¤t->pi_lock, flags); > >> + } else { > >> + current->migration_ctrl.disable_cnt++; > >> + } > >> +} > > > > That pi_lock seems unfortunate, and it isn't obvious what the point of > > it is. > > Indeed. That obviously lacks a big fat comment. > > current->migration_ctrl.disable_cnt++ is obviously a RMW operation. So > you end up with the following: > > CPU0 CPU1 > migrate_disable() > R = current->migration_ctrl.disable_cnt; > set_cpus_allowed_ptr() > task_rq_lock(); > if > > (!p->migration_ctrl.disable_cnt) { > current->migration_ctrl.disable_cnt = R + 1; > stop_one_cpu(); > ---> stopper_thread() > BUG_ON(task->migration_ctrl.disable_cnt); > > I tried to back out from that instead of BUG(), but that ended up being > even more of a hacky trainwreck than just biting the bullet and taking > pi_lock.
You don't need the load-store for that I think, pure timing will do. Blergh, lemme prepare more wake-up juice and think about that.