Re: [PATCH-next 2/6] cgroup/cpuset: Properly compute effective xcpus in update_parent_effective_cpumask()
From: Waiman Long
Date: Thu Oct 22 2026 - 10:53:30 EST
On 10/22/26 9:45 AM, Guopeng Zhang wrote:
Hi Longman,OK, I will look into that again.
在 2026/10/22 11:16, Waiman Long 写道:
Since the introduction of cpuset.cpus.exclusive in commitI re-tested the repro from the link below on top of this series
e2ffe502ba45 ("cgroup/cpuset: Add cpuset.cpus.exclusive for v2"), a
compute_effective_exclusive_cpumask() helper which is later renamed to
compute_excpus() is added to compute the effective xcpus of a partition
root. However update_parent_effective_cpumask() is not updated in all
proper places to use the new helper to check if a given cpuset should
remain a valid partition root. This can cause a cpuset to incorrectly
remain a valid partition root even if no effective xcpus is left.
Fix that problem by using compute_excpus() to compute the effective xcpus
into delmask in all cases except partcmd_disable and partcmd_invalidate
and use it for tasks_nocpu_error() check as well as checking its
emptiness. This enables us to catch invalid partition root more
accurately in update_parent_effective_cpumask().
(applied on cgroup/for-7.4-fixes), and it still fails:
https://lore.kernel.org/all/20261010075947.333549-3-guopeng.zhang@xxxxxxxxx/
cd /sys/fs/cgroup
echo +cpuset > cgroup.subtree_control
mkdir A
echo 0-7 > A/cpuset.cpus
echo 0-7 > A/cpuset.cpus.exclusive
echo root > A/cpuset.cpus.partition
cat A/cpuset.cpus.partition
echo 1 > A/cpuset.cpus.exclusive
cat A/cpuset.cpus.partition
The last command still shows "root invalid (Parent unable to distribute
cpu downstream)".
In this case compute_excpus() returns the stale user mask unchanged.
user_xcpus() is gone from update_parent_effective_cpumask(), but the
dependency just moved one level down: the new code calls
compute_excpus(cs, tmp->delmask), and its first line is
cpumask_and(excpus, user_xcpus(cs), parent->effective_xcpus)
Here user_xcpus(cs) is cs->exclusive_cpus, which still holds the old
0-7 when compute_excpus() runs: update_exclusive_cpumask() publishes
cs->exclusive_cpus only after partition_cpus_change(). Nothing corrects
it either - the sibling trim is skipped as exclusive_cpus is non-empty,
and for a direct child of the root cpuset the intersection with
parent->effective_xcpus is a no-op. So the stale 0-7 mask still reaches
tasks_nocpu_error() and PERR_NOCPUS is set again. Please point it out
if I got the analysis wrong.
I have rebased my fix on top of this series and the partition recovers
correctly with it applied. Should the fix be folded into this patch,
or is there a better way to fix it? Happy to hear your thoughts.
Also, is the test case below worth adding here as well? Note that it
fails until the fix is in:
# An invalid local partition that took all the CPUs from its
# populated parent should recover after its cpuset.cpus.exclusive
# list is changed to a valid one.
"C0-7:X0-7:P1 . . . X1 . . . 0 A1:1|XA1:1 A1:P1 ."
This test case assumes that there are only 8 CPUs in the test system which may not be true in many cases. We need test case that will work no matter the system it is running on.
Cheers,
Longman
Thanks,
Guopeng
Some test case are added to test_cpuset_prs.sh for invalidating a
partition root due to its effective xcpus becoming empty as well as
making sure that an invalid partition can only be revived when exclusive
CPUs are available agin.
Fixes: e2ffe502ba45 ("cgroup/cpuset: Add cpuset.cpus.exclusive for v2")
Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
---
kernel/cgroup/cpuset.c | 36 +++++++++----------
.../selftests/cgroup/test_cpuset_prs.sh | 6 ++++
2 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 7ead8c3b7974..db6e4663f4be 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1798,7 +1798,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
int deleting; /* Deleting cpus from parent's effective_cpus */
int old_prs, new_prs;
int part_error = PERR_NONE; /* Partition error? */
- struct cpumask *xcpus = user_xcpus(cs);
int parent_prs = parent->partition_root_state;
bool nocpu;
@@ -1841,30 +1840,29 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
if (!newmask && xcpus_empty(cs))
return PERR_CPUSEMPTY;
- nocpu = tasks_nocpu_error(parent, cs, xcpus);
+ /*
+ * Except for partcmd_disable, use delmask to compute the current
+ * effective xcpus value and use it for tasks_nocpu_error check.
+ */
+ if (cmd != partcmd_disable) {
+ compute_excpus(cs, tmp->delmask);
+ nocpu = tasks_nocpu_error(parent, cs, tmp->delmask);
+ }
if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
- /*
- * Need to call compute_excpus() in case
- * exclusive_cpus not set. Sibling conflict should only happen
- * if exclusive_cpus isn't set.
- */
- xcpus = tmp->delmask;
- if (compute_excpus(cs, xcpus))
- WARN_ON_ONCE(!cpumask_empty(cs->exclusive_cpus));
new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
/*
* Enabling partition root is not allowed if its
* effective_xcpus is empty.
*/
- if (cpumask_empty(xcpus))
+ if (cpumask_empty(tmp->delmask))
return PERR_INVCPUS;
- if (prstate_housekeeping_conflict(new_prs, parent_prs, xcpus, NULL))
+ if (prstate_housekeeping_conflict(new_prs, parent_prs, tmp->delmask, NULL))
return PERR_HKEEPING;
- if (tasks_nocpu_error(parent, cs, xcpus))
+ if (nocpu)
return PERR_NOCPUS;
/*
@@ -1876,9 +1874,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
*
* Warn if it is not the case.
*/
- cpumask_and(tmp->new_cpus, xcpus, cpu_active_mask);
+ cpumask_and(tmp->new_cpus, tmp->delmask, cpu_active_mask);
WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
-
deleting = true;
} else if (cmd == partcmd_disable) {
/*
@@ -1973,7 +1970,7 @@ 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.
*/
- if (nocpu) {
+ if (nocpu || cpumask_empty(tmp->delmask)) {
part_error = PERR_NOCPUS;
if (is_partition_valid(cs))
adding = cpumask_and(tmp->addmask,
@@ -1986,8 +1983,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
cpumask_copy(tmp->addmask, cs->effective_xcpus);
adding = true;
}
- } else if (is_partition_invalid(cs) && !cpumask_empty(xcpus) &&
- cpumask_subset(xcpus, parent->effective_xcpus)) {
+ } else if (is_partition_invalid(cs)) {
struct cgroup_subsys_state *css;
struct cpuset *child;
bool exclusive = true;
@@ -1998,7 +1994,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
* check.
*/
if (prstate_housekeeping_conflict(cs->partition_root_state,
- parent_prs, xcpus, NULL)) {
+ parent_prs, tmp->delmask, NULL)) {
part_error = PERR_HKEEPING;
goto write_error;
}
@@ -2014,7 +2010,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
rcu_read_unlock();
if (exclusive)
deleting = cpumask_and(tmp->delmask,
- xcpus, parent->effective_cpus);
+ tmp->delmask, parent->effective_cpus);
else
part_error = PERR_NOTEXCL;
}
diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index d04aad697b96..47791591002f 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
@@ -359,6 +359,8 @@ TEST_MATRIX=(
A1:P0|A2:P1|A3:P2|B1:P1 2-4|3"
" C1-6:P1 C3-6:P1 C5-6:P1 . . P2 . . 0 A1:1-2|A2:3-6|A3:5-6 \
A1:P1|A2:P2|A3:P-1 3-6"
+ " C1-6:P2 C3-6:P2 C3-4:P2 . . C5-6 . . 0 A1:1-4|A2:5-6|A3:5-6 \
+ A1:P2|A2:P2|A3:P-2 1-6"
" C1-4:X2-4 C2-4:X3-4:P1 C3-4:X4:P2 \
. . X5 . . 0 A1:1-4|A2:2-4|A3:3-4 \
A1:P0|A2:P-1|A3:P-2 ."
@@ -427,6 +429,10 @@ 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"
+ # 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"
+
# Taking away all CPUs from parent or itself if there are tasks
# will make the partition invalid.
" C2-3:P1 C3:P1 . . T C2-3 . . 0 A1:2-3|A2:2-3 A1:P1|A2:P-1"