[PATCH 6/6] cgroup/cpuset: Don't make a forking task wait for memory migration

[Matthew Ruffell]

From: Caron Wills <caron.wills@xxxxxxxxxx>

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 <caron.wills@xxxxxxxxxx>
Signed-off-by: Matthew Ruffell <matthew.ruffell@xxxxxxxxxxxxx>
---
 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