* [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework
@ 2026-11-07 12:00 Ridong Chen
2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
Sashiko, Guopeng Zhang, Ridong Chen
From: Ridong Chen <chenridong@xiaomi.com>
This is the first part of the cpuset local partition rework posted as
RFC v4 [1]. The rework is split into three parts to make it easier to
review and merge:
1) fixes and preparatory cleanups (this series);
2) introduce partition_enable/disable/update() and rework the local
partition code on top of them;
3) clean up the remaining partition state handling outside these
helpers.
Patch 1 is a fix for an existing bug, found while looking into a review
comment on RFC v4: when cpuset.cpus.exclusive of an already invalid
partition root is cleared, its effective_xcpus is not reset, so its
former exclusive CPUs can't be claimed by a sibling. A test case is
added for it.
Patches 2-4 are the first three patches of RFC v4, rebased onto
next-20261104:
2) add an early empty cpumask check in partition_xcpus_add/del(), so
callers don't need to check it;
3) settle the isolation state of CPUs given back to the parent in
partition_xcpus_del(), so isolated_cpus_release() can be removed;
4) generalize validate_partition() so it can be used for both local
and remote partitions.
All patches build on their own, and test_cpuset_prs.sh passes with
this series applied.
[1] https://lore.kernel.org/all/20261101114206.2434398-1-ridong.chen@linux.dev/
Changes in v3:
- Patch 3: merge isolated_cpus_set_state() into isolated_cpus_update(),
dropping its @old_prs argument and the WARN_ON_ONCE(), and drop the
state check in partition_xcpus_add() (Waiman).
Drop the Acked-by from Guopeng as the patch changed.
- Patch 4: use top_cpuset as the parent of a remote partition and pass
the parent to tasks_nocpu_error() instead of parent_cs(cs) (Waiman).
Changes in v2:
- Patch 1: force update_cpumasks_hier() after reset_partition_data(),
so that the descendants don't keep the released exclusive CPUs, and
add a test case for it (Sashiko).
- Patch 1: fix the Fixes tags (Sashiko).
- Link to v1: https://lore.kernel.org/all/20261105125215.2205892-1-ridong.chen@linux.dev/
Changes since RFC v4:
- New patch 1 to fix the invalid partition effective_xcpus reset.
- Patch 3: only settle the isolation state of the CPUs still owned by
the parent when invalidating a child partition in
compute_partition_effective_cpumask(), as the parent may be shrunk
and not updated yet. This code is removed later in the rework.
- Rebase onto next-20261104, which includes commit 362212853cd0
("cgroup/cpuset: Fix incorrect CPU accounting with parent's effective_xcpus expansion").
Chen Ridong (2):
cpuset: add early empty cpumask check in partition_xcpus_add/del
cpuset: generalize the validate_partition() interface
Ridong Chen (2):
cgroup/cpuset: Reset partition data when an invalid partition stays
invalid
cpuset: settle isolation state when CPUs are given back to parent
kernel/cgroup/cpuset.c | 236 +++++++-----------
.../selftests/cgroup/test_cpuset_prs.sh | 6 +
2 files changed, 100 insertions(+), 142 deletions(-)
base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid
2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
@ 2026-11-07 12:00 ` Ridong Chen
2026-11-09 14:59 ` Guopeng Zhang
2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
Sashiko, Guopeng Zhang, Ridong Chen
From: Ridong Chen <chenridong@xiaomi.com>
When cpuset.cpus or cpuset.cpus.exclusive of a partition root is
changed, update_cpumask() and update_exclusive_cpumask() copy the new
effective_xcpus computed for the trial cpuset into the cpuset, and call
reset_partition_data() only if the partition was valid before and is
invalid now:
if ((old_prs > 0) && !is_partition_valid(cs))
reset_partition_data(cs);
An invalid partition root (old_prs < 0) that stays invalid is not
covered. Its effective_xcpus is then left with the trial value, which
is computed from user_xcpus() and so falls back to cpuset.cpus when
cpuset.cpus.exclusive is cleared. As an invalid partition root does not
own any exclusive CPUs, and rm_siblings_excl_cpus() treats a non-empty
effective_xcpus of a sibling without cpuset.cpus.exclusive as owned,
those CPUs can then no longer be claimed by a sibling:
# A1: cpuset.cpus=2-3, cpuset.cpus.exclusive=2-3, root, with tasks
# offline CPUs 2-3 -> A1 becomes invalid
echo > A1/cpuset.cpus.exclusive
cat A1/cpuset.cpus.exclusive.effective # 2-3, expected empty
echo 3 > B1/cpuset.cpus.exclusive # -EINVAL
Reset the partition data whenever the cpuset is a partition root, valid
or not, and ends up invalid. As effective_xcpus may be cleared by that,
also force update_cpumasks_hier() to update the subtree, so that the
effective_xcpus of the descendants doesn't keep the released CPUs. Add
test cases for it.
Fixes: de9f15e21c55 ("cpuset: use parse_cpulist for setting cpus.exclusive")
Fixes: c5866c9a007d ("cpuset: refactor CPU mask buffer parsing logic")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20261101114206.2434398-1-ridong.chen%40linux.dev?part=15
Reviewed-by: Waiman Long <longman@redhat.com>
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
kernel/cgroup/cpuset.c | 10 ++++++++--
tools/testing/selftests/cgroup/test_cpuset_prs.sh | 6 ++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 3052413e41e7..cf38e8708ddd 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2607,8 +2607,11 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
spin_lock_irq(&callback_lock);
cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
- if ((old_prs > 0) && !is_partition_valid(cs))
+ if (old_prs && !is_partition_valid(cs)) {
reset_partition_data(cs);
+ /* effective_xcpus may be cleared, update the subtree too */
+ force = true;
+ }
spin_unlock_irq(&callback_lock);
/* effective_cpus/effective_xcpus will be updated here */
@@ -2672,8 +2675,11 @@ static int update_exclusive_cpumask(struct cpuset *cs, struct cpuset *trialcs,
spin_lock_irq(&callback_lock);
cpumask_copy(cs->exclusive_cpus, trialcs->exclusive_cpus);
cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
- if ((old_prs > 0) && !is_partition_valid(cs))
+ if (old_prs && !is_partition_valid(cs)) {
reset_partition_data(cs);
+ /* effective_xcpus may be cleared, update the subtree too */
+ force = true;
+ }
spin_unlock_irq(&callback_lock);
/*
diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index bdca6c471f87..32e1e44fa0b1 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -434,6 +434,12 @@ TEST_MATRIX=(
# cpuset.cpus is allowed, but those extra CPUs are ignored.
" C2-3:P1 C3:P1 . . . C2-4 . . 0 A1:|A2:2-3 A1:P1|A2:P1"
+ # Clearing cpuset.cpus.exclusive of an invalid partition releases its
+ # exclusive CPUs, including those of its descendants, so a sibling
+ # can claim them.
+ " CX2-3:P1 . . C3 T:O2=0:O3=0:X . . X3 0 XA1:|XB1:3 A1:P-1"
+ " CX2-3:P1 X3 . C3 T:O2=0:O3=0:X . . X3 0 XA1:|XA2:|XB1:3 A1:P-1"
+
# A child cannot recover until its exclusive CPUs are available again.
" CX1-4:P1 CX3-4:P1 . . X1-2 . . . 0 A1:1-2|A2:1-2|XA2: A1:P1|A2:P-1"
" CX1-4:P1 CX3-4:P1 . . X1-2:X1-4 . . . 0 A1:1-2|A2:3-4|XA2:3-4 A1:P1|A2:P1"
base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del
2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
@ 2026-11-07 12:00 ` Ridong Chen
2026-11-09 15:48 ` Tejun Heo
2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
3 siblings, 1 reply; 16+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
Sashiko, Guopeng Zhang, Ridong Chen
From: Chen Ridong <chenridong@huawei.com>
Add a check for an empty cpumask at the start of partition_xcpus_add()
and partition_xcpus_del(). This allows the functions to return early,
avoiding unnecessary computation when there is no work to be done. With
these changes, partition_xcpus_add() and partition_xcpus_del() can be
called even if xcpus is empty. The caller no longer needs to check whether
xcpus is empty, eliminating many conditional statements.
Signed-off-by: Chen Ridong <chenridong@huawei.com>
Reviewed-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Reviewed-by: Waiman Long <longman@redhat.com>
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
kernel/cgroup/cpuset.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index cf38e8708ddd..85e5b529d144 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1315,10 +1315,13 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
{
WARN_ON_ONCE(new_prs < 0);
lockdep_assert_held(&callback_lock);
+
+ if (cpumask_empty(xcpus))
+ return;
+
if (!parent)
parent = &top_cpuset;
-
if (parent == &top_cpuset)
cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
@@ -1342,6 +1345,10 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
{
WARN_ON_ONCE(old_prs < 0);
lockdep_assert_held(&callback_lock);
+
+ if (cpumask_empty(xcpus))
+ return;
+
if (!parent)
parent = &top_cpuset;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
@ 2026-11-07 12:00 ` Ridong Chen
2026-11-09 14:51 ` Waiman Long
2026-11-09 15:20 ` Guopeng Zhang
2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
3 siblings, 2 replies; 16+ messages in thread
From: Ridong Chen @ 2026-11-07 12:00 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
Sashiko, Guopeng Zhang, Ridong Chen
From: Ridong Chen <chenridong@xiaomi.com>
partition_xcpus_del() only updates isolated_cpus when the partition that
gives back the CPUs has a different state than its parent. CPUs that were
further distributed to descendant isolated partitions are not taken into
account, so isolated_cpus_release() has to be called afterwards in
several places to walk up the ancestors and fix isolated_cpus.
Make partition_xcpus_del() always set the isolation state of the given
back CPUs to the state of the parent that receives them. As the
effective_xcpus of a partition covers the CPUs distributed to its
descendants, the isolation state of those CPUs is settled at the same
time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
compute_partition_effective_cpumask() invalidates a child partition
without going through partition_xcpus_del(), so do the same there: the
CPUs of the invalidated child take the isolation state of its parent.
As the parent may be shrunk and its effective_xcpus not updated yet,
only do so for the CPUs the parent still owns. This is to keep the
isolation state correct for now, the in-place invalidation here will be
removed later in the partition rework.
isolated_cpus_update() assumes the old and new states are different,
which no longer holds now that partition_xcpus_del() proceeds regardless
of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
it simply sets the isolation state of the given CPUs, and use it
everywhere. The state check in partition_xcpus_add() is dropped as well,
as setting the state is a no-op when the states are the same.
isolated_cpus_release() is no longer needed and is removed.
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
1 file changed, 19 insertions(+), 79 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 85e5b529d144..fc9de6e16ff5 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
}
/*
- * isolated_cpus_update - Update the isolated_cpus mask
- * @old_prs: old partition_root_state
- * @new_prs: new partition_root_state
- * @xcpus: exclusive CPUs with state change
+ * isolated_cpus_update - Set the isolation state of @xcpus
+ * @new_prs: partition root state that @xcpus will belong to
+ * @xcpus: exclusive CPUs whose isolation state may change
+ *
+ * CPUs isolated at boot time are kept isolated.
*/
-static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
+static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
{
bool updated = false;
int cpu;
- WARN_ON_ONCE(old_prs == new_prs);
lockdep_assert_held(&callback_lock);
lockdep_assert_held(&cpuset_mutex);
for_each_cpu(cpu, xcpus)
@@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
update_housekeeping = true;
}
-/*
- * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
- * @cs: the current cpuset
- * @xcpus: the set of effective xcpus to be considered for removal
- *
- * This function is called when an isolated partition is invalidated or when
- * some of its effective xcpus are to be removed from isolated_cpus due to
- * changes in parent cpuset. The given @xcpus must be modifiable as the function
- * will update its value to determine the right set of CPUs to be removed.
- */
-static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
-{
- struct cpuset *ancestor = parent_cs(cs);
- struct cpuset *iancestor = NULL;
-
- /*
- * Search the furthest isolated partition root ancestor available
- * until top_cpuset or a remote partition root is hit.
- */
- for (; ancestor; ancestor = parent_cs(ancestor)) {
- if (ancestor->partition_root_state == PRS_ISOLATED)
- iancestor = ancestor;
- if (is_remote_partition(ancestor))
- break;
- }
-
- /*
- * If a furthest isolated partition acnestor is found, use it to
- * determine which of the CPUs in @xcpus should be removed from
- * isolated_cpus. Otherwise, all of them will be removed.
- */
- if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
- return; /* No CPU should be removed from isolated_cpus */
-
- spin_lock_irq(&callback_lock);
- isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
- spin_unlock_irq(&callback_lock);
-}
-
/*
* partition_xcpus_add - Add new exclusive CPUs to partition
* @new_prs: new partition_root_state
@@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
if (parent == &top_cpuset)
cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
- if (new_prs != parent->partition_root_state)
- isolated_cpus_update(parent->partition_root_state, new_prs,
- xcpus);
+ isolated_cpus_update(new_prs, xcpus);
cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
}
@@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
if (parent == &top_cpuset)
cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
- if (old_prs != parent->partition_root_state)
- isolated_cpus_update(old_prs, parent->partition_root_state,
- xcpus);
+ /* CPUs given back, including those of descendants, take parent's state */
+ isolated_cpus_update(parent->partition_root_state, xcpus);
cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
cpumask_and(parent->effective_cpus, parent->effective_cpus, cpu_active_mask);
@@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
struct cgroup_subsys_state *css;
struct cpuset *child;
bool populated = partition_is_populated(cs, NULL);
+ static cpumask_t xcpus; /* Protected by cpuset_mutex */
/*
* Check child partition roots to see if they should be invalidated
@@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
int parent_prs = cs->partition_root_state;
/*
- * Invalidate child partition and update isolated_cpus
- * if necessary.
+ * Invalidate child partition. Its CPUs still owned by
+ * cs take the isolation state of cs. effective_xcpus
+ * of cs may not be updated yet, so recompute it.
*/
spin_lock_irq(&callback_lock);
WRITE_ONCE(child->prs_err, PERR_NOCPUS);
- if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
- isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
- child->effective_xcpus);
+ compute_excpus(cs, &xcpus);
+ cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
+ isolated_cpus_update(parent_prs, &xcpus);
child->partition_root_state = -old_prs;
spin_unlock_irq(&callback_lock);
notify_partition_change(child, old_prs);
@@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
*
* A partition error happens when parent has tasks and all
* its effective CPUs will have to be distributed out.
- *
- * For an isolated partition with a shrunk root partition parent
- * we may need to return some or all of its isolated CPUs.
*/
nocpu |= cpumask_empty(tmp->delmask);
- if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
- cpumask_clear(tmp->addmask);
- if (nocpu)
- cpumask_copy(tmp->addmask, cs->effective_xcpus);
- else if (!cpumask_subset(cs->effective_xcpus, parent->effective_xcpus))
- cpumask_andnot(tmp->addmask, cs->effective_xcpus,
- parent->effective_xcpus);
- if (!cpumask_empty(tmp->addmask))
- isolated_cpus_release(cs, tmp->addmask);
- /* Expansion of parent's effective_xcpus is handled below */
- }
if (nocpu) {
part_error = PERR_NOCPUS;
if (is_partition_valid(cs))
@@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
/*
* When parent is not a valid partition root,
* child partition roots become invalid too.
- *
- * For a to be invalidated isolated partition,
- * call return_isolated_cpus() to remove the
- * relevant CPUs from isolated_cpus.
+ * The isolation state of their CPUs has been
+ * settled when the parent gave them back.
*/
if (is_partition_valid(cp))
new_prs = -old_prs;
- if (old_prs == PRS_ISOLATED) {
- cpumask_copy(tmp->delmask, cp->effective_xcpus);
- isolated_cpus_release(cp, tmp->delmask);
- }
WRITE_ONCE(cp->prs_err,
is_partition_invalid(parent)
? PERR_INVPARENT : PERR_NOTPART);
@@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
if (!is_partition_valid(cs))
reset_partition_data(cs);
else if (isolcpus_updated)
- isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
+ isolated_cpus_update(new_prs, cs->effective_xcpus);
spin_unlock_irq(&callback_lock);
/* Force update if partition is disabled & update effective_xcpus */
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 4/4] cpuset: generalize the validate_partition() interface
2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
` (2 preceding siblings ...)
2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
@ 2026-11-07 12:01 ` Ridong Chen
2026-11-09 14:52 ` Waiman Long
3 siblings, 1 reply; 16+ messages in thread
From: Ridong Chen @ 2026-11-07 12:01 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Ridong Chen, Chen Ridong, cgroups, linux-kernel, linux-kselftest,
Sashiko, Guopeng Zhang, Ridong Chen
From: Chen Ridong <chenridong@huawei.com>
Refactor validate_partition() to take the parent partition, the new
partition root state, the new effective exclusive CPUs and the CPUs to
be deleted, instead of a trial cpuset. A NULL parent stands for a
remote partition, whose CPUs come from top_cpuset. This makes the
interface flexible enough to handle both local and remote partition
validation.
The no CPU left for tasks check now uses the given parent instead of
parent_cs(cs), and top_cpuset for a remote partition, as that is where
its CPUs are taken from. The cgroup parent of a remote partition does
not give it any CPU.
The housekeeping conflict check is folded into validate_partition(). It
is skipped when the parent is not a valid partition root, as @cs can't
be a valid local partition in that case.
Use the new interface for local partition enablement in
update_parent_effective_cpumask() and in partition_cpus_change(),
replacing the open-coded checks there. The order of the checks is now
the same for both paths: empty CPUs, housekeeping conflict, then no CPU
left for tasks.
Signed-off-by: Chen Ridong <chenridong@huawei.com>
Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
kernel/cgroup/cpuset.c | 119 ++++++++++++++++++++---------------------
1 file changed, 59 insertions(+), 60 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index fc9de6e16ff5..0ec20412b3ce 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1414,6 +1414,48 @@ static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
return res;
}
+/**
+ * validate_partition - Validate a cpuset partition configuration
+ * @cs: The cpuset to validate
+ * @parent: Parent partition of @cs, NULL for remote partition
+ * @new_prs: The proposed new partition root state
+ * @new_excpus: The new effective exclusive CPUs mask to validate
+ * @del: exclusive CPUs to be deleted from @cs, can be NULL
+ *
+ * A remote partition gets its CPUs from top_cpuset, so top_cpuset is used
+ * as the parent when @parent is NULL.
+ *
+ * The housekeeping check is skipped if @parent is not a valid partition
+ * root as @cs can't be a valid local partition in that case.
+ *
+ * Return: PRS error code (0 if valid, non-zero error code if invalid)
+ */
+static enum prs_errcode validate_partition(struct cpuset *cs,
+ struct cpuset *parent, int new_prs,
+ struct cpumask *new_excpus,
+ struct cpumask *del)
+{
+ int parent_prs;
+
+ if (new_prs == PRS_MEMBER)
+ return PERR_NONE;
+
+ if (cpumask_empty(new_excpus))
+ return PERR_INVCPUS;
+
+ if (!parent)
+ parent = &top_cpuset;
+ parent_prs = parent->partition_root_state;
+ if (parent_prs > 0 &&
+ prstate_housekeeping_conflict(new_prs, parent_prs, new_excpus, del))
+ return PERR_HKEEPING;
+
+ if (tasks_nocpu_error(parent, cs, new_excpus))
+ return PERR_NOCPUS;
+
+ return PERR_NONE;
+}
+
/*
* cpuset_update_sd_hk_unlock - Rebuild sched domains, update HK & unlock
*
@@ -1934,18 +1976,10 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
- /*
- * Enabling partition root is not allowed if its
- * effective_xcpus is empty.
- */
- if (cpumask_empty(tmp->delmask))
- return PERR_INVCPUS;
-
- if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask, NULL))
- return PERR_HKEEPING;
-
- if (nocpu)
- return PERR_NOCPUS;
+ part_error = validate_partition(cs, parent, new_prs,
+ tmp->delmask, NULL);
+ if (part_error)
+ return part_error;
/*
* This function will only be called when all the preliminary
@@ -2424,32 +2458,6 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
return 0;
}
-/**
- * validate_partition - Validate a cpuset partition configuration
- * @cs: The cpuset to validate
- * @trialcs: The trial cpuset containing proposed configuration changes
- *
- * If any validation check fails, the appropriate error code is set in the
- * cpuset's prs_err field.
- *
- * Return: PRS error code (0 if valid, non-zero error code if invalid)
- */
-static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *trialcs)
-{
- struct cpuset *parent = parent_cs(cs);
-
- if (cs_is_member(trialcs))
- return PERR_NONE;
-
- if (cpumask_empty(trialcs->effective_xcpus))
- return PERR_INVCPUS;
-
- if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
- return PERR_NOCPUS;
-
- return PERR_NONE;
-}
-
/**
* partition_cpus_change - Handle partition state changes due to CPU mask updates
* @cs: The target cpuset being modified
@@ -2463,33 +2471,24 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
struct tmpmasks *tmp)
{
enum prs_errcode prs_err;
- int parent_prs = is_remote_partition(cs)
- ? PRS_ROOT : parent_cs(cs)->partition_root_state;
+ struct cpuset *parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
if (cs_is_member(cs))
return;
- prs_err = validate_partition(cs, trialcs);
- if (!prs_err && (parent_prs > 0) &&
- !cpumask_empty(trialcs->effective_xcpus)) {
- int prs = cs->partition_root_state;
-
- /*
- * Check for housekeeping conflict by passing the whole
- * effective_xcpus as well as those CPUs that are to be
- * removed from cs as delmask.
- */
- if (prs > 0)
- cpumask_andnot(tmp->delmask, cs->effective_xcpus,
- trialcs->effective_xcpus);
- else
- cpumask_clear(tmp->delmask);
-
- if (prstate_housekeeping_conflict(prs, parent_prs,
- trialcs->effective_xcpus, tmp->delmask))
- prs_err = PERR_HKEEPING;
- }
+ /*
+ * Check for housekeeping conflict by passing the whole
+ * effective_xcpus as well as those CPUs that are to be
+ * removed from cs as delmask.
+ */
+ if (is_partition_valid(cs))
+ cpumask_andnot(tmp->delmask, cs->effective_xcpus,
+ trialcs->effective_xcpus);
+ else
+ cpumask_clear(tmp->delmask);
+ prs_err = validate_partition(cs, parent, trialcs->partition_root_state,
+ trialcs->effective_xcpus, tmp->delmask);
if (prs_err) {
WRITE_ONCE(cs->prs_err, prs_err);
trialcs->prs_err = prs_err;
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
@ 2026-11-09 14:51 ` Waiman Long
2026-11-10 3:14 ` Ridong Chen
2026-11-09 15:20 ` Guopeng Zhang
1 sibling, 1 reply; 16+ messages in thread
From: Waiman Long @ 2026-11-09 14:51 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
On 11/7/26 7:00 AM, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
>
> partition_xcpus_del() only updates isolated_cpus when the partition that
> gives back the CPUs has a different state than its parent. CPUs that were
> further distributed to descendant isolated partitions are not taken into
> account, so isolated_cpus_release() has to be called afterwards in
> several places to walk up the ancestors and fix isolated_cpus.
>
> Make partition_xcpus_del() always set the isolation state of the given
> back CPUs to the state of the parent that receives them. As the
> effective_xcpus of a partition covers the CPUs distributed to its
> descendants, the isolation state of those CPUs is settled at the same
> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>
> compute_partition_effective_cpumask() invalidates a child partition
> without going through partition_xcpus_del(), so do the same there: the
> CPUs of the invalidated child take the isolation state of its parent.
>
> As the parent may be shrunk and its effective_xcpus not updated yet,
> only do so for the CPUs the parent still owns. This is to keep the
> isolation state correct for now, the in-place invalidation here will be
> removed later in the partition rework.
>
> isolated_cpus_update() assumes the old and new states are different,
> which no longer holds now that partition_xcpus_del() proceeds regardless
> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
> it simply sets the isolation state of the given CPUs, and use it
> everywhere. The state check in partition_xcpus_add() is dropped as well,
> as setting the state is a no-op when the states are the same.
>
> isolated_cpus_release() is no longer needed and is removed.
>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
> kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
> 1 file changed, 19 insertions(+), 79 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 85e5b529d144..fc9de6e16ff5 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
> }
>
> /*
> - * isolated_cpus_update - Update the isolated_cpus mask
> - * @old_prs: old partition_root_state
> - * @new_prs: new partition_root_state
> - * @xcpus: exclusive CPUs with state change
> + * isolated_cpus_update - Set the isolation state of @xcpus
> + * @new_prs: partition root state that @xcpus will belong to
> + * @xcpus: exclusive CPUs whose isolation state may change
> + *
> + * CPUs isolated at boot time are kept isolated.
> */
> -static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
> {
> bool updated = false;
> int cpu;
>
> - WARN_ON_ONCE(old_prs == new_prs);
> lockdep_assert_held(&callback_lock);
> lockdep_assert_held(&cpuset_mutex);
> for_each_cpu(cpu, xcpus)
> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
> update_housekeeping = true;
> }
>
> -/*
> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
> - * @cs: the current cpuset
> - * @xcpus: the set of effective xcpus to be considered for removal
> - *
> - * This function is called when an isolated partition is invalidated or when
> - * some of its effective xcpus are to be removed from isolated_cpus due to
> - * changes in parent cpuset. The given @xcpus must be modifiable as the function
> - * will update its value to determine the right set of CPUs to be removed.
> - */
> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
> -{
> - struct cpuset *ancestor = parent_cs(cs);
> - struct cpuset *iancestor = NULL;
> -
> - /*
> - * Search the furthest isolated partition root ancestor available
> - * until top_cpuset or a remote partition root is hit.
> - */
> - for (; ancestor; ancestor = parent_cs(ancestor)) {
> - if (ancestor->partition_root_state == PRS_ISOLATED)
> - iancestor = ancestor;
> - if (is_remote_partition(ancestor))
> - break;
> - }
> -
> - /*
> - * If a furthest isolated partition acnestor is found, use it to
> - * determine which of the CPUs in @xcpus should be removed from
> - * isolated_cpus. Otherwise, all of them will be removed.
> - */
> - if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
> - return; /* No CPU should be removed from isolated_cpus */
> -
> - spin_lock_irq(&callback_lock);
> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
> - spin_unlock_irq(&callback_lock);
> -}
> -
> /*
> * partition_xcpus_add - Add new exclusive CPUs to partition
> * @new_prs: new partition_root_state
> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
> if (parent == &top_cpuset)
> cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>
> - if (new_prs != parent->partition_root_state)
> - isolated_cpus_update(parent->partition_root_state, new_prs,
> - xcpus);
> + isolated_cpus_update(new_prs, xcpus);
>
> cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
> }
> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
> if (parent == &top_cpuset)
> cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>
> - if (old_prs != parent->partition_root_state)
> - isolated_cpus_update(old_prs, parent->partition_root_state,
> - xcpus);
> + /* CPUs given back, including those of descendants, take parent's state */
> + isolated_cpus_update(parent->partition_root_state, xcpus);
>
> cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
> cpumask_and(parent->effective_cpus, parent->effective_cpus, cpu_active_mask);
> @@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
> struct cgroup_subsys_state *css;
> struct cpuset *child;
> bool populated = partition_is_populated(cs, NULL);
> + static cpumask_t xcpus; /* Protected by cpuset_mutex */
>
> /*
> * Check child partition roots to see if they should be invalidated
> @@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
> int parent_prs = cs->partition_root_state;
>
> /*
> - * Invalidate child partition and update isolated_cpus
> - * if necessary.
> + * Invalidate child partition. Its CPUs still owned by
> + * cs take the isolation state of cs. effective_xcpus
> + * of cs may not be updated yet, so recompute it.
> */
> spin_lock_irq(&callback_lock);
> WRITE_ONCE(child->prs_err, PERR_NOCPUS);
> - if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
> - child->effective_xcpus);
> + compute_excpus(cs, &xcpus);
> + cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
> + isolated_cpus_update(parent_prs, &xcpus);
> child->partition_root_state = -old_prs;
> spin_unlock_irq(&callback_lock);
> notify_partition_change(child, old_prs);
> @@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
> *
> * A partition error happens when parent has tasks and all
> * its effective CPUs will have to be distributed out.
> - *
> - * For an isolated partition with a shrunk root partition parent
> - * we may need to return some or all of its isolated CPUs.
> */
> nocpu |= cpumask_empty(tmp->delmask);
> - if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
> - cpumask_clear(tmp->addmask);
> - if (nocpu)
> - cpumask_copy(tmp->addmask, cs->effective_xcpus);
> - else if (!cpumask_subset(cs->effective_xcpus, parent->effective_xcpus))
> - cpumask_andnot(tmp->addmask, cs->effective_xcpus,
> - parent->effective_xcpus);
> - if (!cpumask_empty(tmp->addmask))
> - isolated_cpus_release(cs, tmp->addmask);
> - /* Expansion of parent's effective_xcpus is handled below */
> - }
> if (nocpu) {
> part_error = PERR_NOCPUS;
> if (is_partition_valid(cs))
> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
> /*
> * When parent is not a valid partition root,
> * child partition roots become invalid too.
> - *
> - * For a to be invalidated isolated partition,
> - * call return_isolated_cpus() to remove the
> - * relevant CPUs from isolated_cpus.
> + * The isolation state of their CPUs has been
> + * settled when the parent gave them back.
> */
> if (is_partition_valid(cp))
> new_prs = -old_prs;
>
> - if (old_prs == PRS_ISOLATED) {
> - cpumask_copy(tmp->delmask, cp->effective_xcpus);
> - isolated_cpus_release(cp, tmp->delmask);
> - }
> WRITE_ONCE(cp->prs_err,
> is_partition_invalid(parent)
> ? PERR_INVPARENT : PERR_NOTPART);
> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
> if (!is_partition_valid(cs))
> reset_partition_data(cs);
> else if (isolcpus_updated)
> - isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
> + isolated_cpus_update(new_prs, cs->effective_xcpus);
> spin_unlock_irq(&callback_lock);
>
> /* Force update if partition is disabled & update effective_xcpus */
The code change looks good to me. However this is not just a code
restructuring, it also includes a bug fix. I wonder if we should clarify
that and maybe add a Fixes tag.
Cheers,
Longman
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] cpuset: generalize the validate_partition() interface
2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
@ 2026-11-09 14:52 ` Waiman Long
2026-11-10 6:36 ` Ridong Chen
0 siblings, 1 reply; 16+ messages in thread
From: Waiman Long @ 2026-11-09 14:52 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
On 11/7/26 7:01 AM, Ridong Chen wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Refactor validate_partition() to take the parent partition, the new
> partition root state, the new effective exclusive CPUs and the CPUs to
> be deleted, instead of a trial cpuset. A NULL parent stands for a
> remote partition, whose CPUs come from top_cpuset. This makes the
> interface flexible enough to handle both local and remote partition
> validation.
>
> The no CPU left for tasks check now uses the given parent instead of
> parent_cs(cs), and top_cpuset for a remote partition, as that is where
> its CPUs are taken from. The cgroup parent of a remote partition does
> not give it any CPU.
>
> The housekeeping conflict check is folded into validate_partition(). It
> is skipped when the parent is not a valid partition root, as @cs can't
> be a valid local partition in that case.
>
> Use the new interface for local partition enablement in
> update_parent_effective_cpumask() and in partition_cpus_change(),
> replacing the open-coded checks there. The order of the checks is now
> the same for both paths: empty CPUs, housekeeping conflict, then no CPU
> left for tasks.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
> kernel/cgroup/cpuset.c | 119 ++++++++++++++++++++---------------------
> 1 file changed, 59 insertions(+), 60 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index fc9de6e16ff5..0ec20412b3ce 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1414,6 +1414,48 @@ static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
> return res;
> }
>
> +/**
> + * validate_partition - Validate a cpuset partition configuration
> + * @cs: The cpuset to validate
> + * @parent: Parent partition of @cs, NULL for remote partition
> + * @new_prs: The proposed new partition root state
> + * @new_excpus: The new effective exclusive CPUs mask to validate
> + * @del: exclusive CPUs to be deleted from @cs, can be NULL
> + *
> + * A remote partition gets its CPUs from top_cpuset, so top_cpuset is used
> + * as the parent when @parent is NULL.
> + *
> + * The housekeeping check is skipped if @parent is not a valid partition
> + * root as @cs can't be a valid local partition in that case.
> + *
> + * Return: PRS error code (0 if valid, non-zero error code if invalid)
> + */
> +static enum prs_errcode validate_partition(struct cpuset *cs,
> + struct cpuset *parent, int new_prs,
> + struct cpumask *new_excpus,
> + struct cpumask *del)
> +{
> + int parent_prs;
> +
> + if (new_prs == PRS_MEMBER)
> + return PERR_NONE;
> +
> + if (cpumask_empty(new_excpus))
> + return PERR_INVCPUS;
> +
> + if (!parent)
> + parent = &top_cpuset;
> + parent_prs = parent->partition_root_state;
> + if (parent_prs > 0 &&
> + prstate_housekeeping_conflict(new_prs, parent_prs, new_excpus, del))
> + return PERR_HKEEPING;
> +
> + if (tasks_nocpu_error(parent, cs, new_excpus))
> + return PERR_NOCPUS;
> +
> + return PERR_NONE;
> +}
> +
> /*
> * cpuset_update_sd_hk_unlock - Rebuild sched domains, update HK & unlock
> *
> @@ -1934,18 +1976,10 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
> if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
> new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
>
> - /*
> - * Enabling partition root is not allowed if its
> - * effective_xcpus is empty.
> - */
> - if (cpumask_empty(tmp->delmask))
> - return PERR_INVCPUS;
> -
> - if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask, NULL))
> - return PERR_HKEEPING;
> -
> - if (nocpu)
> - return PERR_NOCPUS;
> + part_error = validate_partition(cs, parent, new_prs,
> + tmp->delmask, NULL);
> + if (part_error)
> + return part_error;
>
> /*
> * This function will only be called when all the preliminary
> @@ -2424,32 +2458,6 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
> return 0;
> }
>
> -/**
> - * validate_partition - Validate a cpuset partition configuration
> - * @cs: The cpuset to validate
> - * @trialcs: The trial cpuset containing proposed configuration changes
> - *
> - * If any validation check fails, the appropriate error code is set in the
> - * cpuset's prs_err field.
> - *
> - * Return: PRS error code (0 if valid, non-zero error code if invalid)
> - */
> -static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *trialcs)
> -{
> - struct cpuset *parent = parent_cs(cs);
> -
> - if (cs_is_member(trialcs))
> - return PERR_NONE;
> -
> - if (cpumask_empty(trialcs->effective_xcpus))
> - return PERR_INVCPUS;
> -
> - if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
> - return PERR_NOCPUS;
> -
> - return PERR_NONE;
> -}
> -
> /**
> * partition_cpus_change - Handle partition state changes due to CPU mask updates
> * @cs: The target cpuset being modified
> @@ -2463,33 +2471,24 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
> struct tmpmasks *tmp)
> {
> enum prs_errcode prs_err;
> - int parent_prs = is_remote_partition(cs)
> - ? PRS_ROOT : parent_cs(cs)->partition_root_state;
> + struct cpuset *parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>
> if (cs_is_member(cs))
> return;
>
> - prs_err = validate_partition(cs, trialcs);
> - if (!prs_err && (parent_prs > 0) &&
> - !cpumask_empty(trialcs->effective_xcpus)) {
> - int prs = cs->partition_root_state;
> -
> - /*
> - * Check for housekeeping conflict by passing the whole
> - * effective_xcpus as well as those CPUs that are to be
> - * removed from cs as delmask.
> - */
> - if (prs > 0)
> - cpumask_andnot(tmp->delmask, cs->effective_xcpus,
> - trialcs->effective_xcpus);
> - else
> - cpumask_clear(tmp->delmask);
> -
> - if (prstate_housekeeping_conflict(prs, parent_prs,
> - trialcs->effective_xcpus, tmp->delmask))
> - prs_err = PERR_HKEEPING;
> - }
> + /*
> + * Check for housekeeping conflict by passing the whole
> + * effective_xcpus as well as those CPUs that are to be
> + * removed from cs as delmask.
> + */
> + if (is_partition_valid(cs))
> + cpumask_andnot(tmp->delmask, cs->effective_xcpus,
> + trialcs->effective_xcpus);
> + else
> + cpumask_clear(tmp->delmask);
>
> + prs_err = validate_partition(cs, parent, trialcs->partition_root_state,
> + trialcs->effective_xcpus, tmp->delmask);
> if (prs_err) {
> WRITE_ONCE(cs->prs_err, prs_err);
> trialcs->prs_err = prs_err;
Again, this code change looks good to me. Similar to previous patch, it
includes both code restructuring and bug fixes.
Cheers,
Longman
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid
2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
@ 2026-11-09 14:59 ` Guopeng Zhang
0 siblings, 0 replies; 16+ messages in thread
From: Guopeng Zhang @ 2026-11-09 14:59 UTC (permalink / raw)
To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
Michal Koutný,
Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
在 2026/11/7 20:00, Ridong Chen 写道:
> From: Ridong Chen <chenridong@xiaomi.com>
>
> When cpuset.cpus or cpuset.cpus.exclusive of a partition root is
> changed, update_cpumask() and update_exclusive_cpumask() copy the new
> effective_xcpus computed for the trial cpuset into the cpuset, and call
> reset_partition_data() only if the partition was valid before and is
> invalid now:
>
> if ((old_prs > 0) && !is_partition_valid(cs))
> reset_partition_data(cs);
>
> An invalid partition root (old_prs < 0) that stays invalid is not
> covered. Its effective_xcpus is then left with the trial value, which
> is computed from user_xcpus() and so falls back to cpuset.cpus when
> cpuset.cpus.exclusive is cleared. As an invalid partition root does not
> own any exclusive CPUs, and rm_siblings_excl_cpus() treats a non-empty
> effective_xcpus of a sibling without cpuset.cpus.exclusive as owned,
> those CPUs can then no longer be claimed by a sibling:
>
> # A1: cpuset.cpus=2-3, cpuset.cpus.exclusive=2-3, root, with tasks
> # offline CPUs 2-3 -> A1 becomes invalid
> echo > A1/cpuset.cpus.exclusive
> cat A1/cpuset.cpus.exclusive.effective # 2-3, expected empty
> echo 3 > B1/cpuset.cpus.exclusive # -EINVAL
>
> Reset the partition data whenever the cpuset is a partition root, valid
> or not, and ends up invalid. As effective_xcpus may be cleared by that,
> also force update_cpumasks_hier() to update the subtree, so that the
> effective_xcpus of the descendants doesn't keep the released CPUs. Add
> test cases for it.
>
> Fixes: de9f15e21c55 ("cpuset: use parse_cpulist for setting cpus.exclusive")
> Fixes: c5866c9a007d ("cpuset: refactor CPU mask buffer parsing logic")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20261101114206.2434398-1-ridong.chen%40linux.dev?part=15
> Reviewed-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
> kernel/cgroup/cpuset.c | 10 ++++++++--
> tools/testing/selftests/cgroup/test_cpuset_prs.sh | 6 ++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 3052413e41e7..cf38e8708ddd 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2607,8 +2607,11 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
> spin_lock_irq(&callback_lock);
> cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
> cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
> - if ((old_prs > 0) && !is_partition_valid(cs))
> + if (old_prs && !is_partition_valid(cs)) {
> reset_partition_data(cs);
> + /* effective_xcpus may be cleared, update the subtree too */
> + force = true;
> + }
> spin_unlock_irq(&callback_lock);
>
> /* effective_cpus/effective_xcpus will be updated here */
> @@ -2672,8 +2675,11 @@ static int update_exclusive_cpumask(struct cpuset *cs, struct cpuset *trialcs,
> spin_lock_irq(&callback_lock);
> cpumask_copy(cs->exclusive_cpus, trialcs->exclusive_cpus);
> cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
> - if ((old_prs > 0) && !is_partition_valid(cs))
> + if (old_prs && !is_partition_valid(cs)) {
> reset_partition_data(cs);
> + /* effective_xcpus may be cleared, update the subtree too */
> + force = true;
> + }
> spin_unlock_irq(&callback_lock);
>
> /*
> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index bdca6c471f87..32e1e44fa0b1 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -434,6 +434,12 @@ TEST_MATRIX=(
> # cpuset.cpus is allowed, but those extra CPUs are ignored.
> " C2-3:P1 C3:P1 . . . C2-4 . . 0 A1:|A2:2-3 A1:P1|A2:P1"
>
> + # Clearing cpuset.cpus.exclusive of an invalid partition releases its
> + # exclusive CPUs, including those of its descendants, so a sibling
> + # can claim them.
> + " CX2-3:P1 . . C3 T:O2=0:O3=0:X . . X3 0 XA1:|XB1:3 A1:P-1"
> + " CX2-3:P1 X3 . C3 T:O2=0:O3=0:X . . X3 0 XA1:|XA2:|XB1:3 A1:P-1"
> +
> # A child cannot recover until its exclusive CPUs are available again.
> " CX1-4:P1 CX3-4:P1 . . X1-2 . . . 0 A1:1-2|A2:1-2|XA2: A1:P1|A2:P-1"
> " CX1-4:P1 CX3-4:P1 . . X1-2:X1-4 . . . 0 A1:1-2|A2:3-4|XA2:3-4 A1:P1|A2:P1"
>
> base-commit: 8b7179210b22f8e48a82c5ceacb9d56a9c4cfec4
Reviewed-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Tested-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Thanks,
Guopeng
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
2026-11-09 14:51 ` Waiman Long
@ 2026-11-09 15:20 ` Guopeng Zhang
2026-11-10 6:24 ` Ridong Chen
1 sibling, 1 reply; 16+ messages in thread
From: Guopeng Zhang @ 2026-11-09 15:20 UTC (permalink / raw)
To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
Michal Koutný,
Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
在 2026/11/7 20:00, Ridong Chen 写道:
> From: Ridong Chen <chenridong@xiaomi.com>
>
> partition_xcpus_del() only updates isolated_cpus when the partition that
> gives back the CPUs has a different state than its parent. CPUs that were
> further distributed to descendant isolated partitions are not taken into
> account, so isolated_cpus_release() has to be called afterwards in
> several places to walk up the ancestors and fix isolated_cpus.
>
> Make partition_xcpus_del() always set the isolation state of the given
> back CPUs to the state of the parent that receives them. As the
> effective_xcpus of a partition covers the CPUs distributed to its
> descendants, the isolation state of those CPUs is settled at the same
> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>
> compute_partition_effective_cpumask() invalidates a child partition
> without going through partition_xcpus_del(), so do the same there: the
> CPUs of the invalidated child take the isolation state of its parent.
>
> As the parent may be shrunk and its effective_xcpus not updated yet,
> only do so for the CPUs the parent still owns. This is to keep the
> isolation state correct for now, the in-place invalidation here will be
> removed later in the partition rework.
>
> isolated_cpus_update() assumes the old and new states are different,
> which no longer holds now that partition_xcpus_del() proceeds regardless
> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
> it simply sets the isolation state of the given CPUs, and use it
> everywhere. The state check in partition_xcpus_add() is dropped as well,
> as setting the state is a no-op when the states are the same.
>
> isolated_cpus_release() is no longer needed and is removed.
>
Hi Ridong,
Would it be worth briefly mentioning in the commit message that this patch also fixes the issue described here?
https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
Thanks,
Guopeng
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del
2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
@ 2026-11-09 15:48 ` Tejun Heo
0 siblings, 0 replies; 16+ messages in thread
From: Tejun Heo @ 2026-11-09 15:48 UTC (permalink / raw)
To: Ridong Chen
Cc: Waiman Long, Johannes Weiner, Michal Koutný,
Shuah Khan, Chen Ridong, Ridong Chen, Guopeng Zhang,
Guopeng Zhang, Sashiko, cgroups, linux-kernel, linux-kselftest
Applied 1-2 to cgroup/for-7.4-fixes with the following tags added to
patch 1:
Reviewed-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Tested-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-09 14:51 ` Waiman Long
@ 2026-11-10 3:14 ` Ridong Chen
2026-11-10 3:32 ` Waiman Long
2026-11-10 8:18 ` Guopeng Zhang
0 siblings, 2 replies; 16+ messages in thread
From: Ridong Chen @ 2026-11-10 3:14 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
On 11/9/2026 10:51 PM, Waiman Long wrote:
> On 11/7/26 7:00 AM, Ridong Chen wrote:
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> partition_xcpus_del() only updates isolated_cpus when the partition that
>> gives back the CPUs has a different state than its parent. CPUs that were
>> further distributed to descendant isolated partitions are not taken into
>> account, so isolated_cpus_release() has to be called afterwards in
>> several places to walk up the ancestors and fix isolated_cpus.
>>
>> Make partition_xcpus_del() always set the isolation state of the given
>> back CPUs to the state of the parent that receives them. As the
>> effective_xcpus of a partition covers the CPUs distributed to its
>> descendants, the isolation state of those CPUs is settled at the same
>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>
>> compute_partition_effective_cpumask() invalidates a child partition
>> without going through partition_xcpus_del(), so do the same there: the
>> CPUs of the invalidated child take the isolation state of its parent.
>>
>> As the parent may be shrunk and its effective_xcpus not updated yet,
>> only do so for the CPUs the parent still owns. This is to keep the
>> isolation state correct for now, the in-place invalidation here will be
>> removed later in the partition rework.
>>
>> isolated_cpus_update() assumes the old and new states are different,
>> which no longer holds now that partition_xcpus_del() proceeds regardless
>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>> it simply sets the isolation state of the given CPUs, and use it
>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>> as setting the state is a no-op when the states are the same.
>>
>> isolated_cpus_release() is no longer needed and is removed.
>>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>> kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
>> 1 file changed, 19 insertions(+), 79 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 85e5b529d144..fc9de6e16ff5 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
>> }
>> /*
>> - * isolated_cpus_update - Update the isolated_cpus mask
>> - * @old_prs: old partition_root_state
>> - * @new_prs: new partition_root_state
>> - * @xcpus: exclusive CPUs with state change
>> + * isolated_cpus_update - Set the isolation state of @xcpus
>> + * @new_prs: partition root state that @xcpus will belong to
>> + * @xcpus: exclusive CPUs whose isolation state may change
>> + *
>> + * CPUs isolated at boot time are kept isolated.
>> */
>> -static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask
>> *xcpus)
>> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>> {
>> bool updated = false;
>> int cpu;
>> - WARN_ON_ONCE(old_prs == new_prs);
>> lockdep_assert_held(&callback_lock);
>> lockdep_assert_held(&cpuset_mutex);
>> for_each_cpu(cpu, xcpus)
>> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int
>> new_prs, struct cpumask *xcpus
>> update_housekeeping = true;
>> }
>> -/*
>> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
>> - * @cs: the current cpuset
>> - * @xcpus: the set of effective xcpus to be considered for removal
>> - *
>> - * This function is called when an isolated partition is invalidated or when
>> - * some of its effective xcpus are to be removed from isolated_cpus due to
>> - * changes in parent cpuset. The given @xcpus must be modifiable as the function
>> - * will update its value to determine the right set of CPUs to be removed.
>> - */
>> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
>> -{
>> - struct cpuset *ancestor = parent_cs(cs);
>> - struct cpuset *iancestor = NULL;
>> -
>> - /*
>> - * Search the furthest isolated partition root ancestor available
>> - * until top_cpuset or a remote partition root is hit.
>> - */
>> - for (; ancestor; ancestor = parent_cs(ancestor)) {
>> - if (ancestor->partition_root_state == PRS_ISOLATED)
>> - iancestor = ancestor;
>> - if (is_remote_partition(ancestor))
>> - break;
>> - }
>> -
>> - /*
>> - * If a furthest isolated partition acnestor is found, use it to
>> - * determine which of the CPUs in @xcpus should be removed from
>> - * isolated_cpus. Otherwise, all of them will be removed.
>> - */
>> - if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
>> - return; /* No CPU should be removed from isolated_cpus */
>> -
>> - spin_lock_irq(&callback_lock);
>> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
>> - spin_unlock_irq(&callback_lock);
>> -}
>> -
>> /*
>> * partition_xcpus_add - Add new exclusive CPUs to partition
>> * @new_prs: new partition_root_state
>> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct
>> cpuset *parent,
>> if (parent == &top_cpuset)
>> cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>> - if (new_prs != parent->partition_root_state)
>> - isolated_cpus_update(parent->partition_root_state, new_prs,
>> - xcpus);
>> + isolated_cpus_update(new_prs, xcpus);
>> cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
>> }
>> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct
>> cpuset *parent,
>> if (parent == &top_cpuset)
>> cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>> - if (old_prs != parent->partition_root_state)
>> - isolated_cpus_update(old_prs, parent->partition_root_state,
>> - xcpus);
>> + /* CPUs given back, including those of descendants, take parent's state */
>> + isolated_cpus_update(parent->partition_root_state, xcpus);
>> cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
>> cpumask_and(parent->effective_cpus, parent->effective_cpus,
>> cpu_active_mask);
>> @@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct
>> cpuset *cs,
>> struct cgroup_subsys_state *css;
>> struct cpuset *child;
>> bool populated = partition_is_populated(cs, NULL);
>> + static cpumask_t xcpus; /* Protected by cpuset_mutex */
>> /*
>> * Check child partition roots to see if they should be invalidated
>> @@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct
>> cpuset *cs,
>> int parent_prs = cs->partition_root_state;
>> /*
>> - * Invalidate child partition and update isolated_cpus
>> - * if necessary.
>> + * Invalidate child partition. Its CPUs still owned by
>> + * cs take the isolation state of cs. effective_xcpus
>> + * of cs may not be updated yet, so recompute it.
>> */
>> spin_lock_irq(&callback_lock);
>> WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>> - if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
>> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
>> - child->effective_xcpus);
>> + compute_excpus(cs, &xcpus);
>> + cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
>> + isolated_cpus_update(parent_prs, &xcpus);
>> child->partition_root_state = -old_prs;
>> spin_unlock_irq(&callback_lock);
>> notify_partition_change(child, old_prs);
>> @@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct
>> cpuset *cs, int cmd,
>> *
>> * A partition error happens when parent has tasks and all
>> * its effective CPUs will have to be distributed out.
>> - *
>> - * For an isolated partition with a shrunk root partition parent
>> - * we may need to return some or all of its isolated CPUs.
>> */
>> nocpu |= cpumask_empty(tmp->delmask);
>> - if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
>> - cpumask_clear(tmp->addmask);
>> - if (nocpu)
>> - cpumask_copy(tmp->addmask, cs->effective_xcpus);
>> - else if (!cpumask_subset(cs->effective_xcpus, parent-
>> >effective_xcpus))
>> - cpumask_andnot(tmp->addmask, cs->effective_xcpus,
>> - parent->effective_xcpus);
>> - if (!cpumask_empty(tmp->addmask))
>> - isolated_cpus_release(cs, tmp->addmask);
>> - /* Expansion of parent's effective_xcpus is handled below */
>> - }
>> if (nocpu) {
>> part_error = PERR_NOCPUS;
>> if (is_partition_valid(cs))
>> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs,
>> struct tmpmasks *tmp,
>> /*
>> * When parent is not a valid partition root,
>> * child partition roots become invalid too.
>> - *
>> - * For a to be invalidated isolated partition,
>> - * call return_isolated_cpus() to remove the
>> - * relevant CPUs from isolated_cpus.
>> + * The isolation state of their CPUs has been
>> + * settled when the parent gave them back.
>> */
>> if (is_partition_valid(cp))
>> new_prs = -old_prs;
>> - if (old_prs == PRS_ISOLATED) {
>> - cpumask_copy(tmp->delmask, cp->effective_xcpus);
>> - isolated_cpus_release(cp, tmp->delmask);
>> - }
>> WRITE_ONCE(cp->prs_err,
>> is_partition_invalid(parent)
>> ? PERR_INVPARENT : PERR_NOTPART);
>> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>> if (!is_partition_valid(cs))
>> reset_partition_data(cs);
>> else if (isolcpus_updated)
>> - isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>> + isolated_cpus_update(new_prs, cs->effective_xcpus);
>> spin_unlock_irq(&callback_lock);
>> /* Force update if partition is disabled & update effective_xcpus */
>
> The code change looks good to me. However this is not just a code restructuring,
> it also includes a bug fix. I wonder if we should clarify that and maybe add a
> Fixes tag.
>
Hi Longman,
Thanks for the review. You are right, it also fixes the isolated CPU
leak reported by Guopeng [1]. When a root partition with an isolated
child partition is invalidated, partition_xcpus_del() gives the CPUs
back to the parent without updating isolated_cpus as both are root
partitions, so the CPUs of the isolated child are left in
isolated_cpus.
I'd like to describe it in the commit message and add the following
tags in the next version:
Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
Reported-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
Closes: https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
Does that look OK to you?
[1] https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
--
Best regards
Ridong
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-10 3:14 ` Ridong Chen
@ 2026-11-10 3:32 ` Waiman Long
2026-11-10 8:18 ` Guopeng Zhang
1 sibling, 0 replies; 16+ messages in thread
From: Waiman Long @ 2026-11-10 3:32 UTC (permalink / raw)
To: Ridong Chen, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
On 11/9/26 10:14 PM, Ridong Chen wrote:
>
>
> On 11/9/2026 10:51 PM, Waiman Long wrote:
>> On 11/7/26 7:00 AM, Ridong Chen wrote:
>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>
>>> partition_xcpus_del() only updates isolated_cpus when the partition
>>> that
>>> gives back the CPUs has a different state than its parent. CPUs that
>>> were
>>> further distributed to descendant isolated partitions are not taken
>>> into
>>> account, so isolated_cpus_release() has to be called afterwards in
>>> several places to walk up the ancestors and fix isolated_cpus.
>>>
>>> Make partition_xcpus_del() always set the isolation state of the given
>>> back CPUs to the state of the parent that receives them. As the
>>> effective_xcpus of a partition covers the CPUs distributed to its
>>> descendants, the isolation state of those CPUs is settled at the same
>>> time. Boot-time isolated CPUs are still preserved by
>>> isolated_cpu_update().
>>>
>>> compute_partition_effective_cpumask() invalidates a child partition
>>> without going through partition_xcpus_del(), so do the same there: the
>>> CPUs of the invalidated child take the isolation state of its parent.
>>>
>>> As the parent may be shrunk and its effective_xcpus not updated yet,
>>> only do so for the CPUs the parent still owns. This is to keep the
>>> isolation state correct for now, the in-place invalidation here will be
>>> removed later in the partition rework.
>>>
>>> isolated_cpus_update() assumes the old and new states are different,
>>> which no longer holds now that partition_xcpus_del() proceeds
>>> regardless
>>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so
>>> that
>>> it simply sets the isolation state of the given CPUs, and use it
>>> everywhere. The state check in partition_xcpus_add() is dropped as
>>> well,
>>> as setting the state is a no-op when the states are the same.
>>>
>>> isolated_cpus_release() is no longer needed and is removed.
>>>
>>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>>> ---
>>> kernel/cgroup/cpuset.c | 98
>>> ++++++++----------------------------------
>>> 1 file changed, 19 insertions(+), 79 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 85e5b529d144..fc9de6e16ff5 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs,
>>> int cpu)
>>> }
>>> /*
>>> - * isolated_cpus_update - Update the isolated_cpus mask
>>> - * @old_prs: old partition_root_state
>>> - * @new_prs: new partition_root_state
>>> - * @xcpus: exclusive CPUs with state change
>>> + * isolated_cpus_update - Set the isolation state of @xcpus
>>> + * @new_prs: partition root state that @xcpus will belong to
>>> + * @xcpus: exclusive CPUs whose isolation state may change
>>> + *
>>> + * CPUs isolated at boot time are kept isolated.
>>> */
>>> -static void isolated_cpus_update(int old_prs, int new_prs, struct
>>> cpumask *xcpus)
>>> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>>> {
>>> bool updated = false;
>>> int cpu;
>>> - WARN_ON_ONCE(old_prs == new_prs);
>>> lockdep_assert_held(&callback_lock);
>>> lockdep_assert_held(&cpuset_mutex);
>>> for_each_cpu(cpu, xcpus)
>>> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs,
>>> int new_prs, struct cpumask *xcpus
>>> update_housekeeping = true;
>>> }
>>> -/*
>>> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from
>>> isolated_cpus
>>> - * @cs: the current cpuset
>>> - * @xcpus: the set of effective xcpus to be considered for removal
>>> - *
>>> - * This function is called when an isolated partition is
>>> invalidated or when
>>> - * some of its effective xcpus are to be removed from isolated_cpus
>>> due to
>>> - * changes in parent cpuset. The given @xcpus must be modifiable as
>>> the function
>>> - * will update its value to determine the right set of CPUs to be
>>> removed.
>>> - */
>>> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask
>>> *xcpus)
>>> -{
>>> - struct cpuset *ancestor = parent_cs(cs);
>>> - struct cpuset *iancestor = NULL;
>>> -
>>> - /*
>>> - * Search the furthest isolated partition root ancestor available
>>> - * until top_cpuset or a remote partition root is hit.
>>> - */
>>> - for (; ancestor; ancestor = parent_cs(ancestor)) {
>>> - if (ancestor->partition_root_state == PRS_ISOLATED)
>>> - iancestor = ancestor;
>>> - if (is_remote_partition(ancestor))
>>> - break;
>>> - }
>>> -
>>> - /*
>>> - * If a furthest isolated partition acnestor is found, use it to
>>> - * determine which of the CPUs in @xcpus should be removed from
>>> - * isolated_cpus. Otherwise, all of them will be removed.
>>> - */
>>> - if (iancestor && !cpumask_andnot(xcpus, xcpus,
>>> iancestor->effective_xcpus))
>>> - return; /* No CPU should be removed from isolated_cpus */
>>> -
>>> - spin_lock_irq(&callback_lock);
>>> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
>>> - spin_unlock_irq(&callback_lock);
>>> -}
>>> -
>>> /*
>>> * partition_xcpus_add - Add new exclusive CPUs to partition
>>> * @new_prs: new partition_root_state
>>> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs,
>>> struct cpuset *parent,
>>> if (parent == &top_cpuset)
>>> cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>>> - if (new_prs != parent->partition_root_state)
>>> - isolated_cpus_update(parent->partition_root_state, new_prs,
>>> - xcpus);
>>> + isolated_cpus_update(new_prs, xcpus);
>>> cpumask_andnot(parent->effective_cpus, parent->effective_cpus,
>>> xcpus);
>>> }
>>> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs,
>>> struct cpuset *parent,
>>> if (parent == &top_cpuset)
>>> cpumask_andnot(subpartitions_cpus, subpartitions_cpus,
>>> xcpus);
>>> - if (old_prs != parent->partition_root_state)
>>> - isolated_cpus_update(old_prs, parent->partition_root_state,
>>> - xcpus);
>>> + /* CPUs given back, including those of descendants, take
>>> parent's state */
>>> + isolated_cpus_update(parent->partition_root_state, xcpus);
>>> cpumask_or(parent->effective_cpus, parent->effective_cpus,
>>> xcpus);
>>> cpumask_and(parent->effective_cpus, parent->effective_cpus,
>>> cpu_active_mask);
>>> @@ -1612,6 +1570,7 @@ static void
>>> compute_partition_effective_cpumask(struct cpuset *cs,
>>> struct cgroup_subsys_state *css;
>>> struct cpuset *child;
>>> bool populated = partition_is_populated(cs, NULL);
>>> + static cpumask_t xcpus; /* Protected by cpuset_mutex */
>>> /*
>>> * Check child partition roots to see if they should be
>>> invalidated
>>> @@ -1640,14 +1599,15 @@ static void
>>> compute_partition_effective_cpumask(struct cpuset *cs,
>>> int parent_prs = cs->partition_root_state;
>>> /*
>>> - * Invalidate child partition and update isolated_cpus
>>> - * if necessary.
>>> + * Invalidate child partition. Its CPUs still owned by
>>> + * cs take the isolation state of cs. effective_xcpus
>>> + * of cs may not be updated yet, so recompute it.
>>> */
>>> spin_lock_irq(&callback_lock);
>>> WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>> - if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
>>> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
>>> - child->effective_xcpus);
>>> + compute_excpus(cs, &xcpus);
>>> + cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
>>> + isolated_cpus_update(parent_prs, &xcpus);
>>> child->partition_root_state = -old_prs;
>>> spin_unlock_irq(&callback_lock);
>>> notify_partition_change(child, old_prs);
>>> @@ -2080,22 +2040,8 @@ static int
>>> update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>> *
>>> * A partition error happens when parent has tasks and all
>>> * its effective CPUs will have to be distributed out.
>>> - *
>>> - * For an isolated partition with a shrunk root partition
>>> parent
>>> - * we may need to return some or all of its isolated CPUs.
>>> */
>>> nocpu |= cpumask_empty(tmp->delmask);
>>> - if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
>>> - cpumask_clear(tmp->addmask);
>>> - if (nocpu)
>>> - cpumask_copy(tmp->addmask, cs->effective_xcpus);
>>> - else if (!cpumask_subset(cs->effective_xcpus, parent-
>>> >effective_xcpus))
>>> - cpumask_andnot(tmp->addmask, cs->effective_xcpus,
>>> - parent->effective_xcpus);
>>> - if (!cpumask_empty(tmp->addmask))
>>> - isolated_cpus_release(cs, tmp->addmask);
>>> - /* Expansion of parent's effective_xcpus is handled
>>> below */
>>> - }
>>> if (nocpu) {
>>> part_error = PERR_NOCPUS;
>>> if (is_partition_valid(cs))
>>> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct
>>> cpuset *cs, struct tmpmasks *tmp,
>>> /*
>>> * When parent is not a valid partition root,
>>> * child partition roots become invalid too.
>>> - *
>>> - * For a to be invalidated isolated partition,
>>> - * call return_isolated_cpus() to remove the
>>> - * relevant CPUs from isolated_cpus.
>>> + * The isolation state of their CPUs has been
>>> + * settled when the parent gave them back.
>>> */
>>> if (is_partition_valid(cp))
>>> new_prs = -old_prs;
>>> - if (old_prs == PRS_ISOLATED) {
>>> - cpumask_copy(tmp->delmask, cp->effective_xcpus);
>>> - isolated_cpus_release(cp, tmp->delmask);
>>> - }
>>> WRITE_ONCE(cp->prs_err,
>>> is_partition_invalid(parent)
>>> ? PERR_INVPARENT : PERR_NOTPART);
>>> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs,
>>> int new_prs)
>>> if (!is_partition_valid(cs))
>>> reset_partition_data(cs);
>>> else if (isolcpus_updated)
>>> - isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>>> + isolated_cpus_update(new_prs, cs->effective_xcpus);
>>> spin_unlock_irq(&callback_lock);
>>> /* Force update if partition is disabled & update
>>> effective_xcpus */
>>
>> The code change looks good to me. However this is not just a code
>> restructuring, it also includes a bug fix. I wonder if we should
>> clarify that and maybe add a Fixes tag.
>>
> Hi Longman,
>
> Thanks for the review. You are right, it also fixes the isolated CPU
> leak reported by Guopeng [1]. When a root partition with an isolated
> child partition is invalidated, partition_xcpus_del() gives the CPUs
> back to the parent without updating isolated_cpus as both are root
> partitions, so the CPUs of the isolated child are left in
> isolated_cpus.
>
> I'd like to describe it in the commit message and add the following
> tags in the next version:
>
> Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated
> partitions")
> Reported-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> Closes:
> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>
> Does that look OK to you?
>
> [1]
> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>
Yes, that looks good to me.
Thanks,
Longman
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-09 15:20 ` Guopeng Zhang
@ 2026-11-10 6:24 ` Ridong Chen
2026-11-10 6:38 ` Guopeng Zhang
0 siblings, 1 reply; 16+ messages in thread
From: Ridong Chen @ 2026-11-10 6:24 UTC (permalink / raw)
To: Guopeng Zhang, Waiman Long, Tejun Heo, Johannes Weiner,
Michal Koutný,
Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
On 11/9/2026 11:20 PM, Guopeng Zhang wrote:
>
>
> 在 2026/11/7 20:00, Ridong Chen 写道:
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> partition_xcpus_del() only updates isolated_cpus when the partition that
>> gives back the CPUs has a different state than its parent. CPUs that were
>> further distributed to descendant isolated partitions are not taken into
>> account, so isolated_cpus_release() has to be called afterwards in
>> several places to walk up the ancestors and fix isolated_cpus.
>>
>> Make partition_xcpus_del() always set the isolation state of the given
>> back CPUs to the state of the parent that receives them. As the
>> effective_xcpus of a partition covers the CPUs distributed to its
>> descendants, the isolation state of those CPUs is settled at the same
>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>
>> compute_partition_effective_cpumask() invalidates a child partition
>> without going through partition_xcpus_del(), so do the same there: the
>> CPUs of the invalidated child take the isolation state of its parent.
>>
>> As the parent may be shrunk and its effective_xcpus not updated yet,
>> only do so for the CPUs the parent still owns. This is to keep the
>> isolation state correct for now, the in-place invalidation here will be
>> removed later in the partition rework.
>>
>> isolated_cpus_update() assumes the old and new states are different,
>> which no longer holds now that partition_xcpus_del() proceeds regardless
>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>> it simply sets the isolation state of the given CPUs, and use it
>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>> as setting the state is a no-op when the states are the same.
>>
>> isolated_cpus_release() is no longer needed and is removed.
>>
>
> Hi Ridong,
>
> Would it be worth briefly mentioning in the commit message that this patch also fixes the issue described here?
>
> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>
Thanks, Guopeng.
As I replied to Longman, I will add it.
--
Best regards
Ridong
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] cpuset: generalize the validate_partition() interface
2026-11-09 14:52 ` Waiman Long
@ 2026-11-10 6:36 ` Ridong Chen
0 siblings, 0 replies; 16+ messages in thread
From: Ridong Chen @ 2026-11-10 6:36 UTC (permalink / raw)
To: Waiman Long, Tejun Heo, Johannes Weiner, Michal Koutný, Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
On 11/9/2026 10:52 PM, Waiman Long wrote:
> On 11/7/26 7:01 AM, Ridong Chen wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Refactor validate_partition() to take the parent partition, the new
>> partition root state, the new effective exclusive CPUs and the CPUs to
>> be deleted, instead of a trial cpuset. A NULL parent stands for a
>> remote partition, whose CPUs come from top_cpuset. This makes the
>> interface flexible enough to handle both local and remote partition
>> validation.
>>
>> The no CPU left for tasks check now uses the given parent instead of
>> parent_cs(cs), and top_cpuset for a remote partition, as that is where
>> its CPUs are taken from. The cgroup parent of a remote partition does
>> not give it any CPU.
>>
>> The housekeeping conflict check is folded into validate_partition(). It
>> is skipped when the parent is not a valid partition root, as @cs can't
>> be a valid local partition in that case.
>>
>> Use the new interface for local partition enablement in
>> update_parent_effective_cpumask() and in partition_cpus_change(),
>> replacing the open-coded checks there. The order of the checks is now
>> the same for both paths: empty CPUs, housekeeping conflict, then no CPU
>> left for tasks.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>> ---
>> kernel/cgroup/cpuset.c | 119 ++++++++++++++++++++---------------------
>> 1 file changed, 59 insertions(+), 60 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index fc9de6e16ff5..0ec20412b3ce 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1414,6 +1414,48 @@ static bool prstate_housekeeping_conflict(int new_prs,
>> int parent_prs,
>> return res;
>> }
>> +/**
>> + * validate_partition - Validate a cpuset partition configuration
>> + * @cs: The cpuset to validate
>> + * @parent: Parent partition of @cs, NULL for remote partition
>> + * @new_prs: The proposed new partition root state
>> + * @new_excpus: The new effective exclusive CPUs mask to validate
>> + * @del: exclusive CPUs to be deleted from @cs, can be NULL
>> + *
>> + * A remote partition gets its CPUs from top_cpuset, so top_cpuset is used
>> + * as the parent when @parent is NULL.
>> + *
>> + * The housekeeping check is skipped if @parent is not a valid partition
>> + * root as @cs can't be a valid local partition in that case.
>> + *
>> + * Return: PRS error code (0 if valid, non-zero error code if invalid)
>> + */
>> +static enum prs_errcode validate_partition(struct cpuset *cs,
>> + struct cpuset *parent, int new_prs,
>> + struct cpumask *new_excpus,
>> + struct cpumask *del)
>> +{
>> + int parent_prs;
>> +
>> + if (new_prs == PRS_MEMBER)
>> + return PERR_NONE;
>> +
>> + if (cpumask_empty(new_excpus))
>> + return PERR_INVCPUS;
>> +
>> + if (!parent)
>> + parent = &top_cpuset;
>> + parent_prs = parent->partition_root_state;
>> + if (parent_prs > 0 &&
>> + prstate_housekeeping_conflict(new_prs, parent_prs, new_excpus, del))
>> + return PERR_HKEEPING;
>> +
>> + if (tasks_nocpu_error(parent, cs, new_excpus))
>> + return PERR_NOCPUS;
>> +
>> + return PERR_NONE;
>> +}
>> +
>> /*
>> * cpuset_update_sd_hk_unlock - Rebuild sched domains, update HK & unlock
>> *
>> @@ -1934,18 +1976,10 @@ static int update_parent_effective_cpumask(struct
>> cpuset *cs, int cmd,
>> if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
>> new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
>> - /*
>> - * Enabling partition root is not allowed if its
>> - * effective_xcpus is empty.
>> - */
>> - if (cpumask_empty(tmp->delmask))
>> - return PERR_INVCPUS;
>> -
>> - if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask,
>> NULL))
>> - return PERR_HKEEPING;
>> -
>> - if (nocpu)
>> - return PERR_NOCPUS;
>> + part_error = validate_partition(cs, parent, new_prs,
>> + tmp->delmask, NULL);
>> + if (part_error)
>> + return part_error;
>> /*
>> * This function will only be called when all the preliminary
>> @@ -2424,32 +2458,6 @@ static int parse_cpuset_cpulist(const char *buf, struct
>> cpumask *out_mask)
>> return 0;
>> }
>> -/**
>> - * validate_partition - Validate a cpuset partition configuration
>> - * @cs: The cpuset to validate
>> - * @trialcs: The trial cpuset containing proposed configuration changes
>> - *
>> - * If any validation check fails, the appropriate error code is set in the
>> - * cpuset's prs_err field.
>> - *
>> - * Return: PRS error code (0 if valid, non-zero error code if invalid)
>> - */
>> -static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset
>> *trialcs)
>> -{
>> - struct cpuset *parent = parent_cs(cs);
>> -
>> - if (cs_is_member(trialcs))
>> - return PERR_NONE;
>> -
>> - if (cpumask_empty(trialcs->effective_xcpus))
>> - return PERR_INVCPUS;
>> -
>> - if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
>> - return PERR_NOCPUS;
>> -
>> - return PERR_NONE;
>> -}
>> -
>> /**
>> * partition_cpus_change - Handle partition state changes due to CPU mask
>> updates
>> * @cs: The target cpuset being modified
>> @@ -2463,33 +2471,24 @@ static void partition_cpus_change(struct cpuset *cs,
>> struct cpuset *trialcs,
>> struct tmpmasks *tmp)
>> {
>> enum prs_errcode prs_err;
>> - int parent_prs = is_remote_partition(cs)
>> - ? PRS_ROOT : parent_cs(cs)->partition_root_state;
>> + struct cpuset *parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>> if (cs_is_member(cs))
>> return;
>> - prs_err = validate_partition(cs, trialcs);
>> - if (!prs_err && (parent_prs > 0) &&
>> - !cpumask_empty(trialcs->effective_xcpus)) {
>> - int prs = cs->partition_root_state;
>> -
>> - /*
>> - * Check for housekeeping conflict by passing the whole
>> - * effective_xcpus as well as those CPUs that are to be
>> - * removed from cs as delmask.
>> - */
>> - if (prs > 0)
>> - cpumask_andnot(tmp->delmask, cs->effective_xcpus,
>> - trialcs->effective_xcpus);
>> - else
>> - cpumask_clear(tmp->delmask);
>> -
>> - if (prstate_housekeeping_conflict(prs, parent_prs,
>> - trialcs->effective_xcpus, tmp->delmask))
>> - prs_err = PERR_HKEEPING;
>> - }
>> + /*
>> + * Check for housekeeping conflict by passing the whole
>> + * effective_xcpus as well as those CPUs that are to be
>> + * removed from cs as delmask.
>> + */
>> + if (is_partition_valid(cs))
>> + cpumask_andnot(tmp->delmask, cs->effective_xcpus,
>> + trialcs->effective_xcpus);
>> + else
>> + cpumask_clear(tmp->delmask);
>> + prs_err = validate_partition(cs, parent, trialcs->partition_root_state,
>> + trialcs->effective_xcpus, tmp->delmask);
>> if (prs_err) {
>> WRITE_ONCE(cs->prs_err, prs_err);
>> trialcs->prs_err = prs_err;
>
> Again, this code change looks good to me. Similar to previous patch, it includes
> both code restructuring and bug fixes.
>
Hi Longman,
Thanks for the review. Yes, besides the restructuring, it fixes a false
PERR_NOCPUS error for remote partitions. The no CPU left for tasks check
in partition_cpus_change() is done against the cgroup parent, but the
CPUs of a remote partition come from top_cpuset, not from its parent.
It can be reproduced on a 4-CPU system:
cd /sys/fs/cgroup
echo +cpuset > cgroup.subtree_control
mkdir P
echo 2-3 > P/cpuset.cpus
echo 2-3 > P/cpuset.cpus.exclusive
echo +cpuset > P/cgroup.subtree_control
mkdir P/C
echo 2-3 > P/C/cpuset.cpus
echo 2 > P/C/cpuset.cpus.exclusive
echo root > P/C/cpuset.cpus.partition
sleep 1000 &
echo $! > P/cgroup.procs
echo 2-3 > P/C/cpuset.cpus.exclusive
cat P/C/cpuset.cpus.partition
root invalid (Parent is not a partition root)
The effective_cpus of P is 3, a subset of the new exclusive CPUs 2-3 of
C, and P has a task, so the expansion is rejected. However, P is not a
partition root and does not give any CPU to C, and top_cpuset still has
CPUs 0-1 for the task. Enabling C directly with exclusive CPUs 2-3
succeeds, as remote_partition_enable() checks top_cpuset instead. With
this patch, the expansion succeeds too:
cat P/C/cpuset.cpus.partition
root
The issue has been there since remote partitions were introduced. I
reproduced it on commit 181c8e091aae as well, so I'd like to describe
it in the commit message and add
Fixes: 181c8e091aae ("cgroup/cpuset: Introduce remote partition")
in the next version.
Does that look OK to you? Please let me know if you are referring to something else.
--
Best regards
Ridong
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-10 6:24 ` Ridong Chen
@ 2026-11-10 6:38 ` Guopeng Zhang
0 siblings, 0 replies; 16+ messages in thread
From: Guopeng Zhang @ 2026-11-10 6:38 UTC (permalink / raw)
To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
Michal Koutný,
Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
在 2026/11/10 14:24, Ridong Chen 写道:
>
>
> On 11/9/2026 11:20 PM, Guopeng Zhang wrote:
>>
>>
>> 在 2026/11/7 20:00, Ridong Chen 写道:
>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>
>>> partition_xcpus_del() only updates isolated_cpus when the partition that
>>> gives back the CPUs has a different state than its parent. CPUs that were
>>> further distributed to descendant isolated partitions are not taken into
>>> account, so isolated_cpus_release() has to be called afterwards in
>>> several places to walk up the ancestors and fix isolated_cpus.
>>>
>>> Make partition_xcpus_del() always set the isolation state of the given
>>> back CPUs to the state of the parent that receives them. As the
>>> effective_xcpus of a partition covers the CPUs distributed to its
>>> descendants, the isolation state of those CPUs is settled at the same
>>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>>
>>> compute_partition_effective_cpumask() invalidates a child partition
>>> without going through partition_xcpus_del(), so do the same there: the
>>> CPUs of the invalidated child take the isolation state of its parent.
>>>
>>> As the parent may be shrunk and its effective_xcpus not updated yet,
>>> only do so for the CPUs the parent still owns. This is to keep the
>>> isolation state correct for now, the in-place invalidation here will be
>>> removed later in the partition rework.
>>>
>>> isolated_cpus_update() assumes the old and new states are different,
>>> which no longer holds now that partition_xcpus_del() proceeds regardless
>>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>>> it simply sets the isolation state of the given CPUs, and use it
>>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>>> as setting the state is a no-op when the states are the same.
>>>
>>> isolated_cpus_release() is no longer needed and is removed.
>>>
>>
>> Hi Ridong,
>>
>> Would it be worth briefly mentioning in the commit message that this patch also fixes the issue described here?
>>
>> https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>>
> Thanks, Guopeng.
>
> As I replied to Longman, I will add it.
>
Sorry, I just realized that my comment duplicated Longman's. I had some email connectivity issues and only just received the review emails.
Thanks for the clarification!
Cheers,
Guopeng
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent
2026-11-10 3:14 ` Ridong Chen
2026-11-10 3:32 ` Waiman Long
@ 2026-11-10 8:18 ` Guopeng Zhang
1 sibling, 0 replies; 16+ messages in thread
From: Guopeng Zhang @ 2026-11-10 8:18 UTC (permalink / raw)
To: Ridong Chen, Waiman Long, Tejun Heo, Johannes Weiner,
Michal Koutný,
Shuah Khan
Cc: Chen Ridong, cgroups, linux-kernel, linux-kselftest, Sashiko,
Guopeng Zhang, Ridong Chen
在 2026/11/10 11:14, Ridong Chen 写道:
>
>
> On 11/9/2026 10:51 PM, Waiman Long wrote:
>> On 11/7/26 7:00 AM, Ridong Chen wrote:
>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>
>>> partition_xcpus_del() only updates isolated_cpus when the partition that
>>> gives back the CPUs has a different state than its parent. CPUs that were
>>> further distributed to descendant isolated partitions are not taken into
>>> account, so isolated_cpus_release() has to be called afterwards in
>>> several places to walk up the ancestors and fix isolated_cpus.
>>>
>>> Make partition_xcpus_del() always set the isolation state of the given
>>> back CPUs to the state of the parent that receives them. As the
>>> effective_xcpus of a partition covers the CPUs distributed to its
>>> descendants, the isolation state of those CPUs is settled at the same
>>> time. Boot-time isolated CPUs are still preserved by isolated_cpu_update().
>>>
>>> compute_partition_effective_cpumask() invalidates a child partition
>>> without going through partition_xcpus_del(), so do the same there: the
>>> CPUs of the invalidated child take the isolation state of its parent.
>>>
>>> As the parent may be shrunk and its effective_xcpus not updated yet,
>>> only do so for the CPUs the parent still owns. This is to keep the
>>> isolation state correct for now, the in-place invalidation here will be
>>> removed later in the partition rework.
>>>
>>> isolated_cpus_update() assumes the old and new states are different,
>>> which no longer holds now that partition_xcpus_del() proceeds regardless
>>> of the states. Drop its @old_prs argument and the WARN_ON_ONCE() so that
>>> it simply sets the isolation state of the given CPUs, and use it
>>> everywhere. The state check in partition_xcpus_add() is dropped as well,
>>> as setting the state is a no-op when the states are the same.
>>>
>>> isolated_cpus_release() is no longer needed and is removed.
>>>
>>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>>> ---
>>> kernel/cgroup/cpuset.c | 98 ++++++++----------------------------------
>>> 1 file changed, 19 insertions(+), 79 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 85e5b529d144..fc9de6e16ff5 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -1244,17 +1244,17 @@ static bool isolated_cpu_update(int new_prs, int cpu)
>>> }
>>> /*
>>> - * isolated_cpus_update - Update the isolated_cpus mask
>>> - * @old_prs: old partition_root_state
>>> - * @new_prs: new partition_root_state
>>> - * @xcpus: exclusive CPUs with state change
>>> + * isolated_cpus_update - Set the isolation state of @xcpus
>>> + * @new_prs: partition root state that @xcpus will belong to
>>> + * @xcpus: exclusive CPUs whose isolation state may change
>>> + *
>>> + * CPUs isolated at boot time are kept isolated.
>>> */
>>> -static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
>>> +static void isolated_cpus_update(int new_prs, struct cpumask *xcpus)
>>> {
>>> bool updated = false;
>>> int cpu;
>>> - WARN_ON_ONCE(old_prs == new_prs);
>>> lockdep_assert_held(&callback_lock);
>>> lockdep_assert_held(&cpuset_mutex);
>>> for_each_cpu(cpu, xcpus)
>>> @@ -1263,45 +1263,6 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>>> update_housekeeping = true;
>>> }
>>> -/*
>>> - * isolated_cpus_release - Remove the relevant CPUs in @xcpus from isolated_cpus
>>> - * @cs: the current cpuset
>>> - * @xcpus: the set of effective xcpus to be considered for removal
>>> - *
>>> - * This function is called when an isolated partition is invalidated or when
>>> - * some of its effective xcpus are to be removed from isolated_cpus due to
>>> - * changes in parent cpuset. The given @xcpus must be modifiable as the function
>>> - * will update its value to determine the right set of CPUs to be removed.
>>> - */
>>> -static void isolated_cpus_release(struct cpuset *cs, struct cpumask *xcpus)
>>> -{
>>> - struct cpuset *ancestor = parent_cs(cs);
>>> - struct cpuset *iancestor = NULL;
>>> -
>>> - /*
>>> - * Search the furthest isolated partition root ancestor available
>>> - * until top_cpuset or a remote partition root is hit.
>>> - */
>>> - for (; ancestor; ancestor = parent_cs(ancestor)) {
>>> - if (ancestor->partition_root_state == PRS_ISOLATED)
>>> - iancestor = ancestor;
>>> - if (is_remote_partition(ancestor))
>>> - break;
>>> - }
>>> -
>>> - /*
>>> - * If a furthest isolated partition acnestor is found, use it to
>>> - * determine which of the CPUs in @xcpus should be removed from
>>> - * isolated_cpus. Otherwise, all of them will be removed.
>>> - */
>>> - if (iancestor && !cpumask_andnot(xcpus, xcpus, iancestor->effective_xcpus))
>>> - return; /* No CPU should be removed from isolated_cpus */
>>> -
>>> - spin_lock_irq(&callback_lock);
>>> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT, xcpus);
>>> - spin_unlock_irq(&callback_lock);
>>> -}
>>> -
>>> /*
>>> * partition_xcpus_add - Add new exclusive CPUs to partition
>>> * @new_prs: new partition_root_state
>>> @@ -1325,9 +1286,7 @@ static void partition_xcpus_add(int new_prs, struct cpuset *parent,
>>> if (parent == &top_cpuset)
>>> cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>>> - if (new_prs != parent->partition_root_state)
>>> - isolated_cpus_update(parent->partition_root_state, new_prs,
>>> - xcpus);
>>> + isolated_cpus_update(new_prs, xcpus);
>>> cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
>>> }
>>> @@ -1355,9 +1314,8 @@ static void partition_xcpus_del(int old_prs, struct cpuset *parent,
>>> if (parent == &top_cpuset)
>>> cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>>> - if (old_prs != parent->partition_root_state)
>>> - isolated_cpus_update(old_prs, parent->partition_root_state,
>>> - xcpus);
>>> + /* CPUs given back, including those of descendants, take parent's state */
>>> + isolated_cpus_update(parent->partition_root_state, xcpus);
>>> cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
>>> cpumask_and(parent->effective_cpus, parent->effective_cpus, cpu_active_mask);
>>> @@ -1612,6 +1570,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>> struct cgroup_subsys_state *css;
>>> struct cpuset *child;
>>> bool populated = partition_is_populated(cs, NULL);
>>> + static cpumask_t xcpus; /* Protected by cpuset_mutex */
>>> /*
>>> * Check child partition roots to see if they should be invalidated
>>> @@ -1640,14 +1599,15 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>> int parent_prs = cs->partition_root_state;
>>> /*
>>> - * Invalidate child partition and update isolated_cpus
>>> - * if necessary.
>>> + * Invalidate child partition. Its CPUs still owned by
>>> + * cs take the isolation state of cs. effective_xcpus
>>> + * of cs may not be updated yet, so recompute it.
>>> */
>>> spin_lock_irq(&callback_lock);
>>> WRITE_ONCE(child->prs_err, PERR_NOCPUS);
>>> - if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
>>> - isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
>>> - child->effective_xcpus);
>>> + compute_excpus(cs, &xcpus);
>>> + cpumask_and(&xcpus, &xcpus, child->effective_xcpus);
>>> + isolated_cpus_update(parent_prs, &xcpus);
>>> child->partition_root_state = -old_prs;
>>> spin_unlock_irq(&callback_lock);
>>> notify_partition_change(child, old_prs);
>>> @@ -2080,22 +2040,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>> *
>>> * A partition error happens when parent has tasks and all
>>> * its effective CPUs will have to be distributed out.
>>> - *
>>> - * For an isolated partition with a shrunk root partition parent
>>> - * we may need to return some or all of its isolated CPUs.
>>> */
>>> nocpu |= cpumask_empty(tmp->delmask);
>>> - if ((parent_prs == PRS_ROOT) && (old_prs == PRS_ISOLATED)) {
>>> - cpumask_clear(tmp->addmask);
>>> - if (nocpu)
>>> - cpumask_copy(tmp->addmask, cs->effective_xcpus);
>>> - else if (!cpumask_subset(cs->effective_xcpus, parent- >effective_xcpus))
>>> - cpumask_andnot(tmp->addmask, cs->effective_xcpus,
>>> - parent->effective_xcpus);
>>> - if (!cpumask_empty(tmp->addmask))
>>> - isolated_cpus_release(cs, tmp->addmask);
>>> - /* Expansion of parent's effective_xcpus is handled below */
>>> - }
>>> if (nocpu) {
>>> part_error = PERR_NOCPUS;
>>> if (is_partition_valid(cs))
>>> @@ -2341,18 +2287,12 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
>>> /*
>>> * When parent is not a valid partition root,
>>> * child partition roots become invalid too.
>>> - *
>>> - * For a to be invalidated isolated partition,
>>> - * call return_isolated_cpus() to remove the
>>> - * relevant CPUs from isolated_cpus.
>>> + * The isolation state of their CPUs has been
>>> + * settled when the parent gave them back.
>>> */
>>> if (is_partition_valid(cp))
>>> new_prs = -old_prs;
>>> - if (old_prs == PRS_ISOLATED) {
>>> - cpumask_copy(tmp->delmask, cp->effective_xcpus);
>>> - isolated_cpus_release(cp, tmp->delmask);
>>> - }
>>> WRITE_ONCE(cp->prs_err,
>>> is_partition_invalid(parent)
>>> ? PERR_INVPARENT : PERR_NOTPART);
>>> @@ -3108,7 +3048,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>> if (!is_partition_valid(cs))
>>> reset_partition_data(cs);
>>> else if (isolcpus_updated)
>>> - isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>>> + isolated_cpus_update(new_prs, cs->effective_xcpus);
>>> spin_unlock_irq(&callback_lock);
>>> /* Force update if partition is disabled & update effective_xcpus */
>>
>> The code change looks good to me. However this is not just a code restructuring, it also includes a bug fix. I wonder if we should clarify that and maybe add a Fixes tag.
>>
> Hi Longman,
>
> Thanks for the review. You are right, it also fixes the isolated CPU
> leak reported by Guopeng [1]. When a root partition with an isolated
> child partition is invalidated, partition_xcpus_del() gives the CPUs
> back to the parent without updating isolated_cpus as both are root
> partitions, so the CPUs of the isolated child are left in
> isolated_cpus.
>
> I'd like to describe it in the commit message and add the following
> tags in the next version:
>
> Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
> Reported-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> Closes: https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>
> Does that look OK to you?
>
> [1] https://lore.kernel.org/all/20261105130344.664336-4-guopeng.zhang@linux.dev/
>
Hi Ridong,
Since I've dropped my patch, I haven't added the reproducer to the selftests separately. If it's convenient, could you also include it in test_cpuset_prs.sh in v4?
Thanks,
Guopeng
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-11-10 8:18 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-07 12:00 [PATCH v3 0/4] cpuset: preparatory fixes and cleanups for partition rework Ridong Chen
2026-11-07 12:00 ` [PATCH v3 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid Ridong Chen
2026-11-09 14:59 ` Guopeng Zhang
2026-11-07 12:00 ` [PATCH v3 2/4] cpuset: add early empty cpumask check in partition_xcpus_add/del Ridong Chen
2026-11-09 15:48 ` Tejun Heo
2026-11-07 12:00 ` [PATCH v3 3/4] cpuset: settle isolation state when CPUs are given back to parent Ridong Chen
2026-11-09 14:51 ` Waiman Long
2026-11-10 3:14 ` Ridong Chen
2026-11-10 3:32 ` Waiman Long
2026-11-10 8:18 ` Guopeng Zhang
2026-11-09 15:20 ` Guopeng Zhang
2026-11-10 6:24 ` Ridong Chen
2026-11-10 6:38 ` Guopeng Zhang
2026-11-07 12:01 ` [PATCH v3 4/4] cpuset: generalize the validate_partition() interface Ridong Chen
2026-11-09 14:52 ` Waiman Long
2026-11-10 6:36 ` Ridong Chen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®