From: Caron Wills Since commit 65e510cd30d0 ("cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside cpuset_attach_task()") a CLONE_INTO_CGROUP fork into a cpuset that differs from the parent's goes through cpuset_attach_task(), which queues a memory migration of the child's mm and sets attach_ctx.task_work_queued, and cpuset_fork() then schedules flush_migrate_mm_task_workfn() on the *parent*. Two things are wrong with that: - task_work_queued is set whenever is_memory_migrate(cs) is true, which on cgroup v2 is always, even when cpuset_migrate_mm() queued nothing because the nodemasks were equal. The parent then flushes cpuset_migrate_mm_wq for no reason of its own. - cpuset_migrate_mm_wq is a single ordered workqueue, so the flush waits for every migration queued by anyone, not just this fork's. systemd forks services with CLONE_INTO_CGROUP, and as soon as one slice carries a cpuset setting every top-level slice has a cpuset of its own, so PID 1 blocks in flush_workqueue() on every service start for as long as some unrelated cpuset.mems migration takes. On the AWS systems of LP: #2165410 that is minutes, during which logind cannot create sessions and SSH logins fail. Make cpuset_migrate_mm() report whether it queued work and only set task_work_queued in that case, and don't schedule the flush from cpuset_fork() at all: the forking task is neither the cpuset.mems writer nor the cgroup.procs writer for whom the synchronous semantics were introduced by commit e93ad19d0564 ("cpuset: make mm migration asynchronous"), and nothing depends on the child's pages having moved before the parent returns from fork(). The migration itself still runs, in the background. Fixes: 65e510cd30d0 ("cgroup/cpuset: Move mpol_rebind_mm/cpuset_migrate_mm() calls inside cpuset_attach_task()") Link: https://bugs.launchpad.net/bugs/2165410 Signed-off-by: Caron Wills Signed-off-by: Matthew Ruffell --- kernel/cgroup/cpuset.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c index 439b55a310fe..b331689eab24 100644 --- a/kernel/cgroup/cpuset.c +++ b/kernel/cgroup/cpuset.c @@ -2704,26 +2704,32 @@ static void cpuset_migrate_mm_workfn(struct work_struct *work) kfree(mwork); } -static void cpuset_migrate_mm(struct mm_struct *mm, const nodemask_t *from, +/* + * Returns true if migration work was queued, in which case the caller may + * want to flush cpuset_migrate_mm_wq before returning to userspace. + */ +static bool cpuset_migrate_mm(struct mm_struct *mm, const nodemask_t *from, const nodemask_t *to) { struct cpuset_migrate_mm_work *mwork; if (nodes_equal(*from, *to)) { mmput(mm); - return; + return false; } mwork = kzalloc_obj(*mwork); - 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 { + if (!mwork) { mmput(mm); + return false; } + + 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); + return true; } static void flush_migrate_mm_task_workfn(struct callback_head *head) @@ -3376,9 +3382,9 @@ static void cpuset_attach_task(struct cpuset *cs, struct task_struct *task) * migrate mm from. */ if (is_memory_migrate(cs)) { - cpuset_migrate_mm(mm, &oldcs->old_mems_allowed, - &attach_ctx.nodemask_to); - attach_ctx.task_work_queued = true; + if (cpuset_migrate_mm(mm, &oldcs->old_mems_allowed, + &attach_ctx.nodemask_to)) + attach_ctx.task_work_queued = true; } else { mmput(mm); } @@ -3887,8 +3893,13 @@ static void cpuset_fork(struct task_struct *task) attach_ctx.task_work_queued = false; attach_ctx.old_cs = oldcs; cpuset_attach_task(cs, task); - if (attach_ctx.task_work_queued) - schedule_flush_migrate_mm(); + /* + * Unlike a cpuset.mems or cgroup.procs writer, the forking task has + * no business waiting for the child's pages to be migrated, and a + * flush of the ordered cpuset_migrate_mm_wq would make it wait for + * every migration anyone queued. Let the migration run in the + * background. + */ dec_attach_in_progress_locked(); mutex_unlock(&cpuset_mutex); -- 2.55.0