[PATCH-next 1/3] cgroup/cpuset: Handle the special case of non-moving tasks in cpuset_can_attach()

[Waiman Long <longman@redhat.com>]

With cgroup v2 migration of a multithreaded process having threads
in different cgroups of a threaded subtree, it is possible that
cpuset_can_attach() can be called with tasks that are not migrating with
respect to cpuset if cpuset controller is not enabled in some of the
subtree cgroups. IOW, the old cpuset can be the same as the new one. This
can cause problem when we need to track the set of old cpusets and the
new cpusets in singly linked lists as a cpuset cannot be in both lists.

As reported by Tejun, the following is an example threaded subtree with
partial cpuset delegation that can cause this issue to show up.

  P (+cpuset)
  |- R (cpuset)        <- destination
  |  `- C (no cpuset)  -> effective cpuset == R
  `- W (cpuset)

Group leader in R, thread_a in C, thread_b in W; migrate the whole
process into R (echo $PID > R/cgroup.procs). thread_a moves C->R:
its cgroup changes so compare_css_sets() keeps it in the taskset, but
its cpuset css is unchanged (C inherits R's), so task_cs() == cs ==
R. cpuset is in ss_mask because thread_b (W->R) changed. can_attach()
then tags R as a source (thread_a) and the destination (thread_b):

Handle this special case by skipping tasks that are not migrating in
cpuset_can_attach() and avoid calling cpuset_can_attach_check() in this
case. By doing so, the destination cpuset will not be put into source
cpuset linked list.

As the source cpuset cannot be easily determined in cpuset_attach(),
unnecessary work can be performed if a task is not actually
migrating. However, no harm will be done except wasting some
CPU cycles. If it happens that none of the tasks is migrating,
attach_ctx.cpus_updated and attach_ctx.mems_updated will both be
false. So no task iteration will be done in this case.

Reported-by: Tejun Heo <tj@kernel.org>
Closes: https://lore.kernel.org/lkml/e254af713b5345aec3d086771ecf1e71@kernel.org
Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index cf0d005d2b78..d99184ec60b5 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3136,18 +3136,13 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
 
 	/* used later by cpuset_attach() */
 	attach_ctx.old_cs = task_cs(cgroup_taskset_first(tset, &css));
-	oldcs = attach_ctx.old_cs;
+	oldcs = NULL;
 	cs = css_cs(css);
 
 	mutex_lock(&cpuset_mutex);
 	attach_ctx.cpus_updated = false;
 	attach_ctx.mems_updated = false;
 
-	/* Check to see if task is allowed in the cpuset */
-	ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
-	if (ret)
-		goto out_unlock;
-
 	/*
 	 * The attach_ctx.old_cs is used mainly by cpuset_migrate_mm() to get
 	 * the old_mems_allowed value. There are two ways that many-to-one
@@ -3164,17 +3159,27 @@ static int cpuset_can_attach(struct cgroup_taskset *tset)
 	 * of child cpusets must always be a subset of the parent. So no real
 	 * page migration will be necessary no matter which child cpuset is
 	 * selected as attach_ctx.old_cs.
+	 *
+	 * For a v2 threaded subtree where cpuset isn't enabled in some of the
+	 * cgroups, it is possible that oldcs == cs for some of the tasks.
+	 * In this case, we can skip checking on those tasks as there is no
+	 * actual migration wrt cpuset.
 	 */
 	cgroup_taskset_for_each(task, css, tset) {
 		struct cpuset *new_oldcs = task_cs(task);
 
 		if (new_oldcs != oldcs) {
 			oldcs = new_oldcs;
+			if (oldcs == cs)
+				continue;
 			ret = cpuset_can_attach_check(cs, oldcs, &setsched_check);
 			if (ret)
 				goto out_unlock;
 		}
 
+		if (oldcs == cs)
+			continue;
+
 		ret = task_can_attach(task);
 		if (ret)
 			goto out_unlock;
-- 
2.55.0