Linux Kernel Selftest development
 help / color / mirror / Atom feed
* [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions
@ 2026-08-28  9:56 Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes Guopeng Zhang
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-28  9:56 UTC (permalink / raw)
  To: cgroups, longman, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

Several partition type-change and validation paths use effective_xcpus,
which also includes CPUs assigned to valid child partitions. This can
change a child's isolation accounting, make a valid parent transition
fail housekeeping validation or invalidate a parent during a later
CPU-mask update.

Once type-change checks are limited to CPUs owned by the parent, a root
child can hold the last housekeeping CPU while its parent is isolated.
Returning that CPU when the child becomes a member must not consume the
last housekeeping CPU.

A type change that fails housekeeping validation can also leave CPUs
allocated to the invalid partition instead of returning them to the
partition which owns the invalidated subtree.

Use CPUs owned directly by a partition for type-change accounting and
validation. Recheck the housekeeping constraint when child CPUs return,
and release partition CPUs after failed type-change validation. Add
focused selftests for each case.

This is based on cgroup/for-7.3-fixes (87d347a8c854 "selftests/cgroup:
Add test for preserving boot-isolated CPUs").

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.
- Ran targeted A/B tests covering child-owned CPU accounting and
  validation, housekeeping CPU return, and CPU release after local and
  remote type-change validation failures.

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 (6):
  cgroup/cpuset: Respect child CPU ownership in type changes
  selftests/cgroup: Add tests for type-change isolation accounting
  selftests/cgroup: Add tests for type changes with child-owned CPUs
  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                        | 101 +++++-
 .../selftests/cgroup/test_cpuset_prs.sh       | 310 +++++++++++++++++-
 2 files changed, 398 insertions(+), 13 deletions(-)


base-commit: 87d347a8c8545a9234d1dd215023064413284c34
-- 
2.43.0

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

* [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes
  2026-08-28  9:56 [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions Guopeng Zhang
@ 2026-08-28  9:56 ` Guopeng Zhang
  2026-08-31  3:04   ` Ridong Chen
  2026-08-31 15:37   ` Waiman Long
  2026-08-28  9:56 ` [PATCH v2 2/6] selftests/cgroup: Add tests for type-change isolation accounting Guopeng Zhang
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-28  9:56 UTC (permalink / raw)
  To: cgroups, longman, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

effective_xcpus includes CPUs granted to valid child partitions. Changing
a parent between root and isolated must not apply its new isolation state
or housekeeping constraints to those CPUs.

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 subtracting the
effective_xcpus of valid children. Use this mask for type-change isolation
accounting and housekeeping checks. Apply the same ownership rule when
validating a trial CPU mask; otherwise a later CPU-mask update can mark a
parent invalid because of a boot-isolated CPU owned by a valid child.

Limiting the parent check to directly owned CPUs also allows a root child
to hold the last housekeeping CPU while its parent becomes isolated. If
the child then becomes a member, returning that CPU to the isolated parent
would violate the housekeeping constraint. The transition back to member
must remain allowed, so invalidate the outermost isolated ancestor and
return its CPUs to a root partition.

Sashiko pointed out the trial-validation and CPU-return gaps while
reviewing the original series.

Link: https://sashiko.dev/#/patchset/20260820124202.517160-1-guopeng.zhang%40linux.dev?part=6
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 | 92 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 84 insertions(+), 8 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 8f24171b6055..32a37d624c6b 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2155,6 +2155,31 @@ 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 cgroup_subsys_state *css;
+	struct cpuset *child;
+
+	lockdep_assert_held(&cpuset_mutex);
+	cpumask_copy(owned_cpus, partition_cpus);
+
+	rcu_read_lock();
+	cpuset_for_each_child(child, css, cs) {
+		if (is_partition_valid(child))
+			cpumask_andnot(owned_cpus, owned_cpus,
+				       child->effective_xcpus);
+	}
+	rcu_read_unlock();
+}
+
 /*
  * update_cpumasks_hier - Update effective cpumasks and tasks in the subtree
  * @cs:  the cpuset to consider
@@ -2401,7 +2426,9 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
  *
  * 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 cpuset *parent = parent_cs(cs);
 
@@ -2411,8 +2438,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);
 	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))
@@ -2438,7 +2467,7 @@ 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);
 	if (prs_err) {
 		WRITE_ONCE(cs->prs_err, prs_err);
 		trialcs->prs_err = prs_err;
@@ -2917,6 +2946,36 @@ 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);
+	int err;
+
+	lockdep_assert_held(&cpuset_mutex);
+	while (!is_remote_partition(ancestor) &&
+	       (parent_cs(ancestor)->partition_root_state == PRS_ISOLATED))
+		ancestor = parent_cs(ancestor);
+
+	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
@@ -2929,6 +2988,7 @@ 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 tmpmasks tmpmask;
 	bool isolcpus_updated = false;
 
@@ -2985,11 +3045,14 @@ 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);
 		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
 			isolcpus_updated = true;
@@ -2998,6 +3061,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		 * Switching back to member is always allowed even if it
 		 * disables child partitions.
 		 */
+		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
@@ -3025,11 +3095,17 @@ 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, tmpmask.new_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


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

* [PATCH v2 2/6] selftests/cgroup: Add tests for type-change isolation accounting
  2026-08-28  9:56 [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes Guopeng Zhang
@ 2026-08-28  9:56 ` Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 3/6] selftests/cgroup: Add tests for type changes with child-owned CPUs Guopeng Zhang
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-28  9:56 UTC (permalink / raw)
  To: cgroups, longman, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	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


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

* [PATCH v2 3/6] selftests/cgroup: Add tests for type changes with child-owned CPUs
  2026-08-28  9:56 [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 2/6] selftests/cgroup: Add tests for type-change isolation accounting Guopeng Zhang
@ 2026-08-28  9:56 ` Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 4/6] selftests/cgroup: Add tests for housekeeping CPU return to isolated parents Guopeng Zhang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-28  9:56 UTC (permalink / raw)
  To: cgroups, longman, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

Create an isolated child that owns a boot-isolated CPU, then change its
parent type and CPU mask. Check that the child stays isolated and the
parent remains valid.

Cache the boot-isolated CPU data during setup so this test does not depend
on an earlier test calling get_boot_isolated_cpu(). Keep the per-test check
that the selected CPU is online.

Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
---
 .../selftests/cgroup/test_cpuset_prs.sh       | 69 +++++++++++++++++--
 1 file changed, 65 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..ae27245e90bb 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,58 @@ 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."
+}
+
 #
 # Wait for inotify event for the given file and read it
 # $1: cgroup file to wait for
@@ -1286,5 +1346,6 @@ run_state_test TEST_MATRIX
 run_remote_state_test REMOTE_TEST_MATRIX
 test_isolated
 test_boot_isolated
+test_child_owned_cpus
 test_inotify
 echo "All tests PASSED."
-- 
2.43.0


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

* [PATCH v2 4/6] selftests/cgroup: Add tests for housekeeping CPU return to isolated parents
  2026-08-28  9:56 [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions Guopeng Zhang
                   ` (2 preceding siblings ...)
  2026-08-28  9:56 ` [PATCH v2 3/6] selftests/cgroup: Add tests for type changes with child-owned CPUs Guopeng Zhang
@ 2026-08-28  9:56 ` Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 5/6] cgroup/cpuset: Release CPUs when type-change validation fails Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 6/6] selftests/cgroup: Add CPU release tests for type-change validation failures Guopeng Zhang
  5 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-28  9:56 UTC (permalink / raw)
  To: cgroups, longman, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	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.

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

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index ae27245e90bb..72fe5ce48eff 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -1222,6 +1222,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.
@@ -1274,6 +1298,127 @@ test_child_owned_cpus()
 	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
@@ -1347,5 +1492,6 @@ run_remote_state_test REMOTE_TEST_MATRIX
 test_isolated
 test_boot_isolated
 test_child_owned_cpus
+test_housekeeping_cpu_return
 test_inotify
 echo "All tests PASSED."
-- 
2.43.0


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

* [PATCH v2 5/6] cgroup/cpuset: Release CPUs when type-change validation fails
  2026-08-28  9:56 [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions Guopeng Zhang
                   ` (3 preceding siblings ...)
  2026-08-28  9:56 ` [PATCH v2 4/6] selftests/cgroup: Add tests for housekeeping CPU return to isolated parents Guopeng Zhang
@ 2026-08-28  9:56 ` Guopeng Zhang
  2026-08-28  9:56 ` [PATCH v2 6/6] selftests/cgroup: Add CPU release tests for type-change validation failures Guopeng Zhang
  5 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-28  9:56 UTC (permalink / raw)
  To: cgroups, longman, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang

From: Guopeng Zhang <zhangguopeng@kylinos.cn>

When housekeeping validation fails during a root-to-isolated or
isolated-to-root change, update_prstate() records the requested partition
type as invalid but leaves its effective_xcpus allocated. Those CPUs are
not returned 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 type is recorded as invalid, but CPU 15 remains
unavailable to the top cpuset.

Use the common partition-disable path when housekeeping validation fails.
Disable a remote partition with remote_partition_disable(); for a local
partition, return effective_xcpus to its parent. If that return 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 | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 32a37d624c6b..fefc0afa93ec 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2990,6 +2990,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	struct cpuset *parent = parent_cs(cs);
 	struct cpuset *invalidated = NULL;
 	struct tmpmasks tmpmask;
+	bool disable_partition = false;
 	bool isolcpus_updated = false;
 
 	if (old_prs == new_prs)
@@ -3052,15 +3053,21 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 						tmpmask.new_cpus);
 		if (((new_prs == PRS_ISOLATED) &&
 		     !isolated_cpus_can_update(tmpmask.new_cpus, NULL)) ||
-		    prstate_housekeeping_conflict(new_prs, tmpmask.new_cpus))
+		    prstate_housekeeping_conflict(new_prs, tmpmask.new_cpus)) {
 			err = PERR_HKEEPING;
-		else
+			disable_partition = true;
+		} else {
 			isolcpus_updated = true;
+		}
 	} else {
 		/*
 		 * Switching back to member is always allowed even if it
 		 * disables child partitions.
 		 */
+		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


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

* [PATCH v2 6/6] selftests/cgroup: Add CPU release tests for type-change validation failures
  2026-08-28  9:56 [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions Guopeng Zhang
                   ` (4 preceding siblings ...)
  2026-08-28  9:56 ` [PATCH v2 5/6] cgroup/cpuset: Release CPUs when type-change validation fails Guopeng Zhang
@ 2026-08-28  9:56 ` Guopeng Zhang
  2026-09-02  9:05   ` kernel test robot
  5 siblings, 1 reply; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-28  9:56 UTC (permalink / raw)
  To: cgroups, longman, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	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 72fe5ce48eff..9b236826166c 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -1419,6 +1419,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
@@ -1493,5 +1584,7 @@ test_isolated
 test_boot_isolated
 test_child_owned_cpus
 test_housekeeping_cpu_return
+test_partition_type_failure
+test_remote_partition_type_failure
 test_inotify
 echo "All tests PASSED."
-- 
2.43.0


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

* Re: [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes
  2026-08-28  9:56 ` [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes Guopeng Zhang
@ 2026-08-31  3:04   ` Ridong Chen
  2026-08-31 10:50     ` Guopeng Zhang
  2026-08-31 15:37   ` Waiman Long
  1 sibling, 1 reply; 12+ messages in thread
From: Ridong Chen @ 2026-08-31  3:04 UTC (permalink / raw)
  To: Guopeng Zhang, cgroups, longman
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang



On 8/28/2026 5:56 PM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
> 
> effective_xcpus includes CPUs granted to valid child partitions. Changing
> a parent between root and isolated must not apply its new isolation state
> or housekeeping constraints to those CPUs.
> 
> 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 subtracting the
> effective_xcpus of valid children. Use this mask for type-change isolation
> accounting and housekeeping checks. Apply the same ownership rule when
> validating a trial CPU mask; otherwise a later CPU-mask update can mark a
> parent invalid because of a boot-isolated CPU owned by a valid child.
> 
> Limiting the parent check to directly owned CPUs also allows a root child
> to hold the last housekeeping CPU while its parent becomes isolated. If
> the child then becomes a member, returning that CPU to the isolated parent
> would violate the housekeeping constraint. The transition back to member
> must remain allowed, so invalidate the outermost isolated ancestor and
> return its CPUs to a root partition.
> 
> Sashiko pointed out the trial-validation and CPU-return gaps while
> reviewing the original series.
> 
> Link: https://sashiko.dev/#/patchset/20260820124202.517160-1-guopeng.zhang%40linux.dev?part=6
> 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 | 92 ++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 84 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 8f24171b6055..32a37d624c6b 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2155,6 +2155,31 @@ 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 cgroup_subsys_state *css;
> +	struct cpuset *child;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	cpumask_copy(owned_cpus, partition_cpus);
> +
> +	rcu_read_lock();
> +	cpuset_for_each_child(child, css, cs) {
> +		if (is_partition_valid(child))
> +			cpumask_andnot(owned_cpus, owned_cpus,
> +				       child->effective_xcpus);
> +	}
> +	rcu_read_unlock();
> +}
> +

To be honest, it took me a while to understand what this function actually does.

I think we should avoid adding terms like partition_cpus and owned_cpus, as they 
only add confusion. We already have effective_xcpus, effective_cpus, xcpus, and 
so on—introducing more terminology makes the code harder to follow.

As I understand it, this function is computing local_effective_xcpus, where 
"local" refers to the CPUs owned by this cgroup itself, excluding those 
delegated to its children. However, this is really a v1 concept, and I'm not 
sure it's appropriate to bring it into v2.

Just my two cents.

>   /*
>    * update_cpumasks_hier - Update effective cpumasks and tasks in the subtree
>    * @cs:  the cpuset to consider
> @@ -2401,7 +2426,9 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
>    *
>    * 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 cpuset *parent = parent_cs(cs);
>   
> @@ -2411,8 +2438,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);
>   	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))
> @@ -2438,7 +2467,7 @@ 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);
>   	if (prs_err) {
>   		WRITE_ONCE(cs->prs_err, prs_err);
>   		trialcs->prs_err = prs_err;
> @@ -2917,6 +2946,36 @@ 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);
> +	int err;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	while (!is_remote_partition(ancestor) &&
> +	       (parent_cs(ancestor)->partition_root_state == PRS_ISOLATED))
> +		ancestor = parent_cs(ancestor);
> +
> +	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
> @@ -2929,6 +2988,7 @@ 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 tmpmasks tmpmask;
>   	bool isolcpus_updated = false;
>   
> @@ -2985,11 +3045,14 @@ 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);
>   		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))

I don't think prstate_housekeeping_conflict is being used correctly. The 
new_cpus should not be the "local" CPUs.

>   			err = PERR_HKEEPING;
>   		else
>   			isolcpus_updated = true;
> @@ -2998,6 +3061,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   		 * Switching back to member is always allowed even if it
>   		 * disables child partitions.
>   		 */
> +		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
> @@ -3025,11 +3095,17 @@ 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, tmpmask.new_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)

-- 
Best regards
Ridong


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

* Re: [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes
  2026-08-31  3:04   ` Ridong Chen
@ 2026-08-31 10:50     ` Guopeng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-08-31 10:50 UTC (permalink / raw)
  To: Ridong Chen, cgroups, longman
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang



在 2026/8/31 11:04, Ridong Chen 写道:
> 
> 
> On 8/28/2026 5:56 PM, Guopeng Zhang wrote:
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> effective_xcpus includes CPUs granted to valid child partitions. Changing
>> a parent between root and isolated must not apply its new isolation state
>> or housekeeping constraints to those CPUs.
>>
>> 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 subtracting the
>> effective_xcpus of valid children. Use this mask for type-change isolation
>> accounting and housekeeping checks. Apply the same ownership rule when
>> validating a trial CPU mask; otherwise a later CPU-mask update can mark a
>> parent invalid because of a boot-isolated CPU owned by a valid child.
>>
>> Limiting the parent check to directly owned CPUs also allows a root child
>> to hold the last housekeeping CPU while its parent becomes isolated. If
>> the child then becomes a member, returning that CPU to the isolated parent
>> would violate the housekeeping constraint. The transition back to member
>> must remain allowed, so invalidate the outermost isolated ancestor and
>> return its CPUs to a root partition.
>>
>> Sashiko pointed out the trial-validation and CPU-return gaps while
>> reviewing the original series.
>>
>> Link: https://sashiko.dev/#/patchset/20260820124202.517160-1-guopeng.zhang%40linux.dev?part=6
>> 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 | 92 ++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 84 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 8f24171b6055..32a37d624c6b 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -2155,6 +2155,31 @@ 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 cgroup_subsys_state *css;
>> +    struct cpuset *child;
>> +
>> +    lockdep_assert_held(&cpuset_mutex);
>> +    cpumask_copy(owned_cpus, partition_cpus);
>> +
>> +    rcu_read_lock();
>> +    cpuset_for_each_child(child, css, cs) {
>> +        if (is_partition_valid(child))
>> +            cpumask_andnot(owned_cpus, owned_cpus,
>> +                       child->effective_xcpus);
>> +    }
>> +    rcu_read_unlock();
>> +}
>> +
> 
> To be honest, it took me a while to understand what this function actually does.
> 
> I think we should avoid adding terms like partition_cpus and owned_cpus, as they only add confusion. We already have effective_xcpus, effective_cpus, xcpus, and so on—introducing more terminology makes the code harder to follow.
> 
> As I understand it, this function is computing local_effective_xcpus, where "local" refers to the CPUs owned by this cgroup itself, excluding those delegated to its children. However, this is really a v1 concept, and I'm not sure it's appropriate to bring it into v2.
> 
> Just my two cents.
> 

Hi Ridong,

Thanks for your review.

I used the names partition_cpus and owned_cpus to distinguish between
the exclusive CPU allocation held by a partition root and the CPUs used
directly by the current partition. For example:

  top root
    |
    `-- parent: isolated
          cpuset.cpus: 13-15
          |
          `-- child: isolated
                cpuset.cpus: 14-15

The CPU allocation is:

  parent effective_xcpus:  13-15
  parent directly uses:    13
  child effective_xcpus:   14-15

It can also be illustrated as:

  top
    `-- parent isolated partition
          |-- CPU13             used directly by the parent
          `-- child isolated
                |-- CPU14       delegated to the child
                `-- CPU15       delegated to the child

My understanding is that this distinction comes from the cgroup v2
partition hierarchy, where a valid child partition root forms a separate
partition. If I have misunderstood the v1 concept you referred to,
please correct me.

I agree that the current names may not express this distinction clearly.
Would the following names make it more straightforward?

  compute_partition_direct_xcpus()
  partition_xcpus
  direct_xcpus

I have not found better names yet :-), so suggestions are welcome.

>>   /*
>>    * update_cpumasks_hier - Update effective cpumasks and tasks in the subtree
>>    * @cs:  the cpuset to consider
>> @@ -2401,7 +2426,9 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
>>    *
>>    * 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 cpuset *parent = parent_cs(cs);
>>   @@ -2411,8 +2438,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);
>>       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))
>> @@ -2438,7 +2467,7 @@ 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);
>>       if (prs_err) {
>>           WRITE_ONCE(cs->prs_err, prs_err);
>>           trialcs->prs_err = prs_err;
>> @@ -2917,6 +2946,36 @@ 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);
>> +    int err;
>> +
>> +    lockdep_assert_held(&cpuset_mutex);
>> +    while (!is_remote_partition(ancestor) &&
>> +           (parent_cs(ancestor)->partition_root_state == PRS_ISOLATED))
>> +        ancestor = parent_cs(ancestor);
>> +
>> +    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
>> @@ -2929,6 +2988,7 @@ 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 tmpmasks tmpmask;
>>       bool isolcpus_updated = false;
>>   @@ -2985,11 +3045,14 @@ 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);
>>           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))
> 
> I don't think prstate_housekeeping_conflict is being used correctly. The new_cpus should not be the "local" CPUs.
> 

Thanks for raising this.

I check the CPUs used directly by the current partition because, as I
understand it, a valid child partition root forms a separate partition
in cgroup v2. For example:

  parent: isolated
  effective_xcpus: 13-15
  |
  |-- CPU13                    used directly by the parent
  |
  `-- child: isolated
      effective_xcpus: 14-15
      |-- CPU14                used by the child
      `-- CPU15                used by the child, boot-isolated

The CPU allocation is:

  parent effective_xcpus:  13-15
  child effective_xcpus:   14-15
  parent directly uses:    13

When the parent changes from isolated to root, the expected result is:

  parent: root
  |
  |-- CPU13                    becomes non-isolated with the parent
  |
  `-- child: isolated
      |-- CPU14                remains isolated
      `-- CPU15                remains isolated

Under this CPU ownership model, only CPU13 is affected by the type
change. CPUs 14-15 still belong to the separate isolated child
partition. Therefore, I think the housekeeping check should apply to
CPU13 rather than the full CPU13-15 mask.

Checking the full effective_xcpus mask of 13-15 would also treat CPUs
14-15 as being affected by the parent's new state. Since CPU15 is
boot-isolated, this would reject an otherwise valid type change even
though CPU15 remains in the isolated child partition.

However, your comment also made me realize that it is not sufficient to
consider only the CPU ownership at the time of the type change. The
current implementation does not fully handle the case where the child
later becomes invalid and returns its CPUs. For example:

  parent: root
  |
  |-- CPU13
  |-- CPU14                    returned by the invalid child
  `-- CPU15                    returned by the invalid child, boot-isolated

  child: isolated invalid
      exclusive CPUs: empty

At this point, the CPUs used directly by the parent have changed from
CPU13 to CPUs 13-15, so the housekeeping check needs to be performed
again. Otherwise, boot-isolated CPU15 may be returned to a valid root
parent without revalidation.

I still need to work through the implementation details. My current
thinking is to make trial validation and hierarchy propagation use the
same child-validity rules, based on the new parent CPU mask, when
calculating CPU ownership. When an invalid child returns CPUs, the
receiving parent or the relevant ancestor partition must also be
rechecked, while keeping the isolated CPU accounting consistent.

Similar CPU ownership changes can also result from automatic update
paths such as CPU hotplug, remote CPU propagation, or invalid partition
recovery. These paths need to use the same validation rules as well. I
will continue working through the related logic and update it in the
next revision.

Thanks,
Guopeng

>>               err = PERR_HKEEPING;
>>           else
>>               isolcpus_updated = true;
>> @@ -2998,6 +3061,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>            * Switching back to member is always allowed even if it
>>            * disables child partitions.
>>            */
>> +        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
>> @@ -3025,11 +3095,17 @@ 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, tmpmask.new_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)
> 


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

* Re: [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes
  2026-08-28  9:56 ` [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes Guopeng Zhang
  2026-08-31  3:04   ` Ridong Chen
@ 2026-08-31 15:37   ` Waiman Long
  2026-09-03  1:55     ` Guopeng Zhang
  1 sibling, 1 reply; 12+ messages in thread
From: Waiman Long @ 2026-08-31 15:37 UTC (permalink / raw)
  To: Guopeng Zhang, cgroups, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang

On 8/28/26 5:56 AM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> effective_xcpus includes CPUs granted to valid child partitions. Changing
> a parent between root and isolated must not apply its new isolation state
> or housekeeping constraints to those CPUs.
>
> 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 subtracting the
> effective_xcpus of valid children. Use this mask for type-change isolation
> accounting and housekeeping checks. Apply the same ownership rule when
> validating a trial CPU mask; otherwise a later CPU-mask update can mark a
> parent invalid because of a boot-isolated CPU owned by a valid child.
>
> Limiting the parent check to directly owned CPUs also allows a root child
> to hold the last housekeeping CPU while its parent becomes isolated. If
> the child then becomes a member, returning that CPU to the isolated parent
> would violate the housekeeping constraint. The transition back to member
> must remain allowed, so invalidate the outermost isolated ancestor and
> return its CPUs to a root partition.
>
> Sashiko pointed out the trial-validation and CPU-return gaps while
> reviewing the original series.
>
> Link: https://sashiko.dev/#/patchset/20260820124202.517160-1-guopeng.zhang%40linux.dev?part=6
> 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 | 92 ++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 84 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 8f24171b6055..32a37d624c6b 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2155,6 +2155,31 @@ 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 cgroup_subsys_state *css;
> +	struct cpuset *child;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	cpumask_copy(owned_cpus, partition_cpus);
> +
> +	rcu_read_lock();
> +	cpuset_for_each_child(child, css, cs) {
> +		if (is_partition_valid(child))
> +			cpumask_andnot(owned_cpus, owned_cpus,
> +				       child->effective_xcpus);
> +	}
> +	rcu_read_unlock();
> +}
> +
>   /*
>    * update_cpumasks_hier - Update effective cpumasks and tasks in the subtree
>    * @cs:  the cpuset to consider
> @@ -2401,7 +2426,9 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
>    *
>    * 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 cpuset *parent = parent_cs(cs);
>   
> @@ -2411,8 +2438,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);
>   	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))
> @@ -2438,7 +2467,7 @@ 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);
>   	if (prs_err) {
>   		WRITE_ONCE(cs->prs_err, prs_err);
>   		trialcs->prs_err = prs_err;
> @@ -2917,6 +2946,36 @@ 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);
> +	int err;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	while (!is_remote_partition(ancestor) &&
> +	       (parent_cs(ancestor)->partition_root_state == PRS_ISOLATED))
> +		ancestor = parent_cs(ancestor);
That can be dangerous & may cause NULL pointer dereference. 
parent_cs(cs) returns NULL if cs is top_cpuset. So you should always 
check if ancestor is NULL or parent_cs(ancestor) is NULL. Assuming that 
the given cs is never the top_cpuset so the initial ancestor will not be 
NULL. Each ancestor reassignment can become NULL. So don't mix ancestor 
and parent_cs(ancestor) testing in the same compound if statement.
> +
> +	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
> @@ -2929,6 +2988,7 @@ 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 tmpmasks tmpmask;
>   	bool isolcpus_updated = false;
>   
> @@ -2985,11 +3045,14 @@ 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);
>   		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
>   			isolcpus_updated = true;
> @@ -2998,6 +3061,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   		 * Switching back to member is always allowed even if it
>   		 * disables child partitions.
>   		 */
> +		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;
> +
You are adding a new exception here. You should update the comment above 
to talk about the exception.
>   		if (is_remote_partition(cs))
>   			remote_partition_disable(cs, &tmpmask);
>   		else
> @@ -3025,11 +3095,17 @@ 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, tmpmask.new_cpus);
As I have mentioned in my comment to your v1 series, this code can be 
reached from multiple places beside switching from root to isolated and 
vice versa. So tmpmask.new_cpus may not be the "owned xcpus". So 
additional guard should be needed to decide if effective_xcpus or 
tmpmask.new_cpus should be used.
>   	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)
Cheers,
Longman


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

* Re: [PATCH v2 6/6] selftests/cgroup: Add CPU release tests for type-change validation failures
  2026-08-28  9:56 ` [PATCH v2 6/6] selftests/cgroup: Add CPU release tests for type-change validation failures Guopeng Zhang
@ 2026-09-02  9:05   ` kernel test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kernel test robot @ 2026-09-02  9:05 UTC (permalink / raw)
  To: Guopeng Zhang, cgroups, longman, ridong.chen
  Cc: llvm, oe-kbuild-all, tj, hannes, mkoutny, shuah, linux-kernel,
	linux-kselftest, Guopeng Zhang

Hi Guopeng,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 87d347a8c8545a9234d1dd215023064413284c34]

url:    https://github.com/intel-lab-lkp/linux/commits/Guopeng-Zhang/cgroup-cpuset-Respect-child-CPU-ownership-in-type-changes/20260828-175638
base:   87d347a8c8545a9234d1dd215023064413284c34
patch link:    https://lore.kernel.org/r/20260828095643.13395-7-guopeng.zhang%40linux.dev
patch subject: [PATCH v2 6/6] selftests/cgroup: Add CPU release tests for type-change validation failures
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20260902/202609021151.JjV7JZjv-lkp@intel.com/config)
compiler: clang version 22.1.8 (https://github.com/llvm/llvm-project ca7933e47d3a3451d81e72ac174dcb5aa28b59d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260902/202609021151.JjV7JZjv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202609021151.JjV7JZjv-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: kernel/cgroup/cpuset.c:2431 function parameter 'owned_cpus' not described in 'validate_partition'
>> Warning: kernel/cgroup/cpuset.c:2431 function parameter 'owned_cpus' not described in 'validate_partition'

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes
  2026-08-31 15:37   ` Waiman Long
@ 2026-09-03  1:55     ` Guopeng Zhang
  0 siblings, 0 replies; 12+ messages in thread
From: Guopeng Zhang @ 2026-09-03  1:55 UTC (permalink / raw)
  To: Waiman Long, cgroups, ridong.chen
  Cc: tj, hannes, mkoutny, shuah, linux-kernel, linux-kselftest,
	Guopeng Zhang



在 2026/8/31 23:37, Waiman Long 写道:
> On 8/28/26 5:56 AM, Guopeng Zhang wrote:
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> effective_xcpus includes CPUs granted to valid child partitions. Changing
>> a parent between root and isolated must not apply its new isolation state
>> or housekeeping constraints to those CPUs.

...

>>   +/*
>> + * 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);
>> +    int err;
>> +
>> +    lockdep_assert_held(&cpuset_mutex);
>> +    while (!is_remote_partition(ancestor) &&
>> +           (parent_cs(ancestor)->partition_root_state == PRS_ISOLATED))
>> +        ancestor = parent_cs(ancestor);
> That can be dangerous & may cause NULL pointer dereference. parent_cs(cs) returns NULL if cs is top_cpuset. So you should always check if ancestor is NULL or parent_cs(ancestor) is NULL. Assuming that the given cs is never the top_cpuset so the initial ancestor will not be NULL. Each ancestor reassignment can become NULL. So don't mix ancestor and parent_cs(ancestor) testing in the same compound if statement.
>> +

...
>> @@ -2998,6 +3061,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>            * Switching back to member is always allowed even if it
>>            * disables child partitions.
>>            */
>> +        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;
>> +
> You are adding a new exception here. You should update the comment above to talk about the exception.
>>           if (is_remote_partition(cs))
>>               remote_partition_disable(cs, &tmpmask);
>>           else
>> @@ -3025,11 +3095,17 @@ 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, tmpmask.new_cpus);
> As I have mentioned in my comment to your v1 series, this code can be reached from multiple places beside switching from root to isolated and vice versa. So tmpmask.new_cpus may not be the "owned xcpus". So additional guard should be needed to decide if effective_xcpus or tmpmask.new_cpus should be used.
>>       spin_unlock_irq(&callback_lock);

Hi Longman,

Thanks for the review.

I have addressed your comments in v3:

- rewrote the isolated-ancestor walk so that it stops safely before
  dereferencing the parent of the top cpuset;
- expanded the member-transition comment to describe the housekeeping
  exception;
- kept cs->effective_xcpus as the default isolation-accounting mask.
  The mask pointer is changed to the directly owned CPU mask only after
  that mask has been computed and validation has succeeded for a
  root-to-isolated or isolated-to-root transition.

I also reworked the trial ownership calculation after additional
feedback from Sashiko.

For example, suppose the kernel is booted with:

    isolcpus=domain,15

and the hierarchy is:

    parent root partition:    cpuset.cpus=13-15
    child isolated partition: cpuset.cpus=14-15

The parent directly owns only CPU13, while CPUs 14-15 are owned by the
child.

If the parent's CPU mask is then changed to CPU15, the child is still a
valid partition when validate_partition() examines the trial
configuration. The previous code therefore subtracts the child's current
effective_xcpus from the proposed parent mask. This removes CPU15 and
leaves the parent's trial owned mask empty, so CPU15 is not included in
the parent's housekeeping check.

After the new parent mask is applied, however, the child's {14,15} mask
is no longer a subset of the parent's {15} mask. The child is invalidated
with PERR_INVCPUS and CPU15 returns to the parent. The result is a valid
root partition directly owning a boot-isolated CPU which was hidden from
its housekeeping validation.

In v3, trial ownership no longer depends only on whether a child is
currently valid. It uses the common child-partition validation rules to
predict whether each child will remain valid under the proposed parent
mask. CPUs are subtracted from the parent's trial owned mask only for
children which will remain valid. This handles both PERR_INVCPUS and
PERR_NOCPUS consistently.

v3:
https://lore.kernel.org/all/20260902102615.79189-1-guopeng.zhang@linux.dev/

Thanks,
Guopeng



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

end of thread, other threads:[~2026-09-03  1:55 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-28  9:56 [PATCH v2 0/6] cgroup/cpuset: Fix partition type transitions Guopeng Zhang
2026-08-28  9:56 ` [PATCH v2 1/6] cgroup/cpuset: Respect child CPU ownership in type changes Guopeng Zhang
2026-08-31  3:04   ` Ridong Chen
2026-08-31 10:50     ` Guopeng Zhang
2026-08-31 15:37   ` Waiman Long
2026-09-03  1:55     ` Guopeng Zhang
2026-08-28  9:56 ` [PATCH v2 2/6] selftests/cgroup: Add tests for type-change isolation accounting Guopeng Zhang
2026-08-28  9:56 ` [PATCH v2 3/6] selftests/cgroup: Add tests for type changes with child-owned CPUs Guopeng Zhang
2026-08-28  9:56 ` [PATCH v2 4/6] selftests/cgroup: Add tests for housekeeping CPU return to isolated parents Guopeng Zhang
2026-08-28  9:56 ` [PATCH v2 5/6] cgroup/cpuset: Release CPUs when type-change validation fails Guopeng Zhang
2026-08-28  9:56 ` [PATCH v2 6/6] selftests/cgroup: Add CPU release tests for type-change validation failures Guopeng Zhang
2026-09-02  9:05   ` kernel test robot

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