Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework
@ 2026-11-07 12:00 Ridong Chen
  2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
	Sashiko, Guopeng Zhang, Ridong Chen

From: Ridong Chen <chenridong@xiaomi.com>

This is the first part of the cpuset local partition rework posted as
RFC v4 [1]. The rework is split into three parts to make it easier to
review and merge:

  1) fixes and preparatory cleanups (this series);
  2) introduce partition_enable/disable/update() and rework the local
     partition code on top of them;
  3) clean up the remaining partition state handling outside these
     helpers.

Patch 1 is a fix for an existing bug, found while looking into a review
comment on RFC v4: when cpuset.cpus.exclusive of an already invalid
partition root is cleared, its effective_xcpus is not reset, so its
former exclusive CPUs can't be claimed by a sibling. A test case is
added for it.

Patches 2-4 are the first three patches of RFC v4, rebased onto
next-20261104:

  2) add an early empty cpumask check in partition_xcpus_add/del(), so
     callers don't need to check it;
  3) settle the isolation state of CPUs given back to the parent in
     partition_xcpus_del(), so isolated_cpus_release() can be removed;
  4) generalize validate_partition() so it can be used for both local
     and remote partitions.

All patches build on their own, and test_cpuset_prs.sh passes with
this series applied.

[1] https://lore.kernel.org/all/20261101114206.2434398-1-ridong.chen@linux.dev/

Changes in v3:
  - Patch 3: merge isolated_cpus_set_state() into isolated_cpus_update(),
    dropping its @old_prs argument and the WARN_ON_ONCE(), and drop the
    state check in partition_xcpus_add() (Waiman).
    Drop the Acked-by from Guopeng as the patch changed.
  - Patch 4: use top_cpuset as the parent of a remote partition and pass
    the parent to tasks_nocpu_error() instead of parent_cs(cs) (Waiman).

Changes in v2:
  - Patch 1: force update_cpumasks_hier() after reset_partition_data(),
    so that the descendants don't keep the released exclusive CPUs, and
    add a test case for it (Sashiko).
  - Patch 1: fix the Fixes tags (Sashiko).
  - Link to v1: https://lore.kernel.org/all/20261105125215.2205892-1-ridong.chen@linux.dev/

Changes since RFC v4:
  - New patch 1 to fix the invalid partition effective_xcpus reset.
  - Patch 3: only settle the isolation state of the CPUs still owned by
    the parent when invalidating a child partition in
    compute_partition_effective_cpumask(), as the parent may be shrunk
    and not updated yet. This code is removed later in the rework.
  - Rebase onto next-20261104, which includes commit 362212853cd0
    ("cgroup/cpuset: Fix incorrect CPU accounting with parent's effective_xcpus expansion").

Chen Ridong (2):
  cpuset: add early empty cpumask check in partition_xcpus_add/del
  cpuset: generalize the validate_partition() interface

Ridong Chen (2):
  cgroup/cpuset: Reset partition data when an invalid partition stays
    invalid
  cpuset: settle isolation state when CPUs are given back to parent

 kernel/cgroup/cpuset.c                        | 236 +++++++-----------
 .../selftests/cgroup/test_cpuset_prs.sh       |   6 +
 2 files changed, 100 insertions(+), 142 deletions(-)


base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4
-- 
2.34.1


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid
  2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
@ 2026-11-07 12:00 ` Ridong Chen
  2026-11-09 14:59   ` Guopeng Zhang
  2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
	Sashiko, Guopeng Zhang, Ridong Chen

From: Ridong Chen <chenridong@xiaomi.com>

When cpuset.cpus or cpuset.cpus.exclusive of a partition root is
changed, update_cpumask() and update_exclusive_cpumask() copy the new
effective_xcpus computed for the trial cpuset into the cpuset, and call
reset_partition_data() only if the partition was valid before and is
invalid now:

	if ((old_prs > 0) && !is_partition_valid(cs))
		reset_partition_data(cs);

An invalid partition root (old_prs < 0) that stays invalid is not
covered. Its effective_xcpus is then left with the trial value, which
is computed from user_xcpus() and so falls back to cpuset.cpus when
cpuset.cpus.exclusive is cleared. As an invalid partition root does not
own any exclusive CPUs, and rm_siblings_excl_cpus() treats a non-empty
effective_xcpus of a sibling without cpuset.cpus.exclusive as owned,
those CPUs can then no longer be claimed by a sibling:

  # A1: cpuset.cpus=2-3, cpuset.cpus.exclusive=2-3, root, with tasks
  # offline CPUs 2-3 -> A1 becomes invalid
  echo > A1/cpuset.cpus.exclusive
  cat A1/cpuset.cpus.exclusive.effective    # 2-3, expected empty
  echo 3 > B1/cpuset.cpus.exclusive         # -EINVAL

Reset the partition data whenever the cpuset is a partition root, valid
or not, and ends up invalid. As effective_xcpus may be cleared by that,
also force update_cpumasks_hier() to update the subtree, so that the
effective_xcpus of the descendants doesn't keep the released CPUs. Add
test cases for it.

Fixes: de9f15e21c55 ("cpuset: use parse_cpulist for setting cpus.exclusive")
Fixes: c5866c9a007d ("cpuset: refactor CPU mask buffer parsing logic")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20261101114206.2434398-1-ridong.chen%40linux.dev?part=15
Reviewed-by: Waiman Long <longman@redhat.com>
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 kernel/cgroup/cpuset.c                            | 10 ++++++++--
 tools/testing/selftests/cgroup/test_cpuset_prs.sh |  6 ++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 3052413e41e7..cf38e8708ddd 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2607,8 +2607,11 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 	spin_lock_irq(&callback_lock);
 	cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
 	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
-	if ((old_prs > 0) && !is_partition_valid(cs))
+	if (old_prs && !is_partition_valid(cs)) {
 		reset_partition_data(cs);
+		/* effective_xcpus may be cleared, update the subtree too */
+		force = true;
+	}
 	spin_unlock_irq(&callback_lock);
 
 	/* effective_cpus/effective_xcpus will be updated here */
@@ -2672,8 +2675,11 @@ static int update_exclusive_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 	spin_lock_irq(&callback_lock);
 	cpumask_copy(cs->exclusive_cpus, trialcs->exclusive_cpus);
 	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
-	if ((old_prs > 0) && !is_partition_valid(cs))
+	if (old_prs && !is_partition_valid(cs)) {
 		reset_partition_data(cs);
+		/* effective_xcpus may be cleared, update the subtree too */
+		force = true;
+	}
 	spin_unlock_irq(&callback_lock);
 
 	/*
diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index bdca6c471f87..32e1e44fa0b1 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -434,6 +434,12 @@ TEST_MATRIX=(
 	# cpuset.cpus is allowed, but those extra CPUs are ignored.
 	"  C2-3:P1   C3:P1  .      .      .     C2-4    .      .     0 A1:|A2:2-3 A1:P1|A2:P1"
 
+	# Clearing cpuset.cpus.exclusive of an invalid partition releases its
+	# exclusive CPUs, including those of its descendants, so a sibling
+	# can claim them.
+	" CX2-3:P1    .     .     C3  T:O2=0:O3=0:X  .     .      X3   0 XA1:|XB1:3 A1:P-1"
+	" CX2-3:P1   X3     .     C3  T:O2=0:O3=0:X  .     .      X3   0 XA1:|XA2:|XB1:3 A1:P-1"
+
 	# A child cannot recover until its exclusive CPUs are available again.
 	" CX1-4:P1 CX3-4:P1 .     .     X1-2     .      .      .     0 A1:1-2|A2:1-2|XA2: A1:P1|A2:P-1"
 	" CX1-4:P1 CX3-4:P1 .     .  X1-2:X1-4   .      .      .     0 A1:1-2|A2:3-4|XA2:3-4 A1:P1|A2:P1"

base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del
  2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
  2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
@ 2026-11-07 12:00 ` Ridong Chen
  2026-11-09 15:48   ` Tejun Heo
  2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
  2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
  3 siblings, 1 reply; 17+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
	Sashiko, Guopeng Zhang, Ridong Chen

From: Chen Ridong <chenridong@huawei.com>

Add a check for an empty cpumask at the start of partition_xcpus_add()
and partition_xcpus_del(). This allows the functions to return early,
avoiding unnecessary computation when there is no work to be done. With
these changes, partition_xcpus_add() and partition_xcpus_del() can be
called even if xcpus is empty. The caller no longer needs to check whether
xcpus is empty, eliminating many conditional statements.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
Reviewed-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Reviewed-by: Waiman Long <longman@redhat.com>
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 kernel/cgroup/cpuset.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index cf38e8708ddd..85e5b529d144 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1315,10 +1315,13 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
 {
 	WARN_ON_ONCE(new_prs < 0);
 	lockdep_assert_held(&callback_lock);
+
+	if (cpumask_empty(xcpus))
+		return;
+
 	if (!parent)
 		parent = &top_cpuset;
 
-
 	if (parent == &top_cpuset)
 		cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
 
@@ -1342,6 +1345,10 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
 {
 	WARN_ON_ONCE(old_prs < 0);
 	lockdep_assert_held(&callback_lock);
+
+	if (cpumask_empty(xcpus))
+		return;
+
 	if (!parent)
 		parent = &top_cpuset;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
  2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
  2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
@ 2026-11-07 12:00 ` Ridong Chen
  2026-11-09 14:51   ` Waiman Long
  2026-11-09 15:20   ` Guopeng Zhang
  2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
  3 siblings, 2 replies; 17+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
	Sashiko, Guopeng Zhang, Ridong Chen

From: Ridong Chen <chenridong@xiaomi.com>

partition_xcpus_del() only updates isolated_cpus when the partition that
gives back the CPUs has a different state than its parent. CPUs that were
further distributed to descendant isolated partitions are not taken into
account, so isolated_cpus_release() has to be called afterwards in
several places to walk up the ancestors and fix isolated_cpus.

Make partition_xcpus_del() always set the isolation state of the given
back CPUs to the state of the parent that receives them. As the
effective_xcpus of a partition covers the CPUs distributed to its
descendants, the isolation state of those CPUs is settled at the same
time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().

compute_partition_effective_cpumask() invalidates a child partition
without going through partition_xcpus_del(), so do the same there: the
CPUs of the invalidated child take the isolation state of its parent.

As the parent may be shrunk and its effective_xcpus not updated yet,
only do so for the CPUs the parent still owns. This is to keep the
isolation state correct for now, the in-place invalidation here will be
removed later in the partition rework.

isolated_cpus_update() assumes the old and new states are different,
which no longer holds now that partition_xcpus_del() proceeds regardless
of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
it simply sets the isolation state of the given CPUs, and use it
everywhere. The state check in partition_xcpus_add() is dropped as well,
as setting the state is a no-op when the states are the same.

isolated_cpus_release() is no longer needed and is removed.

Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
 1 file changed, 19 insertions(+), 79 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 85e5b529d144..fc9de6e16ff5 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
 }
 
 /*
- * isolated_cpus_update - Update the isolated_cpus mask
- * @old_prs: old partition_root_state
- * @new_prs: new partition_root_state
- * @xcpus: exclusive CPUs with state change
+ * isolated_cpus_update - Set the isolation state of @xcpus
+ * @new_prs: partition root state that @xcpus will belong to
+ * @xcpus: exclusive CPUs whose isolation state may change
+ *
+ * CPUs isolated at boot time are kept isolated.
  */
-static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
+static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
 {
 	bool updated = false;
 	int cpu;
 
-	WARN_ON_ONCE(old_prs == new_prs);
 	lockdep_assert_held(&callback_lock);
 	lockdep_assert_held(&cpuset_mutex);
 	for_each_cpu(cpu, xcpus)
@@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
 		update_housekeeping = true;
 }
 
-/*
- * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
- * @cs: the current cpuset
- * @xcpus: the set of effective xcpus to be considered for removal
- *
- * This function is called when an isolated partition is invalidated or when
- * some of its effective xcpus are to be removed from isolated_cpus due to
- * changes in parent cpuset. The given @xcpus must be modifiable as the function
- * will update its value to determine the right set of CPUs to be removed.
- */
-static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
-{
-	struct cpuset *ancestor = parent_cs(cs);
-	struct cpuset *iancestor = NULL;
-
-	/*
-	 * Search the furthest isolated partition root ancestor available
-	 * until top_cpuset or a remote partition root is hit.
-	 */
-	for (; ancestor; ancestor = parent_cs(ancestor)) {
-		if (ancestor->partition_root_state == PRS_ISOLATED)
-			iancestor = ancestor;
-		if (is_remote_partition(ancestor))
-			break;
-	}
-
-	/*
-	 * If a furthest isolated partition acnestor is found, use it to
-	 * determine which of the CPUs in @xcpus should be removed from
-	 * isolated_cpus. Otherwise, all of them will be removed.
-	 */
-	if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
-		return; /* No CPU should be removed from isolated_cpus */
-
-	spin_lock_irq(&callback_lock);
-	isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
-	spin_unlock_irq(&callback_lock);
-}
-
 /*
  * partition_xcpus_add - Add new exclusive CPUs to partition
  * @new_prs: new partition_root_state
@@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
 	if (parent == &top_cpuset)
 		cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
 
-	if (new_prs != parent->partition_root_state)
-		isolated_cpus_update(parent->partition_root_state, new_prs,
-				     xcpus);
+	isolated_cpus_update(new_prs, xcpus);
 
 	cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
 }
@@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
 	if (parent == &top_cpuset)
 		cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
 
-	if (old_prs != parent->partition_root_state)
-		isolated_cpus_update(old_prs, parent->partition_root_state,
-				     xcpus);
+	/* CPUs given back, including those of descendants, take parent's state */
+	isolated_cpus_update(parent->partition_root_state, xcpus);
 
 	cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
 	cpumask_and(parent->effective_cpus, parent->effective_cpus, cpu_active_mask);
@@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 	struct cgroup_subsys_state *css;
 	struct cpuset *child;
 	bool populated = partition_is_populated(cs, NULL);
+	static cpumask_t xcpus;		/* Protected by cpuset_mutex */
 
 	/*
 	 * Check child partition roots to see if they should be invalidated
@@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 			int parent_prs = cs->partition_root_state;
 
 			/*
-			 * Invalidate child partition and update isolated_cpus
-			 * if necessary.
+			 * Invalidate child partition. Its CPUs still owned by
+			 * cs take the isolation state of cs. effective_xcpus
+			 * of cs may not be updated yet, so recompute it.
 			 */
 			spin_lock_irq(&callback_lock);
 			WRITE_ONCE(child->prs_err, PERR_NOCPUS);
-			if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
-				isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
-						     child->effective_xcpus);
+			compute_excpus(cs, &xcpus);
+			cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
+			isolated_cpus_update(parent_prs, &xcpus);
 			child->partition_root_state = -old_prs;
 			spin_unlock_irq(&callback_lock);
 			notify_partition_change(child, old_prs);
@@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		 *
 		 * A partition error happens when parent has tasks and all
 		 * its effective CPUs will have to be distributed out.
-		 *
-		 * For an isolated partition with a shrunk root partition parent
-		 * we may need to return some or all of its isolated CPUs.
 		 */
 		nocpu |= cpumask_empty(tmp->delmask);
-		if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
-			cpumask_clear(tmp->addmask);
-			if (nocpu)
-				cpumask_copy(tmp->addmask, cs->effective_xcpus);
-			else if (!cpumask_subset(cs->effective_xcpus, parent->effective_xcpus))
-				cpumask_andnot(tmp->addmask, cs->effective_xcpus,
-							     parent->effective_xcpus);
-			if (!cpumask_empty(tmp->addmask))
-				isolated_cpus_release(cs, tmp->addmask);
-			/* Expansion of parent's effective_xcpus is handled below */
-		}
 		if (nocpu) {
 			part_error = PERR_NOCPUS;
 			if (is_partition_valid(cs))
@@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
 				/*
 				 * When parent is not a valid partition root,
 				 * child partition roots become invalid too.
-				 *
-				 * For a to be invalidated isolated partition,
-				 * call return_isolated_cpus() to remove the
-				 * relevant CPUs from isolated_cpus.
+				 * The isolation state of their CPUs has been
+				 * settled when the parent gave them back.
 				 */
 				if (is_partition_valid(cp))
 					new_prs = -old_prs;
 
-				if (old_prs == PRS_ISOLATED) {
-					cpumask_copy(tmp->delmask, cp->effective_xcpus);
-					isolated_cpus_release(cp, tmp->delmask);
-				}
 				WRITE_ONCE(cp->prs_err,
 					   is_partition_invalid(parent)
 					   ? PERR_INVPARENT : PERR_NOTPART);
@@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	if (!is_partition_valid(cs))
 		reset_partition_data(cs);
 	else if (isolcpus_updated)
-		isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
+		isolated_cpus_update(new_prs, cs->effective_xcpus);
 	spin_unlock_irq(&callback_lock);
 
 	/* Force update if partition is disabled & update effective_xcpus */
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v3 4/4] cpuset: generalize the validate_partition() interface
  2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
                   ` (2 preceding siblings ...)
  2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
@ 2026-11-07 12:01 ` Ridong Chen
  2026-11-09 14:52   ` Waiman Long
  3 siblings, 1 reply; 17+ messages in thread
From: Ridong Chen @ 2026-11-07 12:01 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
	Sashiko, Guopeng Zhang, Ridong Chen

From: Chen Ridong <chenridong@huawei.com>

Refactor validate_partition() to take the parent partition, the new
partition root state, the new effective exclusive CPUs and the CPUs to
be deleted, instead of a trial cpuset. A NULL parent stands for a
remote partition, whose CPUs come from top_cpuset. This makes the
interface flexible enough to handle both local and remote partition
validation.

The no CPU left for tasks check now uses the given parent instead of
parent_cs(cs), and top_cpuset for a remote partition, as that is where
its CPUs are taken from. The cgroup parent of a remote partition does
not give it any CPU.

The housekeeping conflict check is folded into validate_partition(). It
is skipped when the parent is not a valid partition root, as @cs can't
be a valid local partition in that case.

Use the new interface for local partition enablement in
update_parent_effective_cpumask() and in partition_cpus_change(),
replacing the open-coded checks there. The order of the checks is now
the same for both paths: empty CPUs, housekeeping conflict, then no CPU
left for tasks.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 kernel/cgroup/cpuset.c | 119 ++++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 60 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index fc9de6e16ff5..0ec20412b3ce 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1414,6 +1414,48 @@ static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
 	return res;
 }
 
+/**
+ * validate_partition - Validate a cpuset partition configuration
+ * @cs: The cpuset to validate
+ * @parent: Parent partition of @cs, NULL for remote partition
+ * @new_prs: The proposed new partition root state
+ * @new_excpus: The new effective exclusive CPUs mask to validate
+ * @del: exclusive CPUs to be deleted from @cs, can be NULL
+ *
+ * A remote partition gets its CPUs from top_cpuset, so top_cpuset is used
+ * as the parent when @parent is NULL.
+ *
+ * The housekeeping check is skipped if @parent is not a valid partition
+ * root as @cs can't be a valid local partition in that case.
+ *
+ * Return: PRS error code (0 if valid, non-zero error code if invalid)
+ */
+static enum prs_errcode validate_partition(struct cpuset *cs,
+					   struct cpuset *parent, int new_prs,
+					   struct cpumask *new_excpus,
+					   struct cpumask *del)
+{
+	int parent_prs;
+
+	if (new_prs == PRS_MEMBER)
+		return PERR_NONE;
+
+	if (cpumask_empty(new_excpus))
+		return PERR_INVCPUS;
+
+	if (!parent)
+		parent = &top_cpuset;
+	parent_prs = parent->partition_root_state;
+	if (parent_prs > 0 &&
+	    prstate_housekeeping_conflict(new_prs, parent_prs, new_excpus, del))
+		return PERR_HKEEPING;
+
+	if (tasks_nocpu_error(parent, cs, new_excpus))
+		return PERR_NOCPUS;
+
+	return PERR_NONE;
+}
+
 /*
  * cpuset_update_sd_hk_unlock - Rebuild sched domains, update HK & unlock
  *
@@ -1934,18 +1976,10 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
 		new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
 
-		/*
-		 * Enabling partition root is not allowed if its
-		 * effective_xcpus is empty.
-		 */
-		if (cpumask_empty(tmp->delmask))
-			return PERR_INVCPUS;
-
-		if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask, NULL))
-			return PERR_HKEEPING;
-
-		if (nocpu)
-			return PERR_NOCPUS;
+		part_error = validate_partition(cs, parent, new_prs,
+						tmp->delmask, NULL);
+		if (part_error)
+			return part_error;
 
 		/*
 		 * This function will only be called when all the preliminary
@@ -2424,32 +2458,6 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
 	return 0;
 }
 
-/**
- * validate_partition - Validate a cpuset partition configuration
- * @cs: The cpuset to validate
- * @trialcs: The trial cpuset containing proposed configuration changes
- *
- * If any validation check fails, the appropriate error code is set in the
- * cpuset's prs_err field.
- *
- * Return: PRS error code (0 if valid, non-zero error code if invalid)
- */
-static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *trialcs)
-{
-	struct cpuset *parent = parent_cs(cs);
-
-	if (cs_is_member(trialcs))
-		return PERR_NONE;
-
-	if (cpumask_empty(trialcs->effective_xcpus))
-		return PERR_INVCPUS;
-
-	if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
-		return PERR_NOCPUS;
-
-	return PERR_NONE;
-}
-
 /**
  * partition_cpus_change - Handle partition state changes due to CPU mask updates
  * @cs: The target cpuset being modified
@@ -2463,33 +2471,24 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
 				  struct tmpmasks *tmp)
 {
 	enum prs_errcode prs_err;
-	int parent_prs = is_remote_partition(cs)
-		       ? PRS_ROOT : parent_cs(cs)->partition_root_state;
+	struct cpuset *parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
 
 	if (cs_is_member(cs))
 		return;
 
-	prs_err = validate_partition(cs, trialcs);
-	if (!prs_err && (parent_prs > 0) &&
-	    !cpumask_empty(trialcs->effective_xcpus)) {
-		int prs = cs->partition_root_state;
-
-		/*
-		 * Check for housekeeping conflict by passing the whole
-		 * effective_xcpus as well as those CPUs that are to be
-		 * removed from cs as delmask.
-		 */
-		if (prs > 0)
-			cpumask_andnot(tmp->delmask, cs->effective_xcpus,
-				       trialcs->effective_xcpus);
-		else
-			cpumask_clear(tmp->delmask);
-
-		if (prstate_housekeeping_conflict(prs, parent_prs,
-						  trialcs->effective_xcpus, tmp->delmask))
-			prs_err = PERR_HKEEPING;
-	}
+	/*
+	 * Check for housekeeping conflict by passing the whole
+	 * effective_xcpus as well as those CPUs that are to be
+	 * removed from cs as delmask.
+	 */
+	if (is_partition_valid(cs))
+		cpumask_andnot(tmp->delmask, cs->effective_xcpus,
+			       trialcs->effective_xcpus);
+	else
+		cpumask_clear(tmp->delmask);
 
+	prs_err = validate_partition(cs, parent, trialcs->partition_root_state,
+				     trialcs->effective_xcpus, tmp->delmask);
 	if (prs_err) {
 		WRITE_ONCE(cs->prs_err, prs_err);
 		trialcs->prs_err = prs_err;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
@ 2026-11-09 14:51   ` Waiman Long
  2026-11-10  3:14     ` Ridong Chen
  2026-11-09 15:20   ` Guopeng Zhang
  1 sibling, 1 reply; 17+ messages in thread
From: Waiman Long @ 2026-11-09 14:51 UTC (permalink / raw)
  To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen

On 11/7/26 7:00 AM, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
>
> partition_xcpus_del() only updates isolated_cpus when the partition that
> gives back the CPUs has a different state than its parent. CPUs that were
> further distributed to descendant isolated partitions are not taken into
> account, so isolated_cpus_release() has to be called afterwards in
> several places to walk up the ancestors and fix isolated_cpus.
>
> Make partition_xcpus_del() always set the isolation state of the given
> back CPUs to the state of the parent that receives them. As the
> effective_xcpus of a partition covers the CPUs distributed to its
> descendants, the isolation state of those CPUs is settled at the same
> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>
> compute_partition_effective_cpumask() invalidates a child partition
> without going through partition_xcpus_del(), so do the same there: the
> CPUs of the invalidated child take the isolation state of its parent.
>
> As the parent may be shrunk and its effective_xcpus not updated yet,
> only do so for the CPUs the parent still owns. This is to keep the
> isolation state correct for now, the in-place invalidation here will be
> removed later in the partition rework.
>
> isolated_cpus_update() assumes the old and new states are different,
> which no longer holds now that partition_xcpus_del() proceeds regardless
> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
> it simply sets the isolation state of the given CPUs, and use it
> everywhere. The state check in partition_xcpus_add() is dropped as well,
> as setting the state is a no-op when the states are the same.
>
> isolated_cpus_release() is no longer needed and is removed.
>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>   kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
>   1 file changed, 19 insertions(+), 79 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 85e5b529d144..fc9de6e16ff5 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
>   }
>   
>   /*
> - * isolated_cpus_update - Update the isolated_cpus mask
> - * @old_prs: old partition_root_state
> - * @new_prs: new partition_root_state
> - * @xcpus: exclusive CPUs with state change
> + * isolated_cpus_update - Set the isolation state of @xcpus
> + * @new_prs: partition root state that @xcpus will belong to
> + * @xcpus: exclusive CPUs whose isolation state may change
> + *
> + * CPUs isolated at boot time are kept isolated.
>    */
> -static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>   {
>   	bool updated = false;
>   	int cpu;
>   
> -	WARN_ON_ONCE(old_prs == new_prs);
>   	lockdep_assert_held(&callback_lock);
>   	lockdep_assert_held(&cpuset_mutex);
>   	for_each_cpu(cpu, xcpus)
> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>   		update_housekeeping = true;
>   }
>   
> -/*
> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
> - * @cs: the current cpuset
> - * @xcpus: the set of effective xcpus to be considered for removal
> - *
> - * This function is called when an isolated partition is invalidated or when
> - * some of its effective xcpus are to be removed from isolated_cpus due to
> - * changes in parent cpuset. The given @xcpus must be modifiable as the function
> - * will update its value to determine the right set of CPUs to be removed.
> - */
> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
> -{
> -	struct cpuset *ancestor = parent_cs(cs);
> -	struct cpuset *iancestor = NULL;
> -
> -	/*
> -	 * Search the furthest isolated partition root ancestor available
> -	 * until top_cpuset or a remote partition root is hit.
> -	 */
> -	for (; ancestor; ancestor = parent_cs(ancestor)) {
> -		if (ancestor->partition_root_state == PRS_ISOLATED)
> -			iancestor = ancestor;
> -		if (is_remote_partition(ancestor))
> -			break;
> -	}
> -
> -	/*
> -	 * If a furthest isolated partition acnestor is found, use it to
> -	 * determine which of the CPUs in @xcpus should be removed from
> -	 * isolated_cpus. Otherwise, all of them will be removed.
> -	 */
> -	if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
> -		return; /* No CPU should be removed from isolated_cpus */
> -
> -	spin_lock_irq(&callback_lock);
> -	isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
> -	spin_unlock_irq(&callback_lock);
> -}
> -
>   /*
>    * partition_xcpus_add - Add new exclusive CPUs to partition
>    * @new_prs: new partition_root_state
> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
>   	if (parent == &top_cpuset)
>   		cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>   
> -	if (new_prs != parent->partition_root_state)
> -		isolated_cpus_update(parent->partition_root_state, new_prs,
> -				     xcpus);
> +	isolated_cpus_update(new_prs, xcpus);
>   
>   	cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
>   }
> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
>   	if (parent == &top_cpuset)
>   		cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>   
> -	if (old_prs != parent->partition_root_state)
> -		isolated_cpus_update(old_prs, parent->partition_root_state,
> -				     xcpus);
> +	/* CPUs given back, including those of descendants, take parent's state */
> +	isolated_cpus_update(parent->partition_root_state, xcpus);
>   
>   	cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
>   	cpumask_and(parent->effective_cpus, parent->effective_cpus, cpu_active_mask);
> @@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   	struct cgroup_subsys_state *css;
>   	struct cpuset *child;
>   	bool populated = partition_is_populated(cs, NULL);
> +	static cpumask_t xcpus;		/* Protected by cpuset_mutex */
>   
>   	/*
>   	 * Check child partition roots to see if they should be invalidated
> @@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   			int parent_prs = cs->partition_root_state;
>   
>   			/*
> -			 * Invalidate child partition and update isolated_cpus
> -			 * if necessary.
> +			 * Invalidate child partition. Its CPUs still owned by
> +			 * cs take the isolation state of cs. effective_xcpus
> +			 * of cs may not be updated yet, so recompute it.
>   			 */
>   			spin_lock_irq(&callback_lock);
>   			WRITE_ONCE(child->prs_err, PERR_NOCPUS);
> -			if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
> -				isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
> -						     child->effective_xcpus);
> +			compute_excpus(cs, &xcpus);
> +			cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
> +			isolated_cpus_update(parent_prs, &xcpus);
>   			child->partition_root_state = -old_prs;
>   			spin_unlock_irq(&callback_lock);
>   			notify_partition_change(child, old_prs);
> @@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   		 *
>   		 * A partition error happens when parent has tasks and all
>   		 * its effective CPUs will have to be distributed out.
> -		 *
> -		 * For an isolated partition with a shrunk root partition parent
> -		 * we may need to return some or all of its isolated CPUs.
>   		 */
>   		nocpu |= cpumask_empty(tmp->delmask);
> -		if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
> -			cpumask_clear(tmp->addmask);
> -			if (nocpu)
> -				cpumask_copy(tmp->addmask, cs->effective_xcpus);
> -			else if (!cpumask_subset(cs->effective_xcpus, parent->effective_xcpus))
> -				cpumask_andnot(tmp->addmask, cs->effective_xcpus,
> -							     parent->effective_xcpus);
> -			if (!cpumask_empty(tmp->addmask))
> -				isolated_cpus_release(cs, tmp->addmask);
> -			/* Expansion of parent's effective_xcpus is handled below */
> -		}
>   		if (nocpu) {
>   			part_error = PERR_NOCPUS;
>   			if (is_partition_valid(cs))
> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
>   				/*
>   				 * When parent is not a valid partition root,
>   				 * child partition roots become invalid too.
> -				 *
> -				 * For a to be invalidated isolated partition,
> -				 * call return_isolated_cpus() to remove the
> -				 * relevant CPUs from isolated_cpus.
> +				 * The isolation state of their CPUs has been
> +				 * settled when the parent gave them back.
>   				 */
>   				if (is_partition_valid(cp))
>   					new_prs = -old_prs;
>   
> -				if (old_prs == PRS_ISOLATED) {
> -					cpumask_copy(tmp->delmask, cp->effective_xcpus);
> -					isolated_cpus_release(cp, tmp->delmask);
> -				}
>   				WRITE_ONCE(cp->prs_err,
>   					   is_partition_invalid(parent)
>   					   ? PERR_INVPARENT : PERR_NOTPART);
> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   	if (!is_partition_valid(cs))
>   		reset_partition_data(cs);
>   	else if (isolcpus_updated)
> -		isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
> +		isolated_cpus_update(new_prs, cs->effective_xcpus);
>   	spin_unlock_irq(&callback_lock);
>   
>   	/* Force update if partition is disabled & update effective_xcpus */

The code change looks good to me. However this is not just a code 
restructuring, it also includes a bug fix. I wonder if we should clarify 
that and maybe add a Fixes tag.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 4/4] cpuset: generalize the validate_partition() interface
  2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
@ 2026-11-09 14:52   ` Waiman Long
  2026-11-10  6:36     ` Ridong Chen
  0 siblings, 1 reply; 17+ messages in thread
From: Waiman Long @ 2026-11-09 14:52 UTC (permalink / raw)
  To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen

On 11/7/26 7:01 AM, Ridong Chen wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Refactor validate_partition() to take the parent partition, the new
> partition root state, the new effective exclusive CPUs and the CPUs to
> be deleted, instead of a trial cpuset. A NULL parent stands for a
> remote partition, whose CPUs come from top_cpuset. This makes the
> interface flexible enough to handle both local and remote partition
> validation.
>
> The no CPU left for tasks check now uses the given parent instead of
> parent_cs(cs), and top_cpuset for a remote partition, as that is where
> its CPUs are taken from. The cgroup parent of a remote partition does
> not give it any CPU.
>
> The housekeeping conflict check is folded into validate_partition(). It
> is skipped when the parent is not a valid partition root, as @cs can't
> be a valid local partition in that case.
>
> Use the new interface for local partition enablement in
> update_parent_effective_cpumask() and in partition_cpus_change(),
> replacing the open-coded checks there. The order of the checks is now
> the same for both paths: empty CPUs, housekeeping conflict, then no CPU
> left for tasks.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>   kernel/cgroup/cpuset.c | 119 ++++++++++++++++++++---------------------
>   1 file changed, 59 insertions(+), 60 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index fc9de6e16ff5..0ec20412b3ce 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1414,6 +1414,48 @@ static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
>   	return res;
>   }
>   
> +/**
> + * validate_partition - Validate a cpuset partition configuration
> + * @cs: The cpuset to validate
> + * @parent: Parent partition of @cs, NULL for remote partition
> + * @new_prs: The proposed new partition root state
> + * @new_excpus: The new effective exclusive CPUs mask to validate
> + * @del: exclusive CPUs to be deleted from @cs, can be NULL
> + *
> + * A remote partition gets its CPUs from top_cpuset, so top_cpuset is used
> + * as the parent when @parent is NULL.
> + *
> + * The housekeeping check is skipped if @parent is not a valid partition
> + * root as @cs can't be a valid local partition in that case.
> + *
> + * Return: PRS error code (0 if valid, non-zero error code if invalid)
> + */
> +static enum prs_errcode validate_partition(struct cpuset *cs,
> +					   struct cpuset *parent, int new_prs,
> +					   struct cpumask *new_excpus,
> +					   struct cpumask *del)
> +{
> +	int parent_prs;
> +
> +	if (new_prs == PRS_MEMBER)
> +		return PERR_NONE;
> +
> +	if (cpumask_empty(new_excpus))
> +		return PERR_INVCPUS;
> +
> +	if (!parent)
> +		parent = &top_cpuset;
> +	parent_prs = parent->partition_root_state;
> +	if (parent_prs > 0 &&
> +	    prstate_housekeeping_conflict(new_prs, parent_prs, new_excpus, del))
> +		return PERR_HKEEPING;
> +
> +	if (tasks_nocpu_error(parent, cs, new_excpus))
> +		return PERR_NOCPUS;
> +
> +	return PERR_NONE;
> +}
> +
>   /*
>    * cpuset_update_sd_hk_unlock - Rebuild sched domains, update HK & unlock
>    *
> @@ -1934,18 +1976,10 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   	if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
>   		new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
>   
> -		/*
> -		 * Enabling partition root is not allowed if its
> -		 * effective_xcpus is empty.
> -		 */
> -		if (cpumask_empty(tmp->delmask))
> -			return PERR_INVCPUS;
> -
> -		if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask, NULL))
> -			return PERR_HKEEPING;
> -
> -		if (nocpu)
> -			return PERR_NOCPUS;
> +		part_error = validate_partition(cs, parent, new_prs,
> +						tmp->delmask, NULL);
> +		if (part_error)
> +			return part_error;
>   
>   		/*
>   		 * This function will only be called when all the preliminary
> @@ -2424,32 +2458,6 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
>   	return 0;
>   }
>   
> -/**
> - * validate_partition - Validate a cpuset partition configuration
> - * @cs: The cpuset to validate
> - * @trialcs: The trial cpuset containing proposed configuration changes
> - *
> - * If any validation check fails, the appropriate error code is set in the
> - * cpuset's prs_err field.
> - *
> - * Return: PRS error code (0 if valid, non-zero error code if invalid)
> - */
> -static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *trialcs)
> -{
> -	struct cpuset *parent = parent_cs(cs);
> -
> -	if (cs_is_member(trialcs))
> -		return PERR_NONE;
> -
> -	if (cpumask_empty(trialcs->effective_xcpus))
> -		return PERR_INVCPUS;
> -
> -	if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
> -		return PERR_NOCPUS;
> -
> -	return PERR_NONE;
> -}
> -
>   /**
>    * partition_cpus_change - Handle partition state changes due to CPU mask updates
>    * @cs: The target cpuset being modified
> @@ -2463,33 +2471,24 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>   				  struct tmpmasks *tmp)
>   {
>   	enum prs_errcode prs_err;
> -	int parent_prs = is_remote_partition(cs)
> -		       ? PRS_ROOT : parent_cs(cs)->partition_root_state;
> +	struct cpuset *parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>   
>   	if (cs_is_member(cs))
>   		return;
>   
> -	prs_err = validate_partition(cs, trialcs);
> -	if (!prs_err && (parent_prs > 0) &&
> -	    !cpumask_empty(trialcs->effective_xcpus)) {
> -		int prs = cs->partition_root_state;
> -
> -		/*
> -		 * Check for housekeeping conflict by passing the whole
> -		 * effective_xcpus as well as those CPUs that are to be
> -		 * removed from cs as delmask.
> -		 */
> -		if (prs > 0)
> -			cpumask_andnot(tmp->delmask, cs->effective_xcpus,
> -				       trialcs->effective_xcpus);
> -		else
> -			cpumask_clear(tmp->delmask);
> -
> -		if (prstate_housekeeping_conflict(prs, parent_prs,
> -						  trialcs->effective_xcpus, tmp->delmask))
> -			prs_err = PERR_HKEEPING;
> -	}
> +	/*
> +	 * Check for housekeeping conflict by passing the whole
> +	 * effective_xcpus as well as those CPUs that are to be
> +	 * removed from cs as delmask.
> +	 */
> +	if (is_partition_valid(cs))
> +		cpumask_andnot(tmp->delmask, cs->effective_xcpus,
> +			       trialcs->effective_xcpus);
> +	else
> +		cpumask_clear(tmp->delmask);
>   
> +	prs_err = validate_partition(cs, parent, trialcs->partition_root_state,
> +				     trialcs->effective_xcpus, tmp->delmask);
>   	if (prs_err) {
>   		WRITE_ONCE(cs->prs_err, prs_err);
>   		trialcs->prs_err = prs_err;

Again, this code change looks good to me. Similar to previous patch, it 
includes both code restructuring and bug fixes.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid
  2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
@ 2026-11-09 14:59   ` Guopeng Zhang
  0 siblings, 0 replies; 17+ messages in thread
From: Guopeng Zhang @ 2026-11-09 14:59 UTC (permalink / raw)
  To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



在 2026/11/7 20:00, Ridong Chen 写道:
> From: Ridong Chen <chenridong@xiaomi.com>
> 
> When cpuset.cpus or cpuset.cpus.exclusive of a partition root is
> changed, update_cpumask() and update_exclusive_cpumask() copy the new
> effective_xcpus computed for the trial cpuset into the cpuset, and call
> reset_partition_data() only if the partition was valid before and is
> invalid now:
> 
> 	if ((old_prs > 0) && !is_partition_valid(cs))
> 		reset_partition_data(cs);
> 
> An invalid partition root (old_prs < 0) that stays invalid is not
> covered. Its effective_xcpus is then left with the trial value, which
> is computed from user_xcpus() and so falls back to cpuset.cpus when
> cpuset.cpus.exclusive is cleared. As an invalid partition root does not
> own any exclusive CPUs, and rm_siblings_excl_cpus() treats a non-empty
> effective_xcpus of a sibling without cpuset.cpus.exclusive as owned,
> those CPUs can then no longer be claimed by a sibling:
> 
>   # A1: cpuset.cpus=2-3, cpuset.cpus.exclusive=2-3, root, with tasks
>   # offline CPUs 2-3 -> A1 becomes invalid
>   echo > A1/cpuset.cpus.exclusive
>   cat A1/cpuset.cpus.exclusive.effective    # 2-3, expected empty
>   echo 3 > B1/cpuset.cpus.exclusive         # -EINVAL
> 
> Reset the partition data whenever the cpuset is a partition root, valid
> or not, and ends up invalid. As effective_xcpus may be cleared by that,
> also force update_cpumasks_hier() to update the subtree, so that the
> effective_xcpus of the descendants doesn't keep the released CPUs. Add
> test cases for it.
> 
> Fixes: de9f15e21c55 ("cpuset: use parse_cpulist for setting cpus.exclusive")
> Fixes: c5866c9a007d ("cpuset: refactor CPU mask buffer parsing logic")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20261101114206.2434398-1-ridong.chen%40linux.dev?part=15
> Reviewed-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>  kernel/cgroup/cpuset.c                            | 10 ++++++++--
>  tools/testing/selftests/cgroup/test_cpuset_prs.sh |  6 ++++++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 3052413e41e7..cf38e8708ddd 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2607,8 +2607,11 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>  	spin_lock_irq(&callback_lock);
>  	cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
>  	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
> -	if ((old_prs > 0) && !is_partition_valid(cs))
> +	if (old_prs && !is_partition_valid(cs)) {
>  		reset_partition_data(cs);
> +		/* effective_xcpus may be cleared, update the subtree too */
> +		force = true;
> +	}
>  	spin_unlock_irq(&callback_lock);
>  
>  	/* effective_cpus/effective_xcpus will be updated here */
> @@ -2672,8 +2675,11 @@ static int update_exclusive_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>  	spin_lock_irq(&callback_lock);
>  	cpumask_copy(cs->exclusive_cpus, trialcs->exclusive_cpus);
>  	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
> -	if ((old_prs > 0) && !is_partition_valid(cs))
> +	if (old_prs && !is_partition_valid(cs)) {
>  		reset_partition_data(cs);
> +		/* effective_xcpus may be cleared, update the subtree too */
> +		force = true;
> +	}
>  	spin_unlock_irq(&callback_lock);
>  
>  	/*
> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index bdca6c471f87..32e1e44fa0b1 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -434,6 +434,12 @@ TEST_MATRIX=(
>  	# cpuset.cpus is allowed, but those extra CPUs are ignored.
>  	"  C2-3:P1   C3:P1  .      .      .     C2-4    .      .     0 A1:|A2:2-3 A1:P1|A2:P1"
>  
> +	# Clearing cpuset.cpus.exclusive of an invalid partition releases its
> +	# exclusive CPUs, including those of its descendants, so a sibling
> +	# can claim them.
> +	" CX2-3:P1    .     .     C3  T:O2=0:O3=0:X  .     .      X3   0 XA1:|XB1:3 A1:P-1"
> +	" CX2-3:P1   X3     .     C3  T:O2=0:O3=0:X  .     .      X3   0 XA1:|XA2:|XB1:3 A1:P-1"
> +
>  	# A child cannot recover until its exclusive CPUs are available again.
>  	" CX1-4:P1 CX3-4:P1 .     .     X1-2     .      .      .     0 A1:1-2|A2:1-2|XA2: A1:P1|A2:P-1"
>  	" CX1-4:P1 CX3-4:P1 .     .  X1-2:X1-4   .      .      .     0 A1:1-2|A2:3-4|XA2:3-4 A1:P1|A2:P1"
> 
> base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4

Reviewed-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Tested-by: Guopeng Zhang <zhangguopeng@kylinos.cn>

Thanks,
Guopeng

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
  2026-11-09 14:51   ` Waiman Long
@ 2026-11-09 15:20   ` Guopeng Zhang
  2026-11-10  6:24     ` Ridong Chen
  1 sibling, 1 reply; 17+ messages in thread
From: Guopeng Zhang @ 2026-11-09 15:20 UTC (permalink / raw)
  To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



在 2026/11/7 20:00, Ridong Chen 写道:
> From: Ridong Chen <chenridong@xiaomi.com>
> 
> partition_xcpus_del() only updates isolated_cpus when the partition that
> gives back the CPUs has a different state than its parent. CPUs that were
> further distributed to descendant isolated partitions are not taken into
> account, so isolated_cpus_release() has to be called afterwards in
> several places to walk up the ancestors and fix isolated_cpus.
> 
> Make partition_xcpus_del() always set the isolation state of the given
> back CPUs to the state of the parent that receives them. As the
> effective_xcpus of a partition covers the CPUs distributed to its
> descendants, the isolation state of those CPUs is settled at the same
> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
> 
> compute_partition_effective_cpumask() invalidates a child partition
> without going through partition_xcpus_del(), so do the same there: the
> CPUs of the invalidated child take the isolation state of its parent.
> 
> As the parent may be shrunk and its effective_xcpus not updated yet,
> only do so for the CPUs the parent still owns. This is to keep the
> isolation state correct for now, the in-place invalidation here will be
> removed later in the partition rework.
> 
> isolated_cpus_update() assumes the old and new states are different,
> which no longer holds now that partition_xcpus_del() proceeds regardless
> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
> it simply sets the isolation state of the given CPUs, and use it
> everywhere. The state check in partition_xcpus_add() is dropped as well,
> as setting the state is a no-op when the states are the same.
> 
> isolated_cpus_release() is no longer needed and is removed.
> 

Hi Ridong,

Would it be worth briefly mentioning in the commit message that this patch also fixes the issue described here?

https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/

Thanks,
Guopeng


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del
  2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
@ 2026-11-09 15:48   ` Tejun Heo
  0 siblings, 0 replies; 17+ messages in thread
From: Tejun Heo @ 2026-11-09 15:48 UTC (permalink / raw)
  To: Ridong Chen
  Cc: Waiman Long, Johannes Weiner, Michal Koutný, Shuah Khan,
	Chen Ridong, Ridong Chen, Guopeng Zhang, Guopeng Zhang, Sashiko,
	cgroups, linux-kernel, linux-kselftest

Applied 1-2 to cgroup/for-7.4-fixes with the following tags added to
patch 1:

  Reviewed-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
  Tested-by: Guopeng Zhang <zhangguopeng@kylinos.cn>

Thanks.

--
tejun

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-09 14:51   ` Waiman Long
@ 2026-11-10  3:14     ` Ridong Chen
  2026-11-10  3:32       ` Waiman Long
  2026-11-10  8:18       ` Guopeng Zhang
  0 siblings, 2 replies; 17+ messages in thread
From: Ridong Chen @ 2026-11-10  3:14 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



On 11/9/2026 10:51 PM, Waiman Long wrote:
> On 11/7/26 7:00 AM, Ridong Chen wrote:
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> partition_xcpus_del() only updates isolated_cpus when the partition that
>> gives back the CPUs has a different state than its parent. CPUs that were
>> further distributed to descendant isolated partitions are not taken into
>> account, so isolated_cpus_release() has to be called afterwards in
>> several places to walk up the ancestors and fix isolated_cpus.
>>
>> Make partition_xcpus_del() always set the isolation state of the given
>> back CPUs to the state of the parent that receives them. As the
>> effective_xcpus of a partition covers the CPUs distributed to its
>> descendants, the isolation state of those CPUs is settled at the same
>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>
>> compute_partition_effective_cpumask() invalidates a child partition
>> without going through partition_xcpus_del(), so do the same there: the
>> CPUs of the invalidated child take the isolation state of its parent.
>>
>> As the parent may be shrunk and its effective_xcpus not updated yet,
>> only do so for the CPUs the parent still owns. This is to keep the
>> isolation state correct for now, the in-place invalidation here will be
>> removed later in the partition rework.
>>
>> isolated_cpus_update() assumes the old and new states are different,
>> which no longer holds now that partition_xcpus_del() proceeds regardless
>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>> it simply sets the isolation state of the given CPUs, and use it
>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>> as setting the state is a no-op when the states are the same.
>>
>> isolated_cpus_release() is no longer needed and is removed.
>>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
>>   1 file changed, 19 insertions(+), 79 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 85e5b529d144..fc9de6e16ff5 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
>>   }
>>   /*
>> - * isolated_cpus_update - Update the isolated_cpus mask
>> - * @old_prs: old partition_root_state
>> - * @new_prs: new partition_root_state
>> - * @xcpus: exclusive CPUs with state change
>> + * isolated_cpus_update - Set the isolation state of @xcpus
>> + * @new_prs: partition root state that @xcpus will belong to
>> + * @xcpus: exclusive CPUs whose isolation state may change
>> + *
>> + * CPUs isolated at boot time are kept isolated.
>>    */
>> -static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask 
>> *xcpus)
>> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>>   {
>>       bool updated = false;
>>       int cpu;
>> -    WARN_ON_ONCE(old_prs == new_prs);
>>       lockdep_assert_held(&callback_lock);
>>       lockdep_assert_held(&cpuset_mutex);
>>       for_each_cpu(cpu, xcpus)
>> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int 
>> new_prs, struct cpumask *xcpus
>>           update_housekeeping = true;
>>   }
>> -/*
>> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
>> - * @cs: the current cpuset
>> - * @xcpus: the set of effective xcpus to be considered for removal
>> - *
>> - * This function is called when an isolated partition is invalidated or when
>> - * some of its effective xcpus are to be removed from isolated_cpus due to
>> - * changes in parent cpuset. The given @xcpus must be modifiable as the function
>> - * will update its value to determine the right set of CPUs to be removed.
>> - */
>> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
>> -{
>> -    struct cpuset *ancestor = parent_cs(cs);
>> -    struct cpuset *iancestor = NULL;
>> -
>> -    /*
>> -     * Search the furthest isolated partition root ancestor available
>> -     * until top_cpuset or a remote partition root is hit.
>> -     */
>> -    for (; ancestor; ancestor = parent_cs(ancestor)) {
>> -        if (ancestor->partition_root_state == PRS_ISOLATED)
>> -            iancestor = ancestor;
>> -        if (is_remote_partition(ancestor))
>> -            break;
>> -    }
>> -
>> -    /*
>> -     * If a furthest isolated partition acnestor is found, use it to
>> -     * determine which of the CPUs in @xcpus should be removed from
>> -     * isolated_cpus. Otherwise, all of them will be removed.
>> -     */
>> -    if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
>> -        return; /* No CPU should be removed from isolated_cpus */
>> -
>> -    spin_lock_irq(&callback_lock);
>> -    isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
>> -    spin_unlock_irq(&callback_lock);
>> -}
>> -
>>   /*
>>    * partition_xcpus_add - Add new exclusive CPUs to partition
>>    * @new_prs: new partition_root_state
>> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct 
>> cpuset *parent,
>>       if (parent == &top_cpuset)
>>           cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>> -    if (new_prs != parent->partition_root_state)
>> -        isolated_cpus_update(parent->partition_root_state, new_prs,
>> -                     xcpus);
>> +    isolated_cpus_update(new_prs, xcpus);
>>       cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
>>   }
>> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct 
>> cpuset *parent,
>>       if (parent == &top_cpuset)
>>           cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>> -    if (old_prs != parent->partition_root_state)
>> -        isolated_cpus_update(old_prs, parent->partition_root_state,
>> -                     xcpus);
>> +    /* CPUs given back, including those of descendants, take parent's state */
>> +    isolated_cpus_update(parent->partition_root_state, xcpus);
>>       cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
>>       cpumask_and(parent->effective_cpus, parent->effective_cpus, 
>> cpu_active_mask);
>> @@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct 
>> cpuset *cs,
>>       struct cgroup_subsys_state *css;
>>       struct cpuset *child;
>>       bool populated = partition_is_populated(cs, NULL);
>> +    static cpumask_t xcpus;        /* Protected by cpuset_mutex */
>>       /*
>>        * Check child partition roots to see if they should be invalidated
>> @@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct 
>> cpuset *cs,
>>               int parent_prs = cs->partition_root_state;
>>               /*
>> -             * Invalidate child partition and update isolated_cpus
>> -             * if necessary.
>> +             * Invalidate child partition. Its CPUs still owned by
>> +             * cs take the isolation state of cs. effective_xcpus
>> +             * of cs may not be updated yet, so recompute it.
>>                */
>>               spin_lock_irq(&callback_lock);
>>               WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>> -            if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
>> -                isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
>> -                             child->effective_xcpus);
>> +            compute_excpus(cs, &xcpus);
>> +            cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
>> +            isolated_cpus_update(parent_prs, &xcpus);
>>               child->partition_root_state = -old_prs;
>>               spin_unlock_irq(&callback_lock);
>>               notify_partition_change(child, old_prs);
>> @@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct 
>> cpuset *cs, int cmd,
>>            *
>>            * A partition error happens when parent has tasks and all
>>            * its effective CPUs will have to be distributed out.
>> -         *
>> -         * For an isolated partition with a shrunk root partition parent
>> -         * we may need to return some or all of its isolated CPUs.
>>            */
>>           nocpu |= cpumask_empty(tmp->delmask);
>> -        if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
>> -            cpumask_clear(tmp->addmask);
>> -            if (nocpu)
>> -                cpumask_copy(tmp->addmask, cs->effective_xcpus);
>> -            else if (!cpumask_subset(cs->effective_xcpus, parent- 
>> >effective_xcpus))
>> -                cpumask_andnot(tmp->addmask, cs->effective_xcpus,
>> -                                 parent->effective_xcpus);
>> -            if (!cpumask_empty(tmp->addmask))
>> -                isolated_cpus_release(cs, tmp->addmask);
>> -            /* Expansion of parent's effective_xcpus is handled below */
>> -        }
>>           if (nocpu) {
>>               part_error = PERR_NOCPUS;
>>               if (is_partition_valid(cs))
>> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, 
>> struct tmpmasks *tmp,
>>                   /*
>>                    * When parent is not a valid partition root,
>>                    * child partition roots become invalid too.
>> -                 *
>> -                 * For a to be invalidated isolated partition,
>> -                 * call return_isolated_cpus() to remove the
>> -                 * relevant CPUs from isolated_cpus.
>> +                 * The isolation state of their CPUs has been
>> +                 * settled when the parent gave them back.
>>                    */
>>                   if (is_partition_valid(cp))
>>                       new_prs = -old_prs;
>> -                if (old_prs == PRS_ISOLATED) {
>> -                    cpumask_copy(tmp->delmask, cp->effective_xcpus);
>> -                    isolated_cpus_release(cp, tmp->delmask);
>> -                }
>>                   WRITE_ONCE(cp->prs_err,
>>                          is_partition_invalid(parent)
>>                          ? PERR_INVPARENT : PERR_NOTPART);
>> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>       if (!is_partition_valid(cs))
>>           reset_partition_data(cs);
>>       else if (isolcpus_updated)
>> -        isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>> +        isolated_cpus_update(new_prs, cs->effective_xcpus);
>>       spin_unlock_irq(&callback_lock);
>>       /* Force update if partition is disabled & update effective_xcpus */
> 
> The code change looks good to me. However this is not just a code restructuring, 
> it also includes a bug fix. I wonder if we should clarify that and maybe add a 
> Fixes tag.
> 
Hi Longman,

Thanks for the review. You are right, it also fixes the isolated CPU
leak reported by Guopeng [1]. When a root partition with an isolated
child partition is invalidated, partition_xcpus_del() gives the CPUs
back to the parent without updating isolated_cpus as both are root
partitions, so the CPUs of the isolated child are left in
isolated_cpus.

I'd like to describe it in the commit message and add the following
tags in the next version:

Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
Reported-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Closes: https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/

Does that look OK to you?

[1] https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-10  3:14     ` Ridong Chen
@ 2026-11-10  3:32       ` Waiman Long
  2026-11-10  8:18       ` Guopeng Zhang
  1 sibling, 0 replies; 17+ messages in thread
From: Waiman Long @ 2026-11-10  3:32 UTC (permalink / raw)
  To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen


On 11/9/26 10:14 PM, Ridong Chen wrote:
>
>
> On 11/9/2026 10:51 PM, Waiman Long wrote:
>> On 11/7/26 7:00 AM, Ridong Chen wrote:
>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>
>>> partition_xcpus_del() only updates isolated_cpus when the partition 
>>> that
>>> gives back the CPUs has a different state than its parent. CPUs that 
>>> were
>>> further distributed to descendant isolated partitions are not taken 
>>> into
>>> account, so isolated_cpus_release() has to be called afterwards in
>>> several places to walk up the ancestors and fix isolated_cpus.
>>>
>>> Make partition_xcpus_del() always set the isolation state of the given
>>> back CPUs to the state of the parent that receives them. As the
>>> effective_xcpus of a partition covers the CPUs distributed to its
>>> descendants, the isolation state of those CPUs is settled at the same
>>> time. Boot-time isolated CPUs are still preserved by 
>>> isolated_cpu_update().
>>>
>>> compute_partition_effective_cpumask() invalidates a child partition
>>> without going through partition_xcpus_del(), so do the same there: the
>>> CPUs of the invalidated child take the isolation state of its parent.
>>>
>>> As the parent may be shrunk and its effective_xcpus not updated yet,
>>> only do so for the CPUs the parent still owns. This is to keep the
>>> isolation state correct for now, the in-place invalidation here will be
>>> removed later in the partition rework.
>>>
>>> isolated_cpus_update() assumes the old and new states are different,
>>> which no longer holds now that partition_xcpus_del() proceeds 
>>> regardless
>>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so 
>>> that
>>> it simply sets the isolation state of the given CPUs, and use it
>>> everywhere. The state check in partition_xcpus_add() is dropped as 
>>> well,
>>> as setting the state is a no-op when the states are the same.
>>>
>>> isolated_cpus_release() is no longer needed and is removed.
>>>
>>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>>> ---
>>>   kernel/cgroup/cpuset.c | 98 
>>> ++++++++----------------------------------
>>>   1 file changed, 19 insertions(+), 79 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 85e5b529d144..fc9de6e16ff5 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, 
>>> int cpu)
>>>   }
>>>   /*
>>> - * isolated_cpus_update - Update the isolated_cpus mask
>>> - * @old_prs: old partition_root_state
>>> - * @new_prs: new partition_root_state
>>> - * @xcpus: exclusive CPUs with state change
>>> + * isolated_cpus_update - Set the isolation state of @xcpus
>>> + * @new_prs: partition root state that @xcpus will belong to
>>> + * @xcpus: exclusive CPUs whose isolation state may change
>>> + *
>>> + * CPUs isolated at boot time are kept isolated.
>>>    */
>>> -static void isolated_cpus_update(int old_prs, int new_prs, struct 
>>> cpumask *xcpus)
>>> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>>>   {
>>>       bool updated = false;
>>>       int cpu;
>>> -    WARN_ON_ONCE(old_prs == new_prs);
>>>       lockdep_assert_held(&callback_lock);
>>>       lockdep_assert_held(&cpuset_mutex);
>>>       for_each_cpu(cpu, xcpus)
>>> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, 
>>> int new_prs, struct cpumask *xcpus
>>>           update_housekeeping = true;
>>>   }
>>> -/*
>>> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from 
>>> isolated_cpus
>>> - * @cs: the current cpuset
>>> - * @xcpus: the set of effective xcpus to be considered for removal
>>> - *
>>> - * This function is called when an isolated partition is 
>>> invalidated or when
>>> - * some of its effective xcpus are to be removed from isolated_cpus 
>>> due to
>>> - * changes in parent cpuset. The given @xcpus must be modifiable as 
>>> the function
>>> - * will update its value to determine the right set of CPUs to be 
>>> removed.
>>> - */
>>> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask 
>>> *xcpus)
>>> -{
>>> -    struct cpuset *ancestor = parent_cs(cs);
>>> -    struct cpuset *iancestor = NULL;
>>> -
>>> -    /*
>>> -     * Search the furthest isolated partition root ancestor available
>>> -     * until top_cpuset or a remote partition root is hit.
>>> -     */
>>> -    for (; ancestor; ancestor = parent_cs(ancestor)) {
>>> -        if (ancestor->partition_root_state == PRS_ISOLATED)
>>> -            iancestor = ancestor;
>>> -        if (is_remote_partition(ancestor))
>>> -            break;
>>> -    }
>>> -
>>> -    /*
>>> -     * If a furthest isolated partition acnestor is found, use it to
>>> -     * determine which of the CPUs in @xcpus should be removed from
>>> -     * isolated_cpus. Otherwise, all of them will be removed.
>>> -     */
>>> -    if (iancestor && !cpumask_andnot(xcpus, xcpus, 
>>> iancestor->effective_xcpus))
>>> -        return; /* No CPU should be removed from isolated_cpus */
>>> -
>>> -    spin_lock_irq(&callback_lock);
>>> -    isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
>>> -    spin_unlock_irq(&callback_lock);
>>> -}
>>> -
>>>   /*
>>>    * partition_xcpus_add - Add new exclusive CPUs to partition
>>>    * @new_prs: new partition_root_state
>>> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, 
>>> struct cpuset *parent,
>>>       if (parent == &top_cpuset)
>>>           cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>>> -    if (new_prs != parent->partition_root_state)
>>> -        isolated_cpus_update(parent->partition_root_state, new_prs,
>>> -                     xcpus);
>>> +    isolated_cpus_update(new_prs, xcpus);
>>>       cpumask_andnot(parent->effective_cpus, parent->effective_cpus, 
>>> xcpus);
>>>   }
>>> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, 
>>> struct cpuset *parent,
>>>       if (parent == &top_cpuset)
>>>           cpumask_andnot(subpartitions_cpus, subpartitions_cpus, 
>>> xcpus);
>>> -    if (old_prs != parent->partition_root_state)
>>> -        isolated_cpus_update(old_prs, parent->partition_root_state,
>>> -                     xcpus);
>>> +    /* CPUs given back, including those of descendants, take 
>>> parent's state */
>>> +    isolated_cpus_update(parent->partition_root_state, xcpus);
>>>       cpumask_or(parent->effective_cpus, parent->effective_cpus, 
>>> xcpus);
>>>       cpumask_and(parent->effective_cpus, parent->effective_cpus, 
>>> cpu_active_mask);
>>> @@ -1612,6 +1570,7 @@ static void 
>>> compute_partition_effective_cpumask(struct cpuset *cs,
>>>       struct cgroup_subsys_state *css;
>>>       struct cpuset *child;
>>>       bool populated = partition_is_populated(cs, NULL);
>>> +    static cpumask_t xcpus;        /* Protected by cpuset_mutex */
>>>       /*
>>>        * Check child partition roots to see if they should be 
>>> invalidated
>>> @@ -1640,14 +1599,15 @@ static void 
>>> compute_partition_effective_cpumask(struct cpuset *cs,
>>>               int parent_prs = cs->partition_root_state;
>>>               /*
>>> -             * Invalidate child partition and update isolated_cpus
>>> -             * if necessary.
>>> +             * Invalidate child partition. Its CPUs still owned by
>>> +             * cs take the isolation state of cs. effective_xcpus
>>> +             * of cs may not be updated yet, so recompute it.
>>>                */
>>>               spin_lock_irq(&callback_lock);
>>>               WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>> -            if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
>>> -                isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
>>> -                             child->effective_xcpus);
>>> +            compute_excpus(cs, &xcpus);
>>> +            cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
>>> +            isolated_cpus_update(parent_prs, &xcpus);
>>>               child->partition_root_state = -old_prs;
>>>               spin_unlock_irq(&callback_lock);
>>>               notify_partition_change(child, old_prs);
>>> @@ -2080,22 +2040,8 @@ static int 
>>> update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>>            *
>>>            * A partition error happens when parent has tasks and all
>>>            * its effective CPUs will have to be distributed out.
>>> -         *
>>> -         * For an isolated partition with a shrunk root partition 
>>> parent
>>> -         * we may need to return some or all of its isolated CPUs.
>>>            */
>>>           nocpu |= cpumask_empty(tmp->delmask);
>>> -        if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
>>> -            cpumask_clear(tmp->addmask);
>>> -            if (nocpu)
>>> -                cpumask_copy(tmp->addmask, cs->effective_xcpus);
>>> -            else if (!cpumask_subset(cs->effective_xcpus, parent- 
>>> >effective_xcpus))
>>> -                cpumask_andnot(tmp->addmask, cs->effective_xcpus,
>>> -                                 parent->effective_xcpus);
>>> -            if (!cpumask_empty(tmp->addmask))
>>> -                isolated_cpus_release(cs, tmp->addmask);
>>> -            /* Expansion of parent's effective_xcpus is handled 
>>> below */
>>> -        }
>>>           if (nocpu) {
>>>               part_error = PERR_NOCPUS;
>>>               if (is_partition_valid(cs))
>>> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct 
>>> cpuset *cs, struct tmpmasks *tmp,
>>>                   /*
>>>                    * When parent is not a valid partition root,
>>>                    * child partition roots become invalid too.
>>> -                 *
>>> -                 * For a to be invalidated isolated partition,
>>> -                 * call return_isolated_cpus() to remove the
>>> -                 * relevant CPUs from isolated_cpus.
>>> +                 * The isolation state of their CPUs has been
>>> +                 * settled when the parent gave them back.
>>>                    */
>>>                   if (is_partition_valid(cp))
>>>                       new_prs = -old_prs;
>>> -                if (old_prs == PRS_ISOLATED) {
>>> -                    cpumask_copy(tmp->delmask, cp->effective_xcpus);
>>> -                    isolated_cpus_release(cp, tmp->delmask);
>>> -                }
>>>                   WRITE_ONCE(cp->prs_err,
>>>                          is_partition_invalid(parent)
>>>                          ? PERR_INVPARENT : PERR_NOTPART);
>>> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, 
>>> int new_prs)
>>>       if (!is_partition_valid(cs))
>>>           reset_partition_data(cs);
>>>       else if (isolcpus_updated)
>>> -        isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>>> +        isolated_cpus_update(new_prs, cs->effective_xcpus);
>>>       spin_unlock_irq(&callback_lock);
>>>       /* Force update if partition is disabled & update 
>>> effective_xcpus */
>>
>> The code change looks good to me. However this is not just a code 
>> restructuring, it also includes a bug fix. I wonder if we should 
>> clarify that and maybe add a Fixes tag.
>>
> Hi Longman,
>
> Thanks for the review. You are right, it also fixes the isolated CPU
> leak reported by Guopeng [1]. When a root partition with an isolated
> child partition is invalidated, partition_xcpus_del() gives the CPUs
> back to the parent without updating isolated_cpus as both are root
> partitions, so the CPUs of the isolated child are left in
> isolated_cpus.
>
> I'd like to describe it in the commit message and add the following
> tags in the next version:
>
> Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated 
> partitions")
> Reported-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> Closes: 
> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>
> Does that look OK to you?
>
> [1] 
> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>
Yes, that looks good to me.

Thanks,
Longman


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-09 15:20   ` Guopeng Zhang
@ 2026-11-10  6:24     ` Ridong Chen
  2026-11-10  6:38       ` Guopeng Zhang
  0 siblings, 1 reply; 17+ messages in thread
From: Ridong Chen @ 2026-11-10  6:24 UTC (permalink / raw)
  To: Guopeng Zhang, Waiman Long, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



On 11/9/2026 11:20 PM, Guopeng Zhang wrote:
> 
> 
> 在 2026/11/7 20:00, Ridong Chen 写道:
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> partition_xcpus_del() only updates isolated_cpus when the partition that
>> gives back the CPUs has a different state than its parent. CPUs that were
>> further distributed to descendant isolated partitions are not taken into
>> account, so isolated_cpus_release() has to be called afterwards in
>> several places to walk up the ancestors and fix isolated_cpus.
>>
>> Make partition_xcpus_del() always set the isolation state of the given
>> back CPUs to the state of the parent that receives them. As the
>> effective_xcpus of a partition covers the CPUs distributed to its
>> descendants, the isolation state of those CPUs is settled at the same
>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>
>> compute_partition_effective_cpumask() invalidates a child partition
>> without going through partition_xcpus_del(), so do the same there: the
>> CPUs of the invalidated child take the isolation state of its parent.
>>
>> As the parent may be shrunk and its effective_xcpus not updated yet,
>> only do so for the CPUs the parent still owns. This is to keep the
>> isolation state correct for now, the in-place invalidation here will be
>> removed later in the partition rework.
>>
>> isolated_cpus_update() assumes the old and new states are different,
>> which no longer holds now that partition_xcpus_del() proceeds regardless
>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>> it simply sets the isolation state of the given CPUs, and use it
>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>> as setting the state is a no-op when the states are the same.
>>
>> isolated_cpus_release() is no longer needed and is removed.
>>
> 
> Hi Ridong,
> 
> Would it be worth briefly mentioning in the commit message that this patch also fixes the issue described here?
> 
> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
> 
Thanks, Guopeng.

As I replied to Longman, I will add it.

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 4/4] cpuset: generalize the validate_partition() interface
  2026-11-09 14:52   ` Waiman Long
@ 2026-11-10  6:36     ` Ridong Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Ridong Chen @ 2026-11-10  6:36 UTC (permalink / raw)
  To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný,
	Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



On 11/9/2026 10:52 PM, Waiman Long wrote:
> On 11/7/26 7:01 AM, Ridong Chen wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Refactor validate_partition() to take the parent partition, the new
>> partition root state, the new effective exclusive CPUs and the CPUs to
>> be deleted, instead of a trial cpuset. A NULL parent stands for a
>> remote partition, whose CPUs come from top_cpuset. This makes the
>> interface flexible enough to handle both local and remote partition
>> validation.
>>
>> The no CPU left for tasks check now uses the given parent instead of
>> parent_cs(cs), and top_cpuset for a remote partition, as that is where
>> its CPUs are taken from. The cgroup parent of a remote partition does
>> not give it any CPU.
>>
>> The housekeeping conflict check is folded into validate_partition(). It
>> is skipped when the parent is not a valid partition root, as @cs can't
>> be a valid local partition in that case.
>>
>> Use the new interface for local partition enablement in
>> update_parent_effective_cpumask() and in partition_cpus_change(),
>> replacing the open-coded checks there. The order of the checks is now
>> the same for both paths: empty CPUs, housekeeping conflict, then no CPU
>> left for tasks.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>>   kernel/cgroup/cpuset.c | 119 ++++++++++++++++++++---------------------
>>   1 file changed, 59 insertions(+), 60 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index fc9de6e16ff5..0ec20412b3ce 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1414,6 +1414,48 @@ static bool prstate_housekeeping_conflict(int new_prs, 
>> int parent_prs,
>>       return res;
>>   }
>> +/**
>> + * validate_partition - Validate a cpuset partition configuration
>> + * @cs: The cpuset to validate
>> + * @parent: Parent partition of @cs, NULL for remote partition
>> + * @new_prs: The proposed new partition root state
>> + * @new_excpus: The new effective exclusive CPUs mask to validate
>> + * @del: exclusive CPUs to be deleted from @cs, can be NULL
>> + *
>> + * A remote partition gets its CPUs from top_cpuset, so top_cpuset is used
>> + * as the parent when @parent is NULL.
>> + *
>> + * The housekeeping check is skipped if @parent is not a valid partition
>> + * root as @cs can't be a valid local partition in that case.
>> + *
>> + * Return: PRS error code (0 if valid, non-zero error code if invalid)
>> + */
>> +static enum prs_errcode validate_partition(struct cpuset *cs,
>> +                       struct cpuset *parent, int new_prs,
>> +                       struct cpumask *new_excpus,
>> +                       struct cpumask *del)
>> +{
>> +    int parent_prs;
>> +
>> +    if (new_prs == PRS_MEMBER)
>> +        return PERR_NONE;
>> +
>> +    if (cpumask_empty(new_excpus))
>> +        return PERR_INVCPUS;
>> +
>> +    if (!parent)
>> +        parent = &top_cpuset;
>> +    parent_prs = parent->partition_root_state;
>> +    if (parent_prs > 0 &&
>> +        prstate_housekeeping_conflict(new_prs, parent_prs, new_excpus, del))
>> +        return PERR_HKEEPING;
>> +
>> +    if (tasks_nocpu_error(parent, cs, new_excpus))
>> +        return PERR_NOCPUS;
>> +
>> +    return PERR_NONE;
>> +}
>> +
>>   /*
>>    * cpuset_update_sd_hk_unlock - Rebuild sched domains, update HK & unlock
>>    *
>> @@ -1934,18 +1976,10 @@ static int update_parent_effective_cpumask(struct 
>> cpuset *cs, int cmd,
>>       if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
>>           new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
>> -        /*
>> -         * Enabling partition root is not allowed if its
>> -         * effective_xcpus is empty.
>> -         */
>> -        if (cpumask_empty(tmp->delmask))
>> -            return PERR_INVCPUS;
>> -
>> -        if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask, 
>> NULL))
>> -            return PERR_HKEEPING;
>> -
>> -        if (nocpu)
>> -            return PERR_NOCPUS;
>> +        part_error = validate_partition(cs, parent, new_prs,
>> +                        tmp->delmask, NULL);
>> +        if (part_error)
>> +            return part_error;
>>           /*
>>            * This function will only be called when all the preliminary
>> @@ -2424,32 +2458,6 @@ static int parse_cpuset_cpulist(const char *buf, struct 
>> cpumask *out_mask)
>>       return 0;
>>   }
>> -/**
>> - * validate_partition - Validate a cpuset partition configuration
>> - * @cs: The cpuset to validate
>> - * @trialcs: The trial cpuset containing proposed configuration changes
>> - *
>> - * If any validation check fails, the appropriate error code is set in the
>> - * cpuset's prs_err field.
>> - *
>> - * Return: PRS error code (0 if valid, non-zero error code if invalid)
>> - */
>> -static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset 
>> *trialcs)
>> -{
>> -    struct cpuset *parent = parent_cs(cs);
>> -
>> -    if (cs_is_member(trialcs))
>> -        return PERR_NONE;
>> -
>> -    if (cpumask_empty(trialcs->effective_xcpus))
>> -        return PERR_INVCPUS;
>> -
>> -    if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
>> -        return PERR_NOCPUS;
>> -
>> -    return PERR_NONE;
>> -}
>> -
>>   /**
>>    * partition_cpus_change - Handle partition state changes due to CPU mask 
>> updates
>>    * @cs: The target cpuset being modified
>> @@ -2463,33 +2471,24 @@ static void partition_cpus_change(struct cpuset *cs, 
>> struct cpuset *trialcs,
>>                     struct tmpmasks *tmp)
>>   {
>>       enum prs_errcode prs_err;
>> -    int parent_prs = is_remote_partition(cs)
>> -               ? PRS_ROOT : parent_cs(cs)->partition_root_state;
>> +    struct cpuset *parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>>       if (cs_is_member(cs))
>>           return;
>> -    prs_err = validate_partition(cs, trialcs);
>> -    if (!prs_err && (parent_prs > 0) &&
>> -        !cpumask_empty(trialcs->effective_xcpus)) {
>> -        int prs = cs->partition_root_state;
>> -
>> -        /*
>> -         * Check for housekeeping conflict by passing the whole
>> -         * effective_xcpus as well as those CPUs that are to be
>> -         * removed from cs as delmask.
>> -         */
>> -        if (prs > 0)
>> -            cpumask_andnot(tmp->delmask, cs->effective_xcpus,
>> -                       trialcs->effective_xcpus);
>> -        else
>> -            cpumask_clear(tmp->delmask);
>> -
>> -        if (prstate_housekeeping_conflict(prs, parent_prs,
>> -                          trialcs->effective_xcpus, tmp->delmask))
>> -            prs_err = PERR_HKEEPING;
>> -    }
>> +    /*
>> +     * Check for housekeeping conflict by passing the whole
>> +     * effective_xcpus as well as those CPUs that are to be
>> +     * removed from cs as delmask.
>> +     */
>> +    if (is_partition_valid(cs))
>> +        cpumask_andnot(tmp->delmask, cs->effective_xcpus,
>> +                   trialcs->effective_xcpus);
>> +    else
>> +        cpumask_clear(tmp->delmask);
>> +    prs_err = validate_partition(cs, parent, trialcs->partition_root_state,
>> +                     trialcs->effective_xcpus, tmp->delmask);
>>       if (prs_err) {
>>           WRITE_ONCE(cs->prs_err, prs_err);
>>           trialcs->prs_err = prs_err;
> 
> Again, this code change looks good to me. Similar to previous patch, it includes 
> both code restructuring and bug fixes.
> 
Hi Longman,

Thanks for the review. Yes, besides the restructuring, it fixes a false
PERR_NOCPUS error for remote partitions. The no CPU left for tasks check
in partition_cpus_change() is done against the cgroup parent, but the
CPUs of a remote partition come from top_cpuset, not from its parent.

It can be reproduced on a 4-CPU system:

	cd /sys/fs/cgroup
	echo +cpuset > cgroup.subtree_control
	mkdir P
	echo 2-3 > P/cpuset.cpus
	echo 2-3 > P/cpuset.cpus.exclusive
	echo +cpuset > P/cgroup.subtree_control
	mkdir P/C
	echo 2-3 > P/C/cpuset.cpus
	echo 2 > P/C/cpuset.cpus.exclusive
	echo root > P/C/cpuset.cpus.partition
	sleep 1000 &
	echo $! > P/cgroup.procs
	echo 2-3 > P/C/cpuset.cpus.exclusive
	cat P/C/cpuset.cpus.partition
	root invalid (Parent is not a partition root)

The effective_cpus of P is 3, a subset of the new exclusive CPUs 2-3 of
C, and P has a task, so the expansion is rejected. However, P is not a
partition root and does not give any CPU to C, and top_cpuset still has
CPUs 0-1 for the task. Enabling C directly with exclusive CPUs 2-3
succeeds, as remote_partition_enable() checks top_cpuset instead. With
this patch, the expansion succeeds too:

	cat P/C/cpuset.cpus.partition
	root

The issue has been there since remote partitions were introduced. I
reproduced it on commit 181c8e091aae as well, so I'd like to describe
it in the commit message and add

	Fixes: 181c8e091aae ("cgroup/cpuset: Introduce remote partition")

in the next version.

Does that look OK to you? Please let me know if you are referring to something else.

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-10  6:24     ` Ridong Chen
@ 2026-11-10  6:38       ` Guopeng Zhang
  0 siblings, 0 replies; 17+ messages in thread
From: Guopeng Zhang @ 2026-11-10  6:38 UTC (permalink / raw)
  To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



在 2026/11/10 14:24, Ridong Chen 写道:
> 
> 
> On 11/9/2026 11:20 PM, Guopeng Zhang wrote:
>>
>>
>> 在 2026/11/7 20:00, Ridong Chen 写道:
>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>
>>> partition_xcpus_del() only updates isolated_cpus when the partition that
>>> gives back the CPUs has a different state than its parent. CPUs that were
>>> further distributed to descendant isolated partitions are not taken into
>>> account, so isolated_cpus_release() has to be called afterwards in
>>> several places to walk up the ancestors and fix isolated_cpus.
>>>
>>> Make partition_xcpus_del() always set the isolation state of the given
>>> back CPUs to the state of the parent that receives them. As the
>>> effective_xcpus of a partition covers the CPUs distributed to its
>>> descendants, the isolation state of those CPUs is settled at the same
>>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>>
>>> compute_partition_effective_cpumask() invalidates a child partition
>>> without going through partition_xcpus_del(), so do the same there: the
>>> CPUs of the invalidated child take the isolation state of its parent.
>>>
>>> As the parent may be shrunk and its effective_xcpus not updated yet,
>>> only do so for the CPUs the parent still owns. This is to keep the
>>> isolation state correct for now, the in-place invalidation here will be
>>> removed later in the partition rework.
>>>
>>> isolated_cpus_update() assumes the old and new states are different,
>>> which no longer holds now that partition_xcpus_del() proceeds regardless
>>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>>> it simply sets the isolation state of the given CPUs, and use it
>>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>>> as setting the state is a no-op when the states are the same.
>>>
>>> isolated_cpus_release() is no longer needed and is removed.
>>>
>>
>> Hi Ridong,
>>
>> Would it be worth briefly mentioning in the commit message that this patch also fixes the issue described here?
>>
>> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>>
> Thanks, Guopeng.
> 
> As I replied to Longman, I will add it.
> 

Sorry, I just realized that my comment duplicated Longman's. I had some email connectivity issues and only just received the review emails.

Thanks for the clarification!

Cheers,
Guopeng

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-10  3:14     ` Ridong Chen
  2026-11-10  3:32       ` Waiman Long
@ 2026-11-10  8:18       ` Guopeng Zhang
  2026-11-12  3:16         ` Ridong Chen
  1 sibling, 1 reply; 17+ messages in thread
From: Guopeng Zhang @ 2026-11-10  8:18 UTC (permalink / raw)
  To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



在 2026/11/10 11:14, Ridong Chen 写道:
> 
> 
> On 11/9/2026 10:51 PM, Waiman Long wrote:
>> On 11/7/26 7:00 AM, Ridong Chen wrote:
>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>
>>> partition_xcpus_del() only updates isolated_cpus when the partition that
>>> gives back the CPUs has a different state than its parent. CPUs that were
>>> further distributed to descendant isolated partitions are not taken into
>>> account, so isolated_cpus_release() has to be called afterwards in
>>> several places to walk up the ancestors and fix isolated_cpus.
>>>
>>> Make partition_xcpus_del() always set the isolation state of the given
>>> back CPUs to the state of the parent that receives them. As the
>>> effective_xcpus of a partition covers the CPUs distributed to its
>>> descendants, the isolation state of those CPUs is settled at the same
>>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>>
>>> compute_partition_effective_cpumask() invalidates a child partition
>>> without going through partition_xcpus_del(), so do the same there: the
>>> CPUs of the invalidated child take the isolation state of its parent.
>>>
>>> As the parent may be shrunk and its effective_xcpus not updated yet,
>>> only do so for the CPUs the parent still owns. This is to keep the
>>> isolation state correct for now, the in-place invalidation here will be
>>> removed later in the partition rework.
>>>
>>> isolated_cpus_update() assumes the old and new states are different,
>>> which no longer holds now that partition_xcpus_del() proceeds regardless
>>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>>> it simply sets the isolation state of the given CPUs, and use it
>>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>>> as setting the state is a no-op when the states are the same.
>>>
>>> isolated_cpus_release() is no longer needed and is removed.
>>>
>>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>>> ---
>>>   kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
>>>   1 file changed, 19 insertions(+), 79 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 85e5b529d144..fc9de6e16ff5 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
>>>   }
>>>   /*
>>> - * isolated_cpus_update - Update the isolated_cpus mask
>>> - * @old_prs: old partition_root_state
>>> - * @new_prs: new partition_root_state
>>> - * @xcpus: exclusive CPUs with state change
>>> + * isolated_cpus_update - Set the isolation state of @xcpus
>>> + * @new_prs: partition root state that @xcpus will belong to
>>> + * @xcpus: exclusive CPUs whose isolation state may change
>>> + *
>>> + * CPUs isolated at boot time are kept isolated.
>>>    */
>>> -static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
>>> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>>>   {
>>>       bool updated = false;
>>>       int cpu;
>>> -    WARN_ON_ONCE(old_prs == new_prs);
>>>       lockdep_assert_held(&callback_lock);
>>>       lockdep_assert_held(&cpuset_mutex);
>>>       for_each_cpu(cpu, xcpus)
>>> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>>>           update_housekeeping = true;
>>>   }
>>> -/*
>>> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
>>> - * @cs: the current cpuset
>>> - * @xcpus: the set of effective xcpus to be considered for removal
>>> - *
>>> - * This function is called when an isolated partition is invalidated or when
>>> - * some of its effective xcpus are to be removed from isolated_cpus due to
>>> - * changes in parent cpuset. The given @xcpus must be modifiable as the function
>>> - * will update its value to determine the right set of CPUs to be removed.
>>> - */
>>> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
>>> -{
>>> -    struct cpuset *ancestor = parent_cs(cs);
>>> -    struct cpuset *iancestor = NULL;
>>> -
>>> -    /*
>>> -     * Search the furthest isolated partition root ancestor available
>>> -     * until top_cpuset or a remote partition root is hit.
>>> -     */
>>> -    for (; ancestor; ancestor = parent_cs(ancestor)) {
>>> -        if (ancestor->partition_root_state == PRS_ISOLATED)
>>> -            iancestor = ancestor;
>>> -        if (is_remote_partition(ancestor))
>>> -            break;
>>> -    }
>>> -
>>> -    /*
>>> -     * If a furthest isolated partition acnestor is found, use it to
>>> -     * determine which of the CPUs in @xcpus should be removed from
>>> -     * isolated_cpus. Otherwise, all of them will be removed.
>>> -     */
>>> -    if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
>>> -        return; /* No CPU should be removed from isolated_cpus */
>>> -
>>> -    spin_lock_irq(&callback_lock);
>>> -    isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
>>> -    spin_unlock_irq(&callback_lock);
>>> -}
>>> -
>>>   /*
>>>    * partition_xcpus_add - Add new exclusive CPUs to partition
>>>    * @new_prs: new partition_root_state
>>> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
>>>       if (parent == &top_cpuset)
>>>           cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>>> -    if (new_prs != parent->partition_root_state)
>>> -        isolated_cpus_update(parent->partition_root_state, new_prs,
>>> -                     xcpus);
>>> +    isolated_cpus_update(new_prs, xcpus);
>>>       cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
>>>   }
>>> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
>>>       if (parent == &top_cpuset)
>>>           cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>>> -    if (old_prs != parent->partition_root_state)
>>> -        isolated_cpus_update(old_prs, parent->partition_root_state,
>>> -                     xcpus);
>>> +    /* CPUs given back, including those of descendants, take parent's state */
>>> +    isolated_cpus_update(parent->partition_root_state, xcpus);
>>>       cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
>>>       cpumask_and(parent->effective_cpus, parent->effective_cpus, cpu_active_mask);
>>> @@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>>       struct cgroup_subsys_state *css;
>>>       struct cpuset *child;
>>>       bool populated = partition_is_populated(cs, NULL);
>>> +    static cpumask_t xcpus;        /* Protected by cpuset_mutex */
>>>       /*
>>>        * Check child partition roots to see if they should be invalidated
>>> @@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>>               int parent_prs = cs->partition_root_state;
>>>               /*
>>> -             * Invalidate child partition and update isolated_cpus
>>> -             * if necessary.
>>> +             * Invalidate child partition. Its CPUs still owned by
>>> +             * cs take the isolation state of cs. effective_xcpus
>>> +             * of cs may not be updated yet, so recompute it.
>>>                */
>>>               spin_lock_irq(&callback_lock);
>>>               WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>> -            if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
>>> -                isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
>>> -                             child->effective_xcpus);
>>> +            compute_excpus(cs, &xcpus);
>>> +            cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
>>> +            isolated_cpus_update(parent_prs, &xcpus);
>>>               child->partition_root_state = -old_prs;
>>>               spin_unlock_irq(&callback_lock);
>>>               notify_partition_change(child, old_prs);
>>> @@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>>            *
>>>            * A partition error happens when parent has tasks and all
>>>            * its effective CPUs will have to be distributed out.
>>> -         *
>>> -         * For an isolated partition with a shrunk root partition parent
>>> -         * we may need to return some or all of its isolated CPUs.
>>>            */
>>>           nocpu |= cpumask_empty(tmp->delmask);
>>> -        if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
>>> -            cpumask_clear(tmp->addmask);
>>> -            if (nocpu)
>>> -                cpumask_copy(tmp->addmask, cs->effective_xcpus);
>>> -            else if (!cpumask_subset(cs->effective_xcpus, parent- >effective_xcpus))
>>> -                cpumask_andnot(tmp->addmask, cs->effective_xcpus,
>>> -                                 parent->effective_xcpus);
>>> -            if (!cpumask_empty(tmp->addmask))
>>> -                isolated_cpus_release(cs, tmp->addmask);
>>> -            /* Expansion of parent's effective_xcpus is handled below */
>>> -        }
>>>           if (nocpu) {
>>>               part_error = PERR_NOCPUS;
>>>               if (is_partition_valid(cs))
>>> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
>>>                   /*
>>>                    * When parent is not a valid partition root,
>>>                    * child partition roots become invalid too.
>>> -                 *
>>> -                 * For a to be invalidated isolated partition,
>>> -                 * call return_isolated_cpus() to remove the
>>> -                 * relevant CPUs from isolated_cpus.
>>> +                 * The isolation state of their CPUs has been
>>> +                 * settled when the parent gave them back.
>>>                    */
>>>                   if (is_partition_valid(cp))
>>>                       new_prs = -old_prs;
>>> -                if (old_prs == PRS_ISOLATED) {
>>> -                    cpumask_copy(tmp->delmask, cp->effective_xcpus);
>>> -                    isolated_cpus_release(cp, tmp->delmask);
>>> -                }
>>>                   WRITE_ONCE(cp->prs_err,
>>>                          is_partition_invalid(parent)
>>>                          ? PERR_INVPARENT : PERR_NOTPART);
>>> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>>       if (!is_partition_valid(cs))
>>>           reset_partition_data(cs);
>>>       else if (isolcpus_updated)
>>> -        isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>>> +        isolated_cpus_update(new_prs, cs->effective_xcpus);
>>>       spin_unlock_irq(&callback_lock);
>>>       /* Force update if partition is disabled & update effective_xcpus */
>>
>> The code change looks good to me. However this is not just a code restructuring, it also includes a bug fix. I wonder if we should clarify that and maybe add a Fixes tag.
>>
> Hi Longman,
> 
> Thanks for the review. You are right, it also fixes the isolated CPU
> leak reported by Guopeng [1]. When a root partition with an isolated
> child partition is invalidated, partition_xcpus_del() gives the CPUs
> back to the parent without updating isolated_cpus as both are root
> partitions, so the CPUs of the isolated child are left in
> isolated_cpus.
> 
> I'd like to describe it in the commit message and add the following
> tags in the next version:
> 
> Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
> Reported-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> Closes: https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
> 
> Does that look OK to you?
> 
> [1] https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
> 
Hi Ridong,

Since I've dropped my patch, I haven't added the reproducer to the selftests separately. If it's convenient, could you also include it in test_cpuset_prs.sh in v4?

Thanks,
Guopeng

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
  2026-11-10  8:18       ` Guopeng Zhang
@ 2026-11-12  3:16         ` Ridong Chen
  0 siblings, 0 replies; 17+ messages in thread
From: Ridong Chen @ 2026-11-12  3:16 UTC (permalink / raw)
  To: Guopeng Zhang, Waiman Long, Tejun Heo, Johannes Weiner,
	Michal Koutný, Shuah Khan
  Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
	Guopeng Zhang, Ridong Chen



On 11/10/2026 4:18 PM, Guopeng Zhang wrote:
> 
> 
> 在 2026/11/10 11:14, Ridong Chen 写道:
>>
>>
>> On 11/9/2026 10:51 PM, Waiman Long wrote:
>>> On 11/7/26 7:00 AM, Ridong Chen wrote:
>>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>>
>>>> partition_xcpus_del() only updates isolated_cpus when the partition that
>>>> gives back the CPUs has a different state than its parent. CPUs that were
>>>> further distributed to descendant isolated partitions are not taken into
>>>> account, so isolated_cpus_release() has to be called afterwards in
>>>> several places to walk up the ancestors and fix isolated_cpus.
>>>>
>>>> Make partition_xcpus_del() always set the isolation state of the given
>>>> back CPUs to the state of the parent that receives them. As the
>>>> effective_xcpus of a partition covers the CPUs distributed to its
>>>> descendants, the isolation state of those CPUs is settled at the same
>>>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>>>
>>>> compute_partition_effective_cpumask() invalidates a child partition
>>>> without going through partition_xcpus_del(), so do the same there: the
>>>> CPUs of the invalidated child take the isolation state of its parent.
>>>>
>>>> As the parent may be shrunk and its effective_xcpus not updated yet,
>>>> only do so for the CPUs the parent still owns. This is to keep the
>>>> isolation state correct for now, the in-place invalidation here will be
>>>> removed later in the partition rework.
>>>>
>>>> isolated_cpus_update() assumes the old and new states are different,
>>>> which no longer holds now that partition_xcpus_del() proceeds regardless
>>>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>>>> it simply sets the isolation state of the given CPUs, and use it
>>>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>>>> as setting the state is a no-op when the states are the same.
>>>>
>>>> isolated_cpus_release() is no longer needed and is removed.
>>>>
>>>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>>>> ---
>>>>    kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
>>>>    1 file changed, 19 insertions(+), 79 deletions(-)
>>>>
>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>> index 85e5b529d144..fc9de6e16ff5 100644
>>>> --- a/kernel/cgroup/cpuset.c
>>>> +++ b/kernel/cgroup/cpuset.c
>>>> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
>>>>    }
>>>>    /*
>>>> - * isolated_cpus_update - Update the isolated_cpus mask
>>>> - * @old_prs: old partition_root_state
>>>> - * @new_prs: new partition_root_state
>>>> - * @xcpus: exclusive CPUs with state change
>>>> + * isolated_cpus_update - Set the isolation state of @xcpus
>>>> + * @new_prs: partition root state that @xcpus will belong to
>>>> + * @xcpus: exclusive CPUs whose isolation state may change
>>>> + *
>>>> + * CPUs isolated at boot time are kept isolated.
>>>>     */
>>>> -static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
>>>> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>>>>    {
>>>>        bool updated = false;
>>>>        int cpu;
>>>> -    WARN_ON_ONCE(old_prs == new_prs);
>>>>        lockdep_assert_held(&callback_lock);
>>>>        lockdep_assert_held(&cpuset_mutex);
>>>>        for_each_cpu(cpu, xcpus)
>>>> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>>>>            update_housekeeping = true;
>>>>    }
>>>> -/*
>>>> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
>>>> - * @cs: the current cpuset
>>>> - * @xcpus: the set of effective xcpus to be considered for removal
>>>> - *
>>>> - * This function is called when an isolated partition is invalidated or when
>>>> - * some of its effective xcpus are to be removed from isolated_cpus due to
>>>> - * changes in parent cpuset. The given @xcpus must be modifiable as the function
>>>> - * will update its value to determine the right set of CPUs to be removed.
>>>> - */
>>>> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
>>>> -{
>>>> -    struct cpuset *ancestor = parent_cs(cs);
>>>> -    struct cpuset *iancestor = NULL;
>>>> -
>>>> -    /*
>>>> -     * Search the furthest isolated partition root ancestor available
>>>> -     * until top_cpuset or a remote partition root is hit.
>>>> -     */
>>>> -    for (; ancestor; ancestor = parent_cs(ancestor)) {
>>>> -        if (ancestor->partition_root_state == PRS_ISOLATED)
>>>> -            iancestor = ancestor;
>>>> -        if (is_remote_partition(ancestor))
>>>> -            break;
>>>> -    }
>>>> -
>>>> -    /*
>>>> -     * If a furthest isolated partition acnestor is found, use it to
>>>> -     * determine which of the CPUs in @xcpus should be removed from
>>>> -     * isolated_cpus. Otherwise, all of them will be removed.
>>>> -     */
>>>> -    if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
>>>> -        return; /* No CPU should be removed from isolated_cpus */
>>>> -
>>>> -    spin_lock_irq(&callback_lock);
>>>> -    isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
>>>> -    spin_unlock_irq(&callback_lock);
>>>> -}
>>>> -
>>>>    /*
>>>>     * partition_xcpus_add - Add new exclusive CPUs to partition
>>>>     * @new_prs: new partition_root_state
>>>> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
>>>>        if (parent == &top_cpuset)
>>>>            cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>>>> -    if (new_prs != parent->partition_root_state)
>>>> -        isolated_cpus_update(parent->partition_root_state, new_prs,
>>>> -                     xcpus);
>>>> +    isolated_cpus_update(new_prs, xcpus);
>>>>        cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
>>>>    }
>>>> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
>>>>        if (parent == &top_cpuset)
>>>>            cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>>>> -    if (old_prs != parent->partition_root_state)
>>>> -        isolated_cpus_update(old_prs, parent->partition_root_state,
>>>> -                     xcpus);
>>>> +    /* CPUs given back, including those of descendants, take parent's state */
>>>> +    isolated_cpus_update(parent->partition_root_state, xcpus);
>>>>        cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
>>>>        cpumask_and(parent->effective_cpus, parent->effective_cpus, cpu_active_mask);
>>>> @@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>>>        struct cgroup_subsys_state *css;
>>>>        struct cpuset *child;
>>>>        bool populated = partition_is_populated(cs, NULL);
>>>> +    static cpumask_t xcpus;        /* Protected by cpuset_mutex */
>>>>        /*
>>>>         * Check child partition roots to see if they should be invalidated
>>>> @@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>>>                int parent_prs = cs->partition_root_state;
>>>>                /*
>>>> -             * Invalidate child partition and update isolated_cpus
>>>> -             * if necessary.
>>>> +             * Invalidate child partition. Its CPUs still owned by
>>>> +             * cs take the isolation state of cs. effective_xcpus
>>>> +             * of cs may not be updated yet, so recompute it.
>>>>                 */
>>>>                spin_lock_irq(&callback_lock);
>>>>                WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>>> -            if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
>>>> -                isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
>>>> -                             child->effective_xcpus);
>>>> +            compute_excpus(cs, &xcpus);
>>>> +            cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
>>>> +            isolated_cpus_update(parent_prs, &xcpus);
>>>>                child->partition_root_state = -old_prs;
>>>>                spin_unlock_irq(&callback_lock);
>>>>                notify_partition_change(child, old_prs);
>>>> @@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>>>             *
>>>>             * A partition error happens when parent has tasks and all
>>>>             * its effective CPUs will have to be distributed out.
>>>> -         *
>>>> -         * For an isolated partition with a shrunk root partition parent
>>>> -         * we may need to return some or all of its isolated CPUs.
>>>>             */
>>>>            nocpu |= cpumask_empty(tmp->delmask);
>>>> -        if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
>>>> -            cpumask_clear(tmp->addmask);
>>>> -            if (nocpu)
>>>> -                cpumask_copy(tmp->addmask, cs->effective_xcpus);
>>>> -            else if (!cpumask_subset(cs->effective_xcpus, parent- >effective_xcpus))
>>>> -                cpumask_andnot(tmp->addmask, cs->effective_xcpus,
>>>> -                                 parent->effective_xcpus);
>>>> -            if (!cpumask_empty(tmp->addmask))
>>>> -                isolated_cpus_release(cs, tmp->addmask);
>>>> -            /* Expansion of parent's effective_xcpus is handled below */
>>>> -        }
>>>>            if (nocpu) {
>>>>                part_error = PERR_NOCPUS;
>>>>                if (is_partition_valid(cs))
>>>> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
>>>>                    /*
>>>>                     * When parent is not a valid partition root,
>>>>                     * child partition roots become invalid too.
>>>> -                 *
>>>> -                 * For a to be invalidated isolated partition,
>>>> -                 * call return_isolated_cpus() to remove the
>>>> -                 * relevant CPUs from isolated_cpus.
>>>> +                 * The isolation state of their CPUs has been
>>>> +                 * settled when the parent gave them back.
>>>>                     */
>>>>                    if (is_partition_valid(cp))
>>>>                        new_prs = -old_prs;
>>>> -                if (old_prs == PRS_ISOLATED) {
>>>> -                    cpumask_copy(tmp->delmask, cp->effective_xcpus);
>>>> -                    isolated_cpus_release(cp, tmp->delmask);
>>>> -                }
>>>>                    WRITE_ONCE(cp->prs_err,
>>>>                           is_partition_invalid(parent)
>>>>                           ? PERR_INVPARENT : PERR_NOTPART);
>>>> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>>>        if (!is_partition_valid(cs))
>>>>            reset_partition_data(cs);
>>>>        else if (isolcpus_updated)
>>>> -        isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>>>> +        isolated_cpus_update(new_prs, cs->effective_xcpus);
>>>>        spin_unlock_irq(&callback_lock);
>>>>        /* Force update if partition is disabled & update effective_xcpus */
>>>
>>> The code change looks good to me. However this is not just a code restructuring, it also includes a bug fix. I wonder if we should clarify that and maybe add a Fixes tag.
>>>
>> Hi Longman,
>>
>> Thanks for the review. You are right, it also fixes the isolated CPU
>> leak reported by Guopeng [1]. When a root partition with an isolated
>> child partition is invalidated, partition_xcpus_del() gives the CPUs
>> back to the parent without updating isolated_cpus as both are root
>> partitions, so the CPUs of the isolated child are left in
>> isolated_cpus.
>>
>> I'd like to describe it in the commit message and add the following
>> tags in the next version:
>>
>> Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
>> Reported-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> Closes: https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>>
>> Does that look OK to you?
>>
>> [1] https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>>
> Hi Ridong,
> 
> Since I've dropped my patch, I haven't added the reproducer to the selftests separately. If it's convenient, could you also include it in test_cpuset_prs.sh in v4?
> 
Hi Guopeng,

Thanks for the suggestion. I looked into it, but the reproducer doesn't
fit into the existing test matrix for now.

To trigger the leak, the root partition has to take all the CPUs of the
top cpuset except one, and that last CPU is then offlined. So the CPU
list depends on how many CPUs the machine has. The test matrix only
uses fixed CPU lists within CPUs 0-7, and on a machine with more CPUs
the top cpuset keeps the others, so offlining one CPU doesn't
invalidate the partition.

It would need a separate test function outside the matrix. The
selftest is already large and getting hard to maintain, so I'd rather
not add a special case for it. Instead, I put the reproducer in the
commit message of patch 1 in v4, so it can still be used to verify the
fix.

-- 
Best regards
Ridong


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-11-12  3:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
2026-11-09 14:59   ` Guopeng Zhang
2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
2026-11-09 15:48   ` Tejun Heo
2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
2026-11-09 14:51   ` Waiman Long
2026-11-10  3:14     ` Ridong Chen
2026-11-10  3:32       ` Waiman Long
2026-11-10  8:18       ` Guopeng Zhang
2026-11-12  3:16         ` Ridong Chen
2026-11-09 15:20   ` Guopeng Zhang
2026-11-10  6:24     ` Ridong Chen
2026-11-10  6:38       ` Guopeng Zhang
2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
2026-11-09 14:52   ` Waiman Long
2026-11-10  6:36     ` Ridong Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox