On 01/19/2016 06:18 PM, Tejun Heo wrote: > If "cpuset.memory_migrate" is set, when a process is moved from one > cpuset to another with a different memory node mask, pages in used by > the process are migrated to the new set of nodes. This was performed > synchronously in the ->attach() callback, which is synchronized > against process management. Recently, the synchronization was changed > from per-process rwsem to global percpu rwsem for simplicity and > optimization. > > Combined with the synchronous mm migration, this led to deadlocks > because mm migration could schedule a work item which may in turn try > to create a new worker blocking on the process management lock held > from cgroup process migration path. > > This heavy an operation shouldn't be performed synchronously from that > deep inside cgroup migration in the first place. This patch punts the > actual migration to an ordered workqueue and updates cgroup process > migration and cpuset config update paths to flush the workqueue after > all locks are released. This way, the operations still seem > synchronous to userland without entangling mm migration with process > management synchronization. CPU hotplug can also invoke mm migration > but there's no reason for it to wait for mm migrations and thus > doesn't synchronize against their completions. > > Signed-off-by: Tejun Heo <[email protected]> > Reported-and-tested-by: Christian Borntraeger <[email protected]>
Hmmm I just realized that this patch slightly differs from the one that I tested. Do we need a retest? > Cc: [email protected] # v4.4+ > --- > include/linux/cpuset.h | 6 ++++ > kernel/cgroup.c | 2 + > kernel/cpuset.c | 71 > +++++++++++++++++++++++++++++++++---------------- > 3 files changed, 57 insertions(+), 22 deletions(-) > > diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h > index 85a868c..fea160e 100644 > --- a/include/linux/cpuset.h > +++ b/include/linux/cpuset.h > @@ -137,6 +137,8 @@ static inline void set_mems_allowed(nodemask_t nodemask) > task_unlock(current); > } > > +extern void cpuset_post_attach_flush(void); > + > #else /* !CONFIG_CPUSETS */ > > static inline bool cpusets_enabled(void) { return false; } > @@ -243,6 +245,10 @@ static inline bool read_mems_allowed_retry(unsigned int > seq) > return false; > } > > +static inline void cpuset_post_attach_flush(void) > +{ > +} > + > #endif /* !CONFIG_CPUSETS */ > > #endif /* _LINUX_CPUSET_H */ > diff --git a/kernel/cgroup.c b/kernel/cgroup.c > index c03a640..88abd4d 100644 > --- a/kernel/cgroup.c > +++ b/kernel/cgroup.c > @@ -58,6 +58,7 @@ > #include <linux/kthread.h> > #include <linux/delay.h> > #include <linux/atomic.h> > +#include <linux/cpuset.h> > #include <net/sock.h> > > /* > @@ -2739,6 +2740,7 @@ static ssize_t __cgroup_procs_write(struct > kernfs_open_file *of, char *buf, > out_unlock_threadgroup: > percpu_up_write(&cgroup_threadgroup_rwsem); > cgroup_kn_unlock(of->kn); > + cpuset_post_attach_flush(); > return ret ?: nbytes; > } > > diff --git a/kernel/cpuset.c b/kernel/cpuset.c > index 3e945fc..41989ab 100644 > --- a/kernel/cpuset.c > +++ b/kernel/cpuset.c > @@ -287,6 +287,8 @@ static struct cpuset top_cpuset = { > static DEFINE_MUTEX(cpuset_mutex); > static DEFINE_SPINLOCK(callback_lock); > > +static struct workqueue_struct *cpuset_migrate_mm_wq; > + > /* > * CPU / memory hotplug is handled asynchronously. > */ > @@ -972,31 +974,51 @@ static int update_cpumask(struct cpuset *cs, struct > cpuset *trialcs, > } > > /* > - * cpuset_migrate_mm > - * > - * Migrate memory region from one set of nodes to another. > - * > - * Temporarilly set tasks mems_allowed to target nodes of migration, > - * so that the migration code can allocate pages on these nodes. > - * > - * While the mm_struct we are migrating is typically from some > - * other task, the task_struct mems_allowed that we are hacking > - * is for our current task, which must allocate new pages for that > - * migrating memory region. > + * Migrate memory region from one set of nodes to another. This is > + * performed asynchronously as it can be called from process migration path > + * holding locks involved in process management. All mm migrations are > + * performed in the queued order and can be waited for by flushing > + * cpuset_migrate_mm_wq. > */ > > +struct cpuset_migrate_mm_work { > + struct work_struct work; > + struct mm_struct *mm; > + nodemask_t from; > + nodemask_t to; > +}; > + > +static void cpuset_migrate_mm_workfn(struct work_struct *work) > +{ > + struct cpuset_migrate_mm_work *mwork = > + container_of(work, struct cpuset_migrate_mm_work, work); > + > + /* on a wq worker, no need to worry about %current's mems_allowed */ > + do_migrate_pages(mwork->mm, &mwork->from, &mwork->to, MPOL_MF_MOVE_ALL); > + mmput(mwork->mm); > + kfree(mwork); > +} > + > static void cpuset_migrate_mm(struct mm_struct *mm, const nodemask_t *from, > const nodemask_t *to) > { > - struct task_struct *tsk = current; > - > - tsk->mems_allowed = *to; > + struct cpuset_migrate_mm_work *mwork; > > - do_migrate_pages(mm, from, to, MPOL_MF_MOVE_ALL); > + mwork = kzalloc(sizeof(*mwork), GFP_KERNEL); > + if (mwork) { > + mwork->mm = mm; > + mwork->from = *from; > + mwork->to = *to; > + INIT_WORK(&mwork->work, cpuset_migrate_mm_workfn); > + queue_work(cpuset_migrate_mm_wq, &mwork->work); > + } else { > + mmput(mm); > + } > +} > > - rcu_read_lock(); > - guarantee_online_mems(task_cs(tsk), &tsk->mems_allowed); > - rcu_read_unlock(); > +void cpuset_post_attach_flush(void) > +{ > + flush_workqueue(cpuset_migrate_mm_wq); > } > > /* > @@ -1097,7 +1119,8 @@ static void update_tasks_nodemask(struct cpuset *cs) > mpol_rebind_mm(mm, &cs->mems_allowed); > if (migrate) > cpuset_migrate_mm(mm, &cs->old_mems_allowed, &newmems); > - mmput(mm); > + else > + mmput(mm); > } > css_task_iter_end(&it); > > @@ -1545,11 +1568,11 @@ static void cpuset_attach(struct cgroup_taskset *tset) > * @old_mems_allowed is the right nodesets that we > * migrate mm from. > */ > - if (is_memory_migrate(cs)) { > + if (is_memory_migrate(cs)) > cpuset_migrate_mm(mm, &oldcs->old_mems_allowed, > &cpuset_attach_nodemask_to); > - } > - mmput(mm); > + else > + mmput(mm); > } > } > > @@ -1714,6 +1737,7 @@ static ssize_t cpuset_write_resmask(struct > kernfs_open_file *of, > mutex_unlock(&cpuset_mutex); > kernfs_unbreak_active_protection(of->kn); > css_put(&cs->css); > + flush_workqueue(cpuset_migrate_mm_wq); > return retval ?: nbytes; > } > > @@ -2359,6 +2383,9 @@ void __init cpuset_init_smp(void) > top_cpuset.effective_mems = node_states[N_MEMORY]; > > register_hotmemory_notifier(&cpuset_track_online_nodes_nb); > + > + cpuset_migrate_mm_wq = alloc_ordered_workqueue("cpuset_migrate_mm", 0); > + BUG_ON(!cpuset_migrate_mm_wq); > } > > /** >
