[PATCH v3 0/7] cgroup/cpuset: Fix partition type transitions

[PATCH v3 0/7] cgroup/cpuset: Fix partition type transitions

[Guopeng Zhang]

Several partition type-change and validation paths use effective_xcpus,
which also includes CPUs assigned to valid child partitions. As a result,
changing a parent between root and isolated can alter a child's isolation
accounting or fail housekeeping validation because of CPUs the parent does
not own.

A proposed parent CPU mask needs different treatment for children which
would become invalid under that mask. Their CPUs return to the parent and
must participate in housekeeping validation. Otherwise a parent can pass
validation with a boot-isolated CPU hidden behind a child which is about
to become invalid.

There are two related failure paths. Returning the last housekeeping CPU
from a root child to an isolated parent must invalidate the appropriate
isolated ancestor. Also, a root/isolated type change which fails
housekeeping validation must release the partition's CPUs instead of
leaving them unavailable to the owner of the invalidated subtree.

Factor out the child partition validity checks and reuse them when
computing the CPUs owned directly by a partition under a proposed
configuration. Use the directly owned CPUs for root/isolated type-change
accounting and housekeeping validation, handle the last-housekeeping-CPU
return case, and run the normal partition-disable path after a failed
type-change validation. Add focused selftests for each case.

This is based on cgroup/for-7.3-fixes (3f4b7d1a49c5 "selftests/cgroup:
test clone3() into a previously killed cgroup").

Testing:

- Ran tools/testing/selftests/cgroup/test_cpuset_prs.sh with
  isolcpus=domain,15; all applicable tests passed.
- Ran the housekeeping CPU-return tests with
  nohz_full=1-14 isolcpus=domain,15; both cases passed.

Changes in v3:

- factor the child partition validity rules into a preparatory patch and
  reuse them for trial ownership calculation;
- during trial validation, subtract only children which remain valid under
  the proposed parent CPU mask, covering both PERR_INVCPUS and PERR_NOCPUS;
- keep cs->effective_xcpus as the default isolation-accounting mask and
  use the directly owned mask only for a successful root/isolated type
  change;
- use an isolated child for the boot-isolated trial-validation tests, as a
  root child containing that CPU fails immediately with PERR_HKEEPING;
- make the isolated-ancestor walk safe when it reaches the top cpuset and
  clarify the member-transition handling;
- clean up nested housekeeping-return test cgroups on failure.

Link: https://lore.kernel.org/all/20260828095643.13395-1-guopeng.zhang@linux.dev/

Changes in v2:

- split the type-transition fixes from the original series;
- merge the child-owned accounting and validation changes;
- validate trial CPU masks against CPUs owned directly by the partition;
- handle a root child returning the last housekeeping CPU, including
  nested isolated ancestors;
- use the common partition-disable path after type-change validation
  failure;
- initialize boot-isolated CPU data during selftest setup;
- split the accounting, child-owned validation and housekeeping-return
  tests.

Link: https://lore.kernel.org/all/20260820124202.517160-1-guopeng.zhang@linux.dev/

Guopeng Zhang (7):
  cgroup/cpuset: Factor out child partition validation
  cgroup/cpuset: Account for child CPU ownership in partition changes
  selftests/cgroup: Add tests for type-change isolation accounting
  selftests/cgroup: Test child CPU ownership in partition changes
  selftests/cgroup: Add tests for housekeeping CPU return to isolated
    parents
  cgroup/cpuset: Release CPUs when type-change validation fails
  selftests/cgroup: Add CPU release tests for type-change validation
    failures

 kernel/cgroup/cpuset.c                        | 182 +++++++-
 .../selftests/cgroup/test_cpuset_prs.sh       | 430 +++++++++++++++++-
 2 files changed, 588 insertions(+), 24 deletions(-)


base-commit: 3f4b7d1a49c5c826f3be9b684313eea5b83ac232
-- 
2.43.0

[PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Guopeng Zhang]

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

compute_partition_effective_cpumask() checks whether each valid child
partition remains covered by the parent exclusive CPU mask and whether it
would consume all remaining CPUs of a populated parent.

Factor these two checks into child_partition_error() so the same rules can
be reused when evaluating a proposed parent configuration. This is a
preparatory refactoring with no intended functional change.

Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
 1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 8f24171b6055..6994dc75d940 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	return 0;
 }
 
+/*
+ * Return the error that will invalidate a child partition under a proposed
+ * parent partition configuration.
+ */
+static enum prs_errcode
+child_partition_error(struct cpuset *child,
+		      const struct cpumask *partition_cpus,
+		      const struct cpumask *remaining_cpus,
+		      bool parent_populated)
+{
+	if (!cpumask_subset(child->effective_xcpus, partition_cpus))
+		return PERR_INVCPUS;
+
+	if (parent_populated &&
+	    cpumask_subset(remaining_cpus, child->effective_xcpus))
+		return PERR_NOCPUS;
+
+	return PERR_NONE;
+}
+
 /**
  * compute_partition_effective_cpumask - compute effective_cpus for partition
  * @cs: partition root cpuset
@@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 
 	rcu_read_lock();
 	cpuset_for_each_child(child, css, cs) {
+		enum prs_errcode child_err;
+
 		if (!is_partition_valid(child))
 			continue;
 
@@ -2129,15 +2151,13 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 		 * partition root.
 		 */
 		WARN_ON_ONCE(is_remote_partition(child));
-		WRITE_ONCE(child->prs_err, 0);
-		if (!cpumask_subset(child->effective_xcpus,
-				    cs->effective_xcpus))
-			WRITE_ONCE(child->prs_err, PERR_INVCPUS);
-		else if (populated &&
-			 cpumask_subset(new_ecpus, child->effective_xcpus))
-			WRITE_ONCE(child->prs_err, PERR_NOCPUS);
-
-		if (child->prs_err) {
+		WRITE_ONCE(child->prs_err, PERR_NONE);
+		child_err = child_partition_error(child, cs->effective_xcpus,
+						  new_ecpus, populated);
+		if (child_err)
+			WRITE_ONCE(child->prs_err, child_err);
+
+		if (child_err) {
 			int old_prs = child->partition_root_state;
 
 			/*
-- 
2.43.0

[PATCH v3 2/7] cgroup/cpuset: Account for child CPU ownership in partition changes

[Guopeng Zhang]

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

effective_xcpus includes CPUs granted to valid child partitions. A change
to the parent must not apply its isolation state or housekeeping checks to
CPUs which remain owned by those children.

For example, on a cgroup v2 system with CPUs 0-3 online:

    cd /sys/fs/cgroup
    echo +cpuset > cgroup.subtree_control
    mkdir type-repro
    echo 1-3 > type-repro/cpuset.cpus
    echo isolated > type-repro/cpuset.cpus.partition
    echo +cpuset > type-repro/cgroup.subtree_control
    mkdir type-repro/child
    echo 2-3 > type-repro/child/cpuset.cpus
    echo isolated > type-repro/child/cpuset.cpus.partition
    echo root > type-repro/cpuset.cpus.partition
    cat cpuset.cpus.isolated

The isolated mask should still contain CPUs 2-3 after the parent becomes a
root partition. Without this change, those CPUs are removed even though
the child remains isolated.

Compute the CPUs owned directly by a partition by excluding CPUs granted
to valid children. When validating a trial parent mask, exclude only CPUs
granted to children that will remain valid under that mask. Reuse the child
validation rules so PERR_INVCPUS and PERR_NOCPUS are handled consistently.

Use the directly owned mask for root/isolated type changes and housekeeping
validation. If a root child returns the last housekeeping CPU to an
isolated parent, invalidate the outermost isolated ancestor so the child
can still become a member without violating the housekeeping constraint.

Link: https://sashiko.dev/#/patchset/20260820124202.517160-1-guopeng.zhang%40linux.dev?part=6
Link: https://sashiko.dev/#/patchset/20260828095643.13395-1-guopeng.zhang@linux.dev?part=1
Fixes: 4a74e418881f ("cgroup/cpuset: Check partition conflict with housekeeping setup")
Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
Fixes: 103b08709e8a ("cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping")
Fixes: b1034a690129 ("cgroup/cpuset: Ensure domain isolated CPUs stay in root or isolated partition")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 kernel/cgroup/cpuset.c | 138 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 128 insertions(+), 10 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 6994dc75d940..19efa62bb694 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2175,6 +2175,46 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 	rcu_read_unlock();
 }
 
+/*
+ * Compute CPUs owned directly by a partition.
+ *
+ * effective_xcpus includes CPUs granted to valid child partitions. Exclude
+ * those CPUs when checking or changing this partition's type.
+ */
+static void compute_partition_owned_cpumask(struct cpuset *cs,
+					    const struct cpumask *partition_cpus,
+					    struct cpumask *owned_cpus,
+					    struct cpumask *remaining_cpus)
+{
+	struct cgroup_subsys_state *css;
+	struct cpuset *child;
+	bool populated = partition_is_populated(cs, NULL);
+
+	lockdep_assert_held(&cpuset_mutex);
+	cpumask_copy(owned_cpus, partition_cpus);
+	cpumask_and(remaining_cpus, partition_cpus, cpu_active_mask);
+
+	rcu_read_lock();
+	cpuset_for_each_child(child, css, cs) {
+		if (!is_partition_valid(child))
+			continue;
+
+		/*
+		 * A child that will become invalid under the proposed
+		 * configuration cannot retain ownership of its CPUs.
+		 */
+		if (child_partition_error(child, partition_cpus,
+					  remaining_cpus, populated))
+			continue;
+
+		cpumask_andnot(owned_cpus, owned_cpus,
+			       child->effective_xcpus);
+		cpumask_andnot(remaining_cpus, remaining_cpus,
+			       child->effective_xcpus);
+	}
+	rcu_read_unlock();
+}
+
 /*
  * update_cpumasks_hier - Update effective cpumasks and tasks in the subtree
  * @cs:  the cpuset to consider
@@ -2415,13 +2455,18 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
  * validate_partition - Validate a cpuset partition configuration
  * @cs: The cpuset to validate
  * @trialcs: The trial cpuset containing proposed configuration changes
+ * @owned_cpus: Scratch mask for CPUs owned directly by the trial partition
+ * @remaining_cpus: Scratch mask used to predict valid child partitions
  *
  * 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)
+static enum prs_errcode validate_partition(struct cpuset *cs,
+					   struct cpuset *trialcs,
+					   struct cpumask *owned_cpus,
+					   struct cpumask *remaining_cpus)
 {
 	struct cpuset *parent = parent_cs(cs);
 
@@ -2431,8 +2476,10 @@ static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *tri
 	if (cpumask_empty(trialcs->effective_xcpus))
 		return PERR_INVCPUS;
 
+	compute_partition_owned_cpumask(cs, trialcs->effective_xcpus,
+					owned_cpus, remaining_cpus);
 	if (prstate_housekeeping_conflict(trialcs->partition_root_state,
-					  trialcs->effective_xcpus))
+					  owned_cpus))
 		return PERR_HKEEPING;
 
 	if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
@@ -2458,7 +2505,8 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
 	if (cs_is_member(cs))
 		return;
 
-	prs_err = validate_partition(cs, trialcs);
+	prs_err = validate_partition(cs, trialcs, tmp->new_cpus,
+				     tmp->addmask);
 	if (prs_err) {
 		WRITE_ONCE(cs->prs_err, prs_err);
 		trialcs->prs_err = prs_err;
@@ -2937,6 +2985,48 @@ int cpuset_update_flag(cpuset_flagbits_t bit, struct cpuset *cs,
 	return err;
 }
 
+/*
+ * Invalidate the highest isolated partition that contains @cs.
+ *
+ * A root partition returning CPUs to an isolated parent can consume the last
+ * housekeeping CPU. Invalidating the whole chain returns the CPUs to a root
+ * partition instead.
+ */
+static struct cpuset *invalidate_isolated_ancestor(struct cpuset *cs,
+						   struct tmpmasks *tmp)
+{
+	struct cpuset *ancestor = parent_cs(cs);
+	struct cpuset *parent;
+	int err;
+
+	lockdep_assert_held(&cpuset_mutex);
+	if (WARN_ON_ONCE(!ancestor))
+		return NULL;
+
+	while ((ancestor != &top_cpuset) &&
+	       !is_remote_partition(ancestor)) {
+		parent = parent_cs(ancestor);
+		if (!parent ||
+		    parent->partition_root_state != PRS_ISOLATED)
+			break;
+		ancestor = parent;
+	}
+
+	if (WARN_ON_ONCE(ancestor == &top_cpuset))
+		return NULL;
+
+	WRITE_ONCE(ancestor->prs_err, PERR_HKEEPING);
+	if (is_remote_partition(ancestor)) {
+		remote_partition_disable(ancestor, tmp);
+	} else {
+		err = update_parent_effective_cpumask(ancestor,
+						      partcmd_invalidate, NULL, tmp);
+		WARN_ON_ONCE(err);
+	}
+
+	return ancestor;
+}
+
 /**
  * update_prstate - update partition_root_state
  * @cs: the cpuset to update
@@ -2949,6 +3039,8 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 {
 	int err = PERR_NONE, old_prs = cs->partition_root_state;
 	struct cpuset *parent = parent_cs(cs);
+	struct cpuset *invalidated = NULL;
+	struct cpumask *isolcpus_update_cpus = cs->effective_xcpus;
 	struct tmpmasks tmpmask;
 	bool isolcpus_updated = false;
 
@@ -3005,19 +3097,38 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	} else if (old_prs && new_prs) {
 		/*
 		 * A change in load balance state only, no change in cpumasks.
-		 * Need to update isolated_cpus.
+		 * Need to update isolated_cpus for CPUs owned by this partition,
+		 * excluding CPUs distributed to valid child partitions.
 		 */
+		compute_partition_owned_cpumask(cs, cs->effective_xcpus,
+						tmpmask.new_cpus,
+						tmpmask.addmask);
 		if (((new_prs == PRS_ISOLATED) &&
-		     !isolated_cpus_can_update(cs->effective_xcpus, NULL)) ||
-		    prstate_housekeeping_conflict(new_prs, cs->effective_xcpus))
+		     !isolated_cpus_can_update(tmpmask.new_cpus, NULL)) ||
+		    prstate_housekeeping_conflict(new_prs, tmpmask.new_cpus)) {
 			err = PERR_HKEEPING;
-		else
+		} else {
+			/*
+			 * Only directly owned CPUs change isolation state for a
+			 * successful root <-> isolated type change.
+			 */
+			isolcpus_update_cpus = tmpmask.new_cpus;
 			isolcpus_updated = true;
+		}
 	} else {
 		/*
 		 * Switching back to member is always allowed even if it
-		 * disables child partitions.
+		 * disables child partitions. If returning CPUs to an isolated
+		 * parent would consume the last housekeeping CPU, invalidate
+		 * the outermost isolated ancestor and return its CPUs instead.
 		 */
+		if (old_prs == PRS_ROOT &&
+		    parent->partition_root_state == PRS_ISOLATED &&
+		    !isolated_cpus_can_update(cs->effective_xcpus, NULL))
+			invalidated = invalidate_isolated_ancestor(cs, &tmpmask);
+		if (invalidated)
+			goto out;
+
 		if (is_remote_partition(cs))
 			remote_partition_disable(cs, &tmpmask);
 		else
@@ -3045,11 +3156,18 @@ 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(old_prs, new_prs,
+				     isolcpus_update_cpus);
 	spin_unlock_irq(&callback_lock);
 
 	/* Force update if switching back to member & update effective_xcpus */
-	update_cpumasks_hier(cs, &tmpmask, !new_prs);
+	if (invalidated) {
+		update_cpumasks_hier(invalidated, &tmpmask, false);
+		update_partition_sd_lb(invalidated, PRS_ISOLATED);
+		notify_partition_change(invalidated, PRS_ISOLATED);
+	} else {
+		update_cpumasks_hier(cs, &tmpmask, !new_prs);
+	}
 
 	/* A newly created partition must have effective_xcpus set */
 	WARN_ON_ONCE(!old_prs && (new_prs > 0)
-- 
2.43.0

[PATCH v3 3/7] selftests/cgroup: Add tests for type-change isolation accounting

[Guopeng Zhang]

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

Add two matrix cases for parent type changes with valid child partitions.
Cover root-to-isolated while the child remains root, and isolated-to-root
while the child remains isolated. Check that only CPUs owned directly by
the parent change isolation state.

Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 tools/testing/selftests/cgroup/test_cpuset_prs.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index 131d8b4551ef..9a1cce4807b4 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -224,6 +224,8 @@ TEST_MATRIX=(
 	"   C0-1     .      .   C2-3:P1   .      .      .     C2     0 "
 	"   C0-1     .      .   C2-3:P1   .      .      .    C4-5    0 B1:4-5"
 	"  C0-3:P1 C2-3:P1  .      .      .      .      .      .     0 A1:0-1|A2:2-3|XA2:2-3"
+	"  C0-3:P1 C2-3:P1  .      .     P2      .      .      .     0 A1:0-1|A2:2-3 A1:P2|A2:P1 0-1"
+	"  C0-3:P2 C2-3:P2  .      .     P1      .      .      .     0 A1:0-1|A2:2-3 A1:P1|A2:P2 2-3"
 	"  C0-3:P1 C2-3:P1  .      .     C1-3    .      .      .     0 A1:1|A2:2-3|XA2:2-3"
 	"  C2-3:P1  C3:P1   .      .     C3      .      .      .     0 A1:|A2:3|XA2:3 A1:P1|A2:P1"
 	"  C2-3:P1  C3:P1   .      .     C3      P0     .      .     0 A1:3|A2:3 A1:P1|A2:P0"
-- 
2.43.0

[PATCH v3 4/7] selftests/cgroup: Test child CPU ownership in partition changes

[Guopeng Zhang]

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

Create an isolated child which owns a boot-isolated CPU, then change its
parent's partition type and CPU mask. Verify that a valid child keeps its
CPUs and isolation state.

Also test trial parent masks which would invalidate the child because its
CPU mask is no longer covered or because it would consume every remaining
CPU of a populated parent. Verify that the returning boot-isolated CPU is
included in the parent's housekeeping check.

Cache the boot-isolated CPU during setup so the tests do not depend on the
order in which earlier test cases run.

Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 .../selftests/cgroup/test_cpuset_prs.sh       | 186 +++++++++++++++++-
 1 file changed, 182 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index 9a1cce4807b4..a3df6b094e0f 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -107,6 +107,17 @@ then
 	echo "Pre-isolated CPUs: $BOOT_ISOLCPUS"
 fi
 
+# Cache the CPUs isolated from scheduler domains at boot.
+BOOT_ISOLATED_FILE=/sys/devices/system/cpu/isolated
+BOOT_CPUS=
+BOOT_CPU=
+if [[ -r $BOOT_ISOLATED_FILE ]]
+then
+	BOOT_CPUS=$(cat $BOOT_ISOLATED_FILE)
+	[[ -n "$BOOT_CPUS" ]] &&
+		BOOT_CPU=$(echo "$BOOT_CPUS" | sed -e 's/[,-].*//')
+fi
+
 cleanup()
 {
 	online_cpus
@@ -1158,25 +1169,22 @@ test_isolated()
 }
 
 #
-# Select an online CPU isolated from scheduler domains at boot.
+# Check that a boot-isolated CPU selected during initialization is online.
 # $1: test name used in the skip message
 #
 get_boot_isolated_cpu()
 {
 	TEST_NAME=$1
-	BOOT_ISOLATED_FILE=/sys/devices/system/cpu/isolated
 
 	[[ -r $BOOT_ISOLATED_FILE ]] || {
 		echo "$TEST_NAME test SKIPPED: boot isolation state unavailable"
 		return 1
 	}
-	BOOT_CPUS=$(cat $BOOT_ISOLATED_FILE)
 	[[ -n "$BOOT_CPUS" ]] || {
 		echo "$TEST_NAME test SKIPPED: no boot-isolated CPU"
 		return 1
 	}
 
-	BOOT_CPU=$(echo "$BOOT_CPUS" | sed -e 's/[,-].*//')
 	CPU_ONLINE=/sys/devices/system/cpu/cpu${BOOT_CPU}/online
 	[[ ! -e $CPU_ONLINE || $(cat $CPU_ONLINE) -eq 1 ]] || {
 		echo "$TEST_NAME test SKIPPED: CPU $BOOT_CPU is offline"
@@ -1214,6 +1222,174 @@ test_boot_isolated()
 	echo "$TEST_NAME test PASSED."
 }
 
+#
+# A parent's type and CPU-mask changes must check only CPUs owned directly by
+# the parent, not a boot-isolated CPU owned by a valid child partition.
+#
+test_child_owned_cpus()
+{
+	TEST_NAME="Child-owned CPU type change"
+	get_boot_isolated_cpu "$TEST_NAME" || return 0
+	echo "Running $TEST_NAME test ..."
+
+	cd $CGROUP2/test
+	echo member > cpuset.cpus.partition
+	echo +cpuset > cgroup.subtree_control
+	echo 2,$BOOT_CPU > cpuset.cpus
+	[[ $(cat cpuset.cpus.effective) = "2,$BOOT_CPU" ]] || {
+		echo "$TEST_NAME test SKIPPED: CPUs 2,$BOOT_CPU are unavailable"
+		echo "" > cpuset.cpus
+		cd $CGROUP2
+		return 0
+	}
+	test_partition isolated
+	mkdir A1
+	cd A1
+	echo $BOOT_CPU > cpuset.cpus
+	test_partition isolated
+	cd ..
+	test_effective_cpus 2
+	test_partition root
+	echo 2-3,$BOOT_CPU > cpuset.cpus
+	test_effective_cpus 2-3
+	[[ $(cat cpuset.cpus.partition) = root ]] || {
+		echo "Parent partition became invalid during CPU update"
+		exit 1
+	}
+	[[ $(cat A1/cpuset.cpus.partition) = isolated ]] || {
+		echo "Child partition changed during parent update"
+		exit 1
+	}
+	check_isolcpus "." || {
+		echo "Parent update corrupted child isolation"
+		exit 1
+	}
+	cd A1
+	test_partition member
+	cd ..
+	rmdir A1
+	test_partition member
+	echo "" > cpuset.cpus
+	cd $CGROUP2
+	echo "$TEST_NAME test PASSED."
+}
+
+#
+# A child that will become invalid under a trial parent CPU mask no longer
+# owns its CPUs for the purpose of the parent's housekeeping validation.
+#
+test_trial_child_invalidation()
+{
+	TEST_NAME="Trial child partition invalidation"
+	get_boot_isolated_cpu "$TEST_NAME" || return 0
+	echo "Running $TEST_NAME test ..."
+
+	cd $CGROUP2/test
+	echo member > cpuset.cpus.partition
+	echo +cpuset > cgroup.subtree_control
+	echo 2-3,$BOOT_CPU > cpuset.cpus
+	[[ $(cat cpuset.cpus.effective) = "2-3,$BOOT_CPU" ]] || {
+		echo "$TEST_NAME test SKIPPED: CPUs 2-3,$BOOT_CPU are unavailable"
+		echo "" > cpuset.cpus
+		cd $CGROUP2
+		return 0
+	}
+
+	#
+	# The child would fail validation with PERR_INVCPUS because CPU 2 is
+	# absent from the trial parent mask. CPU $BOOT_CPU must therefore be
+	# checked as a CPU that will return to the parent, not subtracted as
+	# child-owned.
+	#
+	test_partition isolated
+	mkdir A1
+	cd A1
+	echo 2,$BOOT_CPU > cpuset.cpus
+	test_partition isolated
+	cd ..
+	test_partition root
+	test_effective_cpus 3
+	echo 3,$BOOT_CPU > cpuset.cpus
+	grep -q '^root invalid (partition config conflicts with housekeeping setup)$' \
+		cpuset.cpus.partition || {
+		echo "PERR_INVCPUS trial allowed the parent to bypass housekeeping validation"
+		exit 1
+	}
+	[[ $(cat A1/cpuset.cpus.partition) = "isolated invalid"* ]] || {
+		echo "Child did not become invalid after the parent CPU update"
+		exit 1
+	}
+	[[ $(cat $CGROUP2/cpuset.cpus.effective) = "$CPULIST" ]] || {
+		echo "PERR_INVCPUS trial did not release the parent partition CPUs"
+		exit 1
+	}
+	#
+	# An invalid isolated child retains its isolation state after its
+	# effective_xcpus is cleared. Explicitly transition CPU 2 through an
+	# isolated partition before starting the next case.
+	#
+	echo member > A1/cpuset.cpus.partition
+	rmdir A1
+	echo member > cpuset.cpus.partition
+	echo "" > cpuset.cpus
+	echo 2 > cpuset.cpus
+	test_partition isolated
+	test_partition member
+	echo "" > cpuset.cpus
+	check_isolcpus "." || {
+		echo "PERR_INVCPUS trial cleanup left CPU 2 isolated"
+		exit 1
+	}
+
+	#
+	# With a task in the parent, the child would fail validation with
+	# PERR_NOCPUS because it consumes all CPUs in the trial mask. Its
+	# boot-isolated CPU must therefore participate in the parent's
+	# housekeeping validation.
+	#
+	echo 2-3,$BOOT_CPU > cpuset.cpus
+	test_partition isolated
+	mkdir A1
+	cd A1
+	echo 2,$BOOT_CPU > cpuset.cpus
+	test_partition isolated
+	cd ..
+	test_partition root
+	test_effective_cpus 3
+	echo 0 > cgroup.procs
+	echo 2,$BOOT_CPU > cpuset.cpus
+	echo 0 > $CGROUP2/cgroup.procs
+	grep -q '^root invalid (partition config conflicts with housekeeping setup)$' \
+		cpuset.cpus.partition || {
+		echo "PERR_NOCPUS trial allowed the parent to bypass housekeeping validation"
+		exit 1
+	}
+	[[ $(cat A1/cpuset.cpus.partition) = "isolated invalid"* ]] || {
+		echo "Child did not become invalid after exhausting the parent CPUs"
+		exit 1
+	}
+	[[ $(cat $CGROUP2/cpuset.cpus.effective) = "$CPULIST" ]] || {
+		echo "PERR_NOCPUS trial did not release the parent partition CPUs"
+		exit 1
+	}
+	# Clean up the retained isolation state as in the PERR_INVCPUS case.
+	echo member > A1/cpuset.cpus.partition
+	rmdir A1
+	echo member > cpuset.cpus.partition
+	echo "" > cpuset.cpus
+	echo 2 > cpuset.cpus
+	test_partition isolated
+	test_partition member
+	echo "" > cpuset.cpus
+	check_isolcpus "." || {
+		echo "PERR_NOCPUS trial cleanup left CPU 2 isolated"
+		exit 1
+	}
+
+	cd $CGROUP2
+	echo "$TEST_NAME test PASSED."
+}
+
 #
 # Wait for inotify event for the given file and read it
 # $1: cgroup file to wait for
@@ -1286,5 +1462,7 @@ run_state_test TEST_MATRIX
 run_remote_state_test REMOTE_TEST_MATRIX
 test_isolated
 test_boot_isolated
+test_child_owned_cpus
+test_trial_child_invalidation
 test_inotify
 echo "All tests PASSED."
-- 
2.43.0

[PATCH v3 5/7] selftests/cgroup: Add tests for housekeeping CPU return to isolated parents

[Guopeng Zhang]

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

Return the last housekeeping CPU from a root child to an isolated parent.
Check that the parent becomes invalid and releases its CPUs instead of
adding the returned CPU to the isolated mask.

Repeat the test with two isolated ancestors and check that the outermost
isolated partition is invalidated.

Add the nested test cgroup to the global cleanup list so a failed
assertion does not leave the test hierarchy behind.

Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 .../selftests/cgroup/test_cpuset_prs.sh       | 149 +++++++++++++++++-
 1 file changed, 148 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index a3df6b094e0f..99cba4e5277d 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -122,7 +122,8 @@ cleanup()
 {
 	online_cpus
 	cd $CGROUP2
-	rmdir A1/A2/A3 A1/A2 A1 B1 test/A1 test/B1 test > /dev/null 2>&1
+	rmdir A1/A2/A3 A1/A2 A1 B1 \
+	      test/A1/A2 test/A1 test/B1 test > /dev/null 2>&1
 	rmdir rtest/p1/c11 rtest/p1/c12 rtest/p2/c21 \
 	      rtest/p2/c22 rtest/p1 rtest/p2 rtest > /dev/null 2>&1
 	[[ -n "$SCHED_DEBUG" ]] &&
@@ -1222,6 +1223,30 @@ test_boot_isolated()
 	echo "$TEST_NAME test PASSED."
 }
 
+# Return success if CPU $2 is present in cpulist $1.
+cpulist_contains()
+{
+	local CPULIST_ARG=$1
+	local CPU_ARG=$2
+	local ITEM FIRST LAST
+	local OLD_IFS=$IFS
+
+	IFS=,
+	for ITEM in $CPULIST_ARG
+	do
+		FIRST=${ITEM%-*}
+		LAST=${ITEM#*-}
+		[[ $ITEM != *-* ]] && LAST=$FIRST
+		if [[ $CPU_ARG -ge $FIRST && $CPU_ARG -le $LAST ]]
+		then
+			IFS=$OLD_IFS
+			return 0
+		fi
+	done
+	IFS=$OLD_IFS
+	return 1
+}
+
 #
 # A parent's type and CPU-mask changes must check only CPUs owned directly by
 # the parent, not a boot-isolated CPU owned by a valid child partition.
@@ -1390,6 +1415,127 @@ test_trial_child_invalidation()
 	echo "$TEST_NAME test PASSED."
 }
 
+#
+# Returning the last housekeeping CPU to an isolated parent must invalidate
+# the isolated partition instead of adding that CPU to the isolated mask.
+#
+test_housekeeping_cpu_return()
+{
+	TEST_NAME="Housekeeping CPU return"
+	NOHZ_FILE=/sys/devices/system/cpu/nohz_full
+	[[ -r $NOHZ_FILE ]] || {
+		echo "$TEST_NAME test SKIPPED: no nohz_full state"
+		return 0
+	}
+	NOHZ_CPUS=$(cat $NOHZ_FILE)
+	[[ -n "$NOHZ_CPUS" && "$NOHZ_CPUS" != "(null)" ]] || {
+		echo "$TEST_NAME test SKIPPED: no nohz_full CPUs"
+		return 0
+	}
+
+	HK_CPU=
+	HK_COUNT=0
+	TYPE_CPUS=()
+	for ((CPU=0; CPU < NR_CPUS; CPU++))
+	do
+		CPU_ONLINE=/sys/devices/system/cpu/cpu${CPU}/online
+		[[ ! -e $CPU_ONLINE || $(cat $CPU_ONLINE) -eq 1 ]] || continue
+		cpulist_contains "$BOOT_CPUS" $CPU && continue
+		if cpulist_contains "$NOHZ_CPUS" $CPU
+		then
+			[[ ${#TYPE_CPUS[@]} -lt 3 ]] && TYPE_CPUS+=("$CPU")
+		else
+			HK_CPU=$CPU
+			((HK_COUNT++))
+		fi
+	done
+
+	[[ $HK_COUNT -eq 1 && ${#TYPE_CPUS[@]} -ge 2 ]] || {
+		echo "$TEST_NAME test SKIPPED: requires one full housekeeping CPU"
+		return 0
+	}
+	echo "Running $TEST_NAME test ..."
+
+	cd $CGROUP2/test
+	echo member > cpuset.cpus.partition
+	echo +cpuset > cgroup.subtree_control
+	echo $HK_CPU,${TYPE_CPUS[0]},${TYPE_CPUS[1]} > cpuset.cpus
+	test_partition root
+	mkdir A1
+	cd A1
+	echo $HK_CPU > cpuset.cpus
+	test_partition root
+	cd ..
+	test_partition isolated
+	cd A1
+	test_partition member
+	cd ..
+	grep -q '^isolated invalid (partition config conflicts with housekeeping setup)$' \
+		cpuset.cpus.partition || {
+		echo "Isolated parent remained valid after housekeeping CPU return"
+		exit 1
+	}
+	[[ $(cat $CGROUP2/cpuset.cpus.effective) = "$CPULIST" ]] || {
+		echo "Housekeeping CPU return did not release the partition CPUs"
+		exit 1
+	}
+	check_isolcpus "." || {
+		echo "Housekeeping CPU was added to the isolated mask"
+		exit 1
+	}
+	rmdir A1
+	echo member > cpuset.cpus.partition
+	echo "" > cpuset.cpus
+
+	if [[ ${#TYPE_CPUS[@]} -lt 3 ]]
+	then
+		echo "Nested $TEST_NAME test SKIPPED: requires three nohz_full CPUs"
+		cd $CGROUP2
+		echo "$TEST_NAME test PASSED."
+		return 0
+	fi
+
+	# Repeat the check with two isolated ancestors.
+	echo $HK_CPU,${TYPE_CPUS[0]},${TYPE_CPUS[1]},${TYPE_CPUS[2]} > cpuset.cpus
+	test_partition root
+	mkdir A1
+	cd A1
+	echo $HK_CPU,${TYPE_CPUS[1]},${TYPE_CPUS[2]} > cpuset.cpus
+	test_partition root
+	echo +cpuset > cgroup.subtree_control
+	mkdir A2
+	cd A2
+	echo $HK_CPU > cpuset.cpus
+	test_partition root
+	cd ..
+	test_partition isolated
+	cd ..
+	test_partition isolated
+	cd A1/A2
+	test_partition member
+	cd ../..
+	grep -q '^isolated invalid (partition config conflicts with housekeeping setup)$' \
+		cpuset.cpus.partition || {
+		echo "Outermost isolated partition remained valid after housekeeping CPU return"
+		exit 1
+	}
+	[[ $(cat $CGROUP2/cpuset.cpus.effective) = "$CPULIST" ]] || {
+		echo "Nested housekeeping CPU return did not release the partition CPUs"
+		exit 1
+	}
+	check_isolcpus "." || {
+		echo "Nested housekeeping CPU return added the CPU to the isolated mask"
+		exit 1
+	}
+	rmdir A1/A2
+	echo member > A1/cpuset.cpus.partition
+	rmdir A1
+	echo member > cpuset.cpus.partition
+	echo "" > cpuset.cpus
+	cd $CGROUP2
+	echo "$TEST_NAME test PASSED."
+}
+
 #
 # Wait for inotify event for the given file and read it
 # $1: cgroup file to wait for
@@ -1464,5 +1610,6 @@ test_isolated
 test_boot_isolated
 test_child_owned_cpus
 test_trial_child_invalidation
+test_housekeeping_cpu_return
 test_inotify
 echo "All tests PASSED."
-- 
2.43.0

[PATCH v3 6/7] cgroup/cpuset: Release CPUs when type-change validation fails

[Guopeng Zhang]

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

When housekeeping validation fails during a root/isolated type change,
update_prstate() marks the requested state invalid without running the
partition-disable path. The failed partition's effective_xcpus may be
cleared, but its CPUs remain unavailable to the partition which owns the
invalidated subtree.

This can be reproduced on a cgroup v2 system booted with
isolcpus=domain,15:

    cd /sys/fs/cgroup
    echo +cpuset > cgroup.subtree_control
    mkdir type-fail-repro
    echo 15 > type-fail-repro/cpuset.cpus
    echo isolated > type-fail-repro/cpuset.cpus.partition
    echo root > type-fail-repro/cpuset.cpus.partition
    cat type-fail-repro/cpuset.cpus.partition
    cat cpuset.cpus.effective

The requested root state is recorded as invalid, but CPU 15 remains
unavailable to the top cpuset.

Run the common partition-disable path when housekeeping validation fails.
Disable remote partitions with remote_partition_disable() and return local
partition CPUs to their parent. If that would consume the last housekeeping
CPU, invalidate the outermost isolated ancestor instead.

Fixes: 103b08709e8a ("cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping")
Fixes: b1034a690129 ("cgroup/cpuset: Ensure domain isolated CPUs stay in root or isolated partition")
Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 kernel/cgroup/cpuset.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 19efa62bb694..30d9ea5f3859 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3042,6 +3042,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	struct cpuset *invalidated = NULL;
 	struct cpumask *isolcpus_update_cpus = cs->effective_xcpus;
 	struct tmpmasks tmpmask;
+	bool disable_partition = false;
 	bool isolcpus_updated = false;
 
 	if (old_prs == new_prs)
@@ -3107,6 +3108,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		     !isolated_cpus_can_update(tmpmask.new_cpus, NULL)) ||
 		    prstate_housekeeping_conflict(new_prs, tmpmask.new_cpus)) {
 			err = PERR_HKEEPING;
+			disable_partition = true;
 		} else {
 			/*
 			 * Only directly owned CPUs change isolation state for a
@@ -3122,6 +3124,10 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		 * parent would consume the last housekeeping CPU, invalidate
 		 * the outermost isolated ancestor and return its CPUs instead.
 		 */
+		disable_partition = true;
+	}
+
+	if (disable_partition) {
 		if (old_prs == PRS_ROOT &&
 		    parent->partition_root_state == PRS_ISOLATED &&
 		    !isolated_cpus_can_update(cs->effective_xcpus, NULL))
-- 
2.43.0

[PATCH v3 7/7] selftests/cgroup: Add CPU release tests for type-change validation failures

[Guopeng Zhang]

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

Exercise housekeeping validation failures for local and remote partition
type changes. Check that the requested partition type becomes invalid,
its CPUs are returned to the top cpuset and their boot-time isolation
state is preserved.

Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 .../selftests/cgroup/test_cpuset_prs.sh       | 93 +++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index 99cba4e5277d..a31dc69c6c2f 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -1536,6 +1536,97 @@ test_housekeeping_cpu_return()
 	echo "$TEST_NAME test PASSED."
 }
 
+#
+# A local type change that fails the boot housekeeping constraint must release
+# the partition CPUs and preserve their boot-isolated state.
+#
+test_partition_type_failure()
+{
+	TEST_NAME="Local partition type-change validation failure"
+	get_boot_isolated_cpu "$TEST_NAME" || return 0
+	echo "Running $TEST_NAME test ..."
+
+	cd $CGROUP2/test
+	echo member > cpuset.cpus.partition
+	echo $BOOT_CPU > cpuset.cpus
+	[[ $(cat cpuset.cpus.effective) = "$BOOT_CPU" ]] || {
+		echo "$TEST_NAME test SKIPPED: CPU $BOOT_CPU is unavailable"
+		echo "" > cpuset.cpus
+		cd $CGROUP2
+		return 0
+	}
+	test_partition isolated
+	echo root > cpuset.cpus.partition
+	grep -q '^root invalid (partition config conflicts with housekeeping setup)$' \
+		cpuset.cpus.partition || {
+		echo "Partition type failure did not produce the expected state"
+		exit 1
+	}
+	[[ $(cat $CGROUP2/cpuset.cpus.effective) = "$CPULIST" ]] || {
+		echo "Partition type failure did not release CPU $BOOT_CPU"
+		exit 1
+	}
+	check_isolcpus "." || {
+		echo "Partition type failure lost boot-isolated CPU $BOOT_CPU"
+		exit 1
+	}
+	echo member > cpuset.cpus.partition
+	echo "" > cpuset.cpus
+	cd $CGROUP2
+	echo "$TEST_NAME test PASSED."
+}
+
+#
+# Exercise the same type-change validation failure for a remote partition.
+#
+test_remote_partition_type_failure()
+{
+	TEST_NAME="Remote partition type-change validation failure"
+	get_boot_isolated_cpu "$TEST_NAME" || return 0
+	echo "Running $TEST_NAME test ..."
+
+	cd $CGROUP2/test
+	echo member > cpuset.cpus.partition
+	echo +cpuset > cgroup.subtree_control
+	echo $BOOT_CPU > cpuset.cpus
+	echo $BOOT_CPU > cpuset.cpus.exclusive
+	[[ $(cat cpuset.cpus.effective) = "$BOOT_CPU" ]] || {
+		echo "$TEST_NAME test SKIPPED: CPU $BOOT_CPU is unavailable"
+		echo "" > cpuset.cpus.exclusive
+		echo "" > cpuset.cpus
+		cd $CGROUP2
+		return 0
+	}
+	mkdir A1
+	cd A1
+	echo $BOOT_CPU > cpuset.cpus
+	echo $BOOT_CPU > cpuset.cpus.exclusive
+	test_partition isolated
+	echo root > cpuset.cpus.partition
+	grep -q '^root invalid (partition config conflicts with housekeeping setup)$' \
+		cpuset.cpus.partition || {
+		echo "Remote type failure did not produce the expected state"
+		exit 1
+	}
+	[[ $(cat $CGROUP2/cpuset.cpus.effective) = "$CPULIST" ]] || {
+		echo "Remote type failure did not release CPU $BOOT_CPU"
+		exit 1
+	}
+	check_isolcpus "." || {
+		echo "Remote type failure lost boot-isolated CPU $BOOT_CPU"
+		exit 1
+	}
+	echo member > cpuset.cpus.partition
+	echo "" > cpuset.cpus.exclusive
+	echo "" > cpuset.cpus
+	cd ..
+	rmdir A1
+	echo "" > cpuset.cpus.exclusive
+	echo "" > cpuset.cpus
+	cd $CGROUP2
+	echo "$TEST_NAME test PASSED."
+}
+
 #
 # Wait for inotify event for the given file and read it
 # $1: cgroup file to wait for
@@ -1611,5 +1702,7 @@ test_boot_isolated
 test_child_owned_cpus
 test_trial_child_invalidation
 test_housekeeping_cpu_return
+test_partition_type_failure
+test_remote_partition_type_failure
 test_inotify
 echo "All tests PASSED."
-- 
2.43.0

Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Waiman Long]

On 9/2/26 6:26 AM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> compute_partition_effective_cpumask() checks whether each valid child
> partition remains covered by the parent exclusive CPU mask and whether it
> would consume all remaining CPUs of a populated parent.
>
> Factor these two checks into child_partition_error() so the same rules can
> be reused when evaluating a proposed parent configuration. This is a
> preparatory refactoring with no intended functional change.
>
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>   1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 8f24171b6055..6994dc75d940 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   	return 0;
>   }
>   
> +/*
> + * Return the error that will invalidate a child partition under a proposed
> + * parent partition configuration.
> + */
> +static enum prs_errcode
> +child_partition_error(struct cpuset *child,
> +		      const struct cpumask *partition_cpus,
> +		      const struct cpumask *remaining_cpus,
> +		      bool parent_populated)
I think you should add some functional comments on what "partition_cpus" 
and "remaining_cpus" are supposed to be so that caller knows what to 
pass into this helper.
> +{
> +	if (!cpumask_subset(child->effective_xcpus, partition_cpus))
> +		return PERR_INVCPUS;
> +
> +	if (parent_populated &&
> +	    cpumask_subset(remaining_cpus, child->effective_xcpus))
> +		return PERR_NOCPUS;
> +
> +	return PERR_NONE;
> +}
> +
>   /**
>    * compute_partition_effective_cpumask - compute effective_cpus for partition
>    * @cs: partition root cpuset
> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   
>   	rcu_read_lock();
>   	cpuset_for_each_child(child, css, cs) {
> +		enum prs_errcode child_err;
> +
>   		if (!is_partition_valid(child))
>   			continue;
>   
> @@ -2129,15 +2151,13 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   		 * partition root.
>   		 */
>   		WARN_ON_ONCE(is_remote_partition(child));
> -		WRITE_ONCE(child->prs_err, 0);
> -		if (!cpumask_subset(child->effective_xcpus,
> -				    cs->effective_xcpus))
> -			WRITE_ONCE(child->prs_err, PERR_INVCPUS);
> -		else if (populated &&
> -			 cpumask_subset(new_ecpus, child->effective_xcpus))
> -			WRITE_ONCE(child->prs_err, PERR_NOCPUS);
> -
> -		if (child->prs_err) {
> +		WRITE_ONCE(child->prs_err, PERR_NONE);
> +		child_err = child_partition_error(child, cs->effective_xcpus,
> +						  new_ecpus, populated);
> +		if (child_err)
> +			WRITE_ONCE(child->prs_err, child_err);
> +

You can ignore the inital PERR_NONE write and always write the child_err 
value into child->prs_err.

Not big issue, just some nits.

Cheers,
Longman

> +		if (child_err) {
>   			int old_prs = child->partition_root_state;
>   
>   			/*

Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Waiman Long]

On 9/2/26 6:26 AM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> compute_partition_effective_cpumask() checks whether each valid child
> partition remains covered by the parent exclusive CPU mask and whether it
> would consume all remaining CPUs of a populated parent.
>
> Factor these two checks into child_partition_error() so the same rules can
> be reused when evaluating a proposed parent configuration. This is a
> preparatory refactoring with no intended functional change.
>
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>   1 file changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 8f24171b6055..6994dc75d940 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   	return 0;
>   }
>   
> +/*
> + * Return the error that will invalidate a child partition under a proposed
> + * parent partition configuration.
> + */
> +static enum prs_errcode
> +child_partition_error(struct cpuset *child,
> +		      const struct cpumask *partition_cpus,
> +		      const struct cpumask *remaining_cpus,
> +		      bool parent_populated)
> +{
> +	if (!cpumask_subset(child->effective_xcpus, partition_cpus))
> +		return PERR_INVCPUS;
> +
> +	if (parent_populated &&
> +	    cpumask_subset(remaining_cpus, child->effective_xcpus))
> +		return PERR_NOCPUS;
> +
> +	return PERR_NONE;
> +}
> +
>   /**
>    * compute_partition_effective_cpumask - compute effective_cpus for partition
>    * @cs: partition root cpuset
> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   
>   	rcu_read_lock();
>   	cpuset_for_each_child(child, css, cs) {
> +		enum prs_errcode child_err;
> +
>   		if (!is_partition_valid(child))
>   			continue;
>   
> @@ -2129,15 +2151,13 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   		 * partition root.
>   		 */
>   		WARN_ON_ONCE(is_remote_partition(child));
> -		WRITE_ONCE(child->prs_err, 0);
> -		if (!cpumask_subset(child->effective_xcpus,
> -				    cs->effective_xcpus))

Looking at the patch again, I now see that the current code is 
incorrect. It should be "if (!cpumask_subset(child->effective_xcpus, 
new_ecpus))" So you only need to pass the "parent_excpus" (for instance) 
to the new helper instead of passing two.

Cheers,
Longman

Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Ridong Chen]

On 9/4/2026 2:33 AM, Waiman Long wrote:
> On 9/2/26 6:26 AM, Guopeng Zhang wrote:
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> compute_partition_effective_cpumask() checks whether each valid child
>> partition remains covered by the parent exclusive CPU mask and whether it
>> would consume all remaining CPUs of a populated parent.
>>
>> Factor these two checks into child_partition_error() so the same rules can
>> be reused when evaluating a proposed parent configuration. This is a
>> preparatory refactoring with no intended functional change.
>>
>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>> ---
>>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>>   1 file changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 8f24171b6055..6994dc75d940 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct 
>> cpuset *cs, int cmd,
>>       return 0;
>>   }
>> +/*
>> + * Return the error that will invalidate a child partition under a proposed
>> + * parent partition configuration.
>> + */
>> +static enum prs_errcode
>> +child_partition_error(struct cpuset *child,
>> +              const struct cpumask *partition_cpus,
>> +              const struct cpumask *remaining_cpus,
>> +              bool parent_populated)
> I think you should add some functional comments on what "partition_cpus" and 
> "remaining_cpus" are supposed to be so that caller knows what to pass into this 
> helper.

Would it help to rename them to excpus and local_excpus?
local already implies "excluding children" (just like cgroup.stat.local), so I 
think that makes the intent clearer.

>> +{
>> +    if (!cpumask_subset(child->effective_xcpus, partition_cpus))
>> +        return PERR_INVCPUS;
>> +
>> +    if (parent_populated &&
>> +        cpumask_subset(remaining_cpus, child->effective_xcpus))
>> +        return PERR_NOCPUS;
>> +
>> +    return PERR_NONE;
>> +}
>> +
>>   /**
>>    * compute_partition_effective_cpumask - compute effective_cpus for partition
>>    * @cs: partition root cpuset
>> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct 
>> cpuset *cs,
>>       rcu_read_lock();
>>       cpuset_for_each_child(child, css, cs) {
>> +        enum prs_errcode child_err;
>> +
>>           if (!is_partition_valid(child))
>>               continue;
>> @@ -2129,15 +2151,13 @@ static void compute_partition_effective_cpumask(struct 
>> cpuset *cs,
>>            * partition root.
>>            */
>>           WARN_ON_ONCE(is_remote_partition(child));
>> -        WRITE_ONCE(child->prs_err, 0);
>> -        if (!cpumask_subset(child->effective_xcpus,
>> -                    cs->effective_xcpus))
>> -            WRITE_ONCE(child->prs_err, PERR_INVCPUS);
>> -        else if (populated &&
>> -             cpumask_subset(new_ecpus, child->effective_xcpus))
>> -            WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>> -
>> -        if (child->prs_err) {
>> +        WRITE_ONCE(child->prs_err, PERR_NONE);
>> +        child_err = child_partition_error(child, cs->effective_xcpus,
>> +                          new_ecpus, populated);
>> +        if (child_err)
>> +            WRITE_ONCE(child->prs_err, child_err);
>> +
> 
> You can ignore the inital PERR_NONE write and always write the child_err value 
> into child->prs_err.
> 
> Not big issue, just some nits.
> 
> Cheers,
> Longman
> 
>> +        if (child_err) {
>>               int old_prs = child->partition_root_state;
>>               /*
> 

-- 
Best regards
Ridong

Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Ridong Chen]

On 9/4/2026 9:25 AM, Ridong Chen wrote:
> 
> 
> On 9/4/2026 2:33 AM, Waiman Long wrote:
>> On 9/2/26 6:26 AM, Guopeng Zhang wrote:
>>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>>
>>> compute_partition_effective_cpumask() checks whether each valid child
>>> partition remains covered by the parent exclusive CPU mask and whether it
>>> would consume all remaining CPUs of a populated parent.
>>>
>>> Factor these two checks into child_partition_error() so the same rules can
>>> be reused when evaluating a proposed parent configuration. This is a
>>> preparatory refactoring with no intended functional change.
>>>
>>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>> ---
>>>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>>>   1 file changed, 29 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 8f24171b6055..6994dc75d940 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct 
>>> cpuset *cs, int cmd,
>>>       return 0;
>>>   }
>>> +/*
>>> + * Return the error that will invalidate a child partition under a proposed
>>> + * parent partition configuration.
>>> + */
>>> +static enum prs_errcode
>>> +child_partition_error(struct cpuset *child,
>>> +              const struct cpumask *partition_cpus,
>>> +              const struct cpumask *remaining_cpus,
>>> +              bool parent_populated)
>> I think you should add some functional comments on what "partition_cpus" and 
>> "remaining_cpus" are supposed to be so that caller knows what to pass into 
>> this helper.
> 
> Would it help to rename them to excpus and local_excpus?
> local already implies "excluding children" (just like cgroup.stat.local), so I 
> think that makes the intent clearer.
> 

Regarding the naming: child_partition_error is a bit odd — it sounds like it's 
validating a hierarchical child partition, but it's actually validating the 
partition itself (the child argument). The parent is only needed as context for 
the validation logic, not because we're checking a subordinate partition.

I'd suggest renaming it to something like:

```
static enum prs_errcode cs_partition_error(struct cpuset *cs, ...)

```

That would better reflect what the function actually does.

On a related note, the current partition validation logic is scattered across 
the code. I think we should consider consolidating it into a common set of 
helpers, perhaps like:


```
static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset 
*trialcs, struct cpuset *parent)
{
...
}

static enum prs_errcode validate_local_partition(struct cpuset *cs, struct 
cpuset *trialcs, struct cpuset *parent)
{
	// local partion specific check
	// call validate_partition
	
}

static enum prs_errcode validate_remote_partition(struct cpuset *cs, struct 
cpuset *trialcs, struct cpuset *parent)
{
	// remote partion specific check
	// call validate_partition
}

```


>>> +{
>>> +    if (!cpumask_subset(child->effective_xcpus, partition_cpus))
>>> +        return PERR_INVCPUS;
>>> +
>>> +    if (parent_populated &&
>>> +        cpumask_subset(remaining_cpus, child->effective_xcpus))
>>> +        return PERR_NOCPUS;
>>> +
>>> +    return PERR_NONE;
>>> +}
>>> +
>>>   /**
>>>    * compute_partition_effective_cpumask - compute effective_cpus for partition
>>>    * @cs: partition root cpuset
>>> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct 
>>> cpuset *cs,
>>>       rcu_read_lock();
>>>       cpuset_for_each_child(child, css, cs) {
>>> +        enum prs_errcode child_err;
>>> +
>>>           if (!is_partition_valid(child))
>>>               continue;
>>> @@ -2129,15 +2151,13 @@ static void 
>>> compute_partition_effective_cpumask(struct cpuset *cs,
>>>            * partition root.
>>>            */
>>>           WARN_ON_ONCE(is_remote_partition(child));
>>> -        WRITE_ONCE(child->prs_err, 0);
>>> -        if (!cpumask_subset(child->effective_xcpus,
>>> -                    cs->effective_xcpus))
>>> -            WRITE_ONCE(child->prs_err, PERR_INVCPUS);
>>> -        else if (populated &&
>>> -             cpumask_subset(new_ecpus, child->effective_xcpus))
>>> -            WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>> -
>>> -        if (child->prs_err) {
>>> +        WRITE_ONCE(child->prs_err, PERR_NONE);
>>> +        child_err = child_partition_error(child, cs->effective_xcpus,
>>> +                          new_ecpus, populated);
>>> +        if (child_err)
>>> +            WRITE_ONCE(child->prs_err, child_err);
>>> +
>>
>> You can ignore the inital PERR_NONE write and always write the child_err value 
>> into child->prs_err.
>>
>> Not big issue, just some nits.
>>
>> Cheers,
>> Longman
>>
>>> +        if (child_err) {
>>>               int old_prs = child->partition_root_state;
>>>               /*
>>
> 

-- 
Best regards
Ridong

Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Ridong Chen]

On 9/4/2026 9:25 AM, Ridong Chen wrote:
> 
> 
> On 9/4/2026 2:33 AM, Waiman Long wrote:
>> On 9/2/26 6:26 AM, Guopeng Zhang wrote:
>>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>>
>>> compute_partition_effective_cpumask() checks whether each valid child
>>> partition remains covered by the parent exclusive CPU mask and whether it
>>> would consume all remaining CPUs of a populated parent.
>>>
>>> Factor these two checks into child_partition_error() so the same rules can
>>> be reused when evaluating a proposed parent configuration. This is a
>>> preparatory refactoring with no intended functional change.
>>>
>>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>> ---
>>>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>>>   1 file changed, 29 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 8f24171b6055..6994dc75d940 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct 
>>> cpuset *cs, int cmd,
>>>       return 0;
>>>   }
>>> +/*
>>> + * Return the error that will invalidate a child partition under a proposed
>>> + * parent partition configuration.
>>> + */
>>> +static enum prs_errcode
>>> +child_partition_error(struct cpuset *child,
>>> +              const struct cpumask *partition_cpus,
>>> +              const struct cpumask *remaining_cpus,
>>> +              bool parent_populated)
>> I think you should add some functional comments on what "partition_cpus" and 
>> "remaining_cpus" are supposed to be so that caller knows what to pass into 
>> this helper.
> 
> Would it help to rename them to excpus and local_excpus?
> local already implies "excluding children" (just like cgroup.stat.local), so I 
> think that makes the intent clearer.
> 

Regarding the naming: child_partition_error is a bit odd — it sounds like it's 
validating a hierarchical child partition, but it's actually validating the 
partition itself (the child argument). The parent is only needed as context for 
the validation logic, not because we're checking a subordinate partition.

I'd suggest renaming it to something like:

```
static enum prs_errcode cs_partition_error(struct cpuset *cs, ...)

```

That would better reflect what the function actually does.

On a related note, the current partition validation logic is scattered across 
the code. I think we should consider consolidating it into a common set of 
helpers, perhaps like:


```
static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset 
*trialcs, struct cpuset *parent)
{
...
}

static enum prs_errcode validate_local_partition(struct cpuset *cs, struct 
cpuset *trialcs, struct cpuset *parent)
{
	// local partion specific check
	// call validate_partition
	
}

static enum prs_errcode validate_remote_partition(struct cpuset *cs, struct 
cpuset *trialcs, struct cpuset *parent)
{
	// remote partion specific check
	// call validate_partition
}

```


>>> +{
>>> +    if (!cpumask_subset(child->effective_xcpus, partition_cpus))
>>> +        return PERR_INVCPUS;
>>> +
>>> +    if (parent_populated &&
>>> +        cpumask_subset(remaining_cpus, child->effective_xcpus))
>>> +        return PERR_NOCPUS;
>>> +
>>> +    return PERR_NONE;
>>> +}
>>> +
>>>   /**
>>>    * compute_partition_effective_cpumask - compute effective_cpus for partition
>>>    * @cs: partition root cpuset
>>> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct 
>>> cpuset *cs,
>>>       rcu_read_lock();
>>>       cpuset_for_each_child(child, css, cs) {
>>> +        enum prs_errcode child_err;
>>> +
>>>           if (!is_partition_valid(child))
>>>               continue;
>>> @@ -2129,15 +2151,13 @@ static void 
>>> compute_partition_effective_cpumask(struct cpuset *cs,
>>>            * partition root.
>>>            */
>>>           WARN_ON_ONCE(is_remote_partition(child));
>>> -        WRITE_ONCE(child->prs_err, 0);
>>> -        if (!cpumask_subset(child->effective_xcpus,
>>> -                    cs->effective_xcpus))
>>> -            WRITE_ONCE(child->prs_err, PERR_INVCPUS);
>>> -        else if (populated &&
>>> -             cpumask_subset(new_ecpus, child->effective_xcpus))
>>> -            WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>> -
>>> -        if (child->prs_err) {
>>> +        WRITE_ONCE(child->prs_err, PERR_NONE);
>>> +        child_err = child_partition_error(child, cs->effective_xcpus,
>>> +                          new_ecpus, populated);
>>> +        if (child_err)
>>> +            WRITE_ONCE(child->prs_err, child_err);
>>> +
>>
>> You can ignore the inital PERR_NONE write and always write the child_err value 
>> into child->prs_err.
>>
>> Not big issue, just some nits.
>>
>> Cheers,
>> Longman
>>
>>> +        if (child_err) {
>>>               int old_prs = child->partition_root_state;
>>>               /*
>>
> 

-- 
Best regards
Ridong

Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Ridong Chen]

On 9/4/2026 9:25 AM, Ridong Chen wrote:
> 
> 
> On 9/4/2026 2:33 AM, Waiman Long wrote:
>> On 9/2/26 6:26 AM, Guopeng Zhang wrote:
>>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>>
>>> compute_partition_effective_cpumask() checks whether each valid child
>>> partition remains covered by the parent exclusive CPU mask and whether it
>>> would consume all remaining CPUs of a populated parent.
>>>
>>> Factor these two checks into child_partition_error() so the same rules can
>>> be reused when evaluating a proposed parent configuration. This is a
>>> preparatory refactoring with no intended functional change.
>>>
>>> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>> ---
>>>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>>>   1 file changed, 29 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 8f24171b6055..6994dc75d940 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct 
>>> cpuset *cs, int cmd,
>>>       return 0;
>>>   }
>>> +/*
>>> + * Return the error that will invalidate a child partition under a proposed
>>> + * parent partition configuration.
>>> + */
>>> +static enum prs_errcode
>>> +child_partition_error(struct cpuset *child,
>>> +              const struct cpumask *partition_cpus,
>>> +              const struct cpumask *remaining_cpus,
>>> +              bool parent_populated)
>> I think you should add some functional comments on what "partition_cpus" and 
>> "remaining_cpus" are supposed to be so that caller knows what to pass into 
>> this helper.
> 
> Would it help to rename them to excpus and local_excpus?
> local already implies "excluding children" (just like cgroup.stat.local), so I 
> think that makes the intent clearer.
> 

Regarding the naming: child_partition_error is a bit odd — it sounds like it's 
validating a hierarchical child partition, but it's actually validating the 
partition itself (the child argument). The parent is only needed as context for 
the validation logic, not because we're checking a subordinate partition.

I'd suggest renaming it to something like:

```
static enum prs_errcode cs_partition_error(struct cpuset *cs, ...)

```

That would better reflect what the function actually does.

On a related note, the current partition validation logic is scattered across 
the code. I think we should consider consolidating it into a common set of 
helpers, perhaps like:


```
static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset 
*trialcs, struct cpuset *parent)
{
...
}

static enum prs_errcode validate_local_partition(struct cpuset *cs, struct 
cpuset *trialcs, struct cpuset *parent)
{
	// local partion specific check
	// call validate_partition
	
}

static enum prs_errcode validate_remote_partition(struct cpuset *cs, struct 
cpuset *trialcs, struct cpuset *parent)
{
	// remote partion specific check
	// call validate_partition
}

```


>>> +{
>>> +    if (!cpumask_subset(child->effective_xcpus, partition_cpus))
>>> +        return PERR_INVCPUS;
>>> +
>>> +    if (parent_populated &&
>>> +        cpumask_subset(remaining_cpus, child->effective_xcpus))
>>> +        return PERR_NOCPUS;
>>> +
>>> +    return PERR_NONE;
>>> +}
>>> +
>>>   /**
>>>    * compute_partition_effective_cpumask - compute effective_cpus for partition
>>>    * @cs: partition root cpuset
>>> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct 
>>> cpuset *cs,
>>>       rcu_read_lock();
>>>       cpuset_for_each_child(child, css, cs) {
>>> +        enum prs_errcode child_err;
>>> +
>>>           if (!is_partition_valid(child))
>>>               continue;
>>> @@ -2129,15 +2151,13 @@ static void 
>>> compute_partition_effective_cpumask(struct cpuset *cs,
>>>            * partition root.
>>>            */
>>>           WARN_ON_ONCE(is_remote_partition(child));
>>> -        WRITE_ONCE(child->prs_err, 0);
>>> -        if (!cpumask_subset(child->effective_xcpus,
>>> -                    cs->effective_xcpus))
>>> -            WRITE_ONCE(child->prs_err, PERR_INVCPUS);
>>> -        else if (populated &&
>>> -             cpumask_subset(new_ecpus, child->effective_xcpus))
>>> -            WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>> -
>>> -        if (child->prs_err) {
>>> +        WRITE_ONCE(child->prs_err, PERR_NONE);
>>> +        child_err = child_partition_error(child, cs->effective_xcpus,
>>> +                          new_ecpus, populated);
>>> +        if (child_err)
>>> +            WRITE_ONCE(child->prs_err, child_err);
>>> +
>>
>> You can ignore the inital PERR_NONE write and always write the child_err value 
>> into child->prs_err.
>>
>> Not big issue, just some nits.
>>
>> Cheers,
>> Longman
>>
>>> +        if (child_err) {
>>>               int old_prs = child->partition_root_state;
>>>               /*
>>
> 

-- 
Best regards
Ridong