On 8/28/26 5:56 AM, Guopeng Zhang wrote:
From: Guopeng Zhang <[email protected]>
effective_xcpus includes CPUs granted to valid child partitions. Changing
a parent between root and isolated must not apply its new isolation state
or housekeeping constraints to those CPUs.
For example, on a cgroup v2 system with CPUs 0-3 online:
cd /sys/fs/cgroup
echo +cpuset > cgroup.subtree_control
mkdir type-repro
echo 1-3 > type-repro/cpuset.cpus
echo isolated > type-repro/cpuset.cpus.partition
echo +cpuset > type-repro/cgroup.subtree_control
mkdir type-repro/child
echo 2-3 > type-repro/child/cpuset.cpus
echo isolated > type-repro/child/cpuset.cpus.partition
echo root > type-repro/cpuset.cpus.partition
cat cpuset.cpus.isolated
The isolated mask should still contain CPUs 2-3 after the parent becomes a
root partition. Without this change, those CPUs are removed even though
the child remains isolated.
Compute the CPUs owned directly by a partition by subtracting the
effective_xcpus of valid children. Use this mask for type-change isolation
accounting and housekeeping checks. Apply the same ownership rule when
validating a trial CPU mask; otherwise a later CPU-mask update can mark a
parent invalid because of a boot-isolated CPU owned by a valid child.
Limiting the parent check to directly owned CPUs also allows a root child
to hold the last housekeeping CPU while its parent becomes isolated. If
the child then becomes a member, returning that CPU to the isolated parent
would violate the housekeeping constraint. The transition back to member
must remain allowed, so invalidate the outermost isolated ancestor and
return its CPUs to a root partition.
Sashiko pointed out the trial-validation and CPU-return gaps while
reviewing the original series.
Link:
https://sashiko.dev/#/patchset/20260820124202.517160-1-guopeng.zhang%40linux.dev?part=6
Fixes: 4a74e418881f ("cgroup/cpuset: Check partition conflict with housekeeping
setup")
Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
Fixes: 103b08709e8a ("cgroup/cpuset: Fail if isolated and nohz_full don't leave any
housekeeping")
Fixes: b1034a690129 ("cgroup/cpuset: Ensure domain isolated CPUs stay in root or
isolated partition")
Signed-off-by: Guopeng Zhang <[email protected]>
---
kernel/cgroup/cpuset.c | 92 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 84 insertions(+), 8 deletions(-)
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 8f24171b6055..32a37d624c6b 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2155,6 +2155,31 @@ static void compute_partition_effective_cpumask(struct
cpuset *cs,
rcu_read_unlock();
}
+/*
+ * Compute CPUs owned directly by a partition.
+ *
+ * effective_xcpus includes CPUs granted to valid child partitions. Exclude
+ * those CPUs when checking or changing this partition's type.
+ */
+static void compute_partition_owned_cpumask(struct cpuset *cs,
+ const struct cpumask
*partition_cpus,
+ struct cpumask *owned_cpus)
+{
+ struct cgroup_subsys_state *css;
+ struct cpuset *child;
+
+ lockdep_assert_held(&cpuset_mutex);
+ cpumask_copy(owned_cpus, partition_cpus);
+
+ rcu_read_lock();
+ cpuset_for_each_child(child, css, cs) {
+ if (is_partition_valid(child))
+ cpumask_andnot(owned_cpus, owned_cpus,
+ child->effective_xcpus);
+ }
+ rcu_read_unlock();
+}
+
/*
* update_cpumasks_hier - Update effective cpumasks and tasks in the subtree
* @cs: the cpuset to consider
@@ -2401,7 +2426,9 @@ static int parse_cpuset_cpulist(const char *buf, struct
cpumask *out_mask)
*
* Return: PRS error code (0 if valid, non-zero error code if invalid)
*/
-static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset
*trialcs)
+static enum prs_errcode validate_partition(struct cpuset *cs,
+ struct cpuset *trialcs,
+ struct cpumask *owned_cpus)
{
struct cpuset *parent = parent_cs(cs);
@@ -2411,8 +2438,10 @@ static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *tri
if (cpumask_empty(trialcs->effective_xcpus))
return PERR_INVCPUS;
+ compute_partition_owned_cpumask(cs, trialcs->effective_xcpus,
+ owned_cpus);
if (prstate_housekeeping_conflict(trialcs->partition_root_state,
- trialcs->effective_xcpus))
+ owned_cpus))
return PERR_HKEEPING;
if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
@@ -2438,7 +2467,7 @@ static void partition_cpus_change(struct cpuset *cs,
struct cpuset *trialcs,
if (cs_is_member(cs))
return;
- prs_err = validate_partition(cs, trialcs);
+ prs_err = validate_partition(cs, trialcs, tmp->new_cpus);
if (prs_err) {
WRITE_ONCE(cs->prs_err, prs_err);
trialcs->prs_err = prs_err;
@@ -2917,6 +2946,36 @@ int cpuset_update_flag(cpuset_flagbits_t bit, struct
cpuset *cs,
return err;
}
+/*
+ * Invalidate the highest isolated partition that contains @cs.
+ *
+ * A root partition returning CPUs to an isolated parent can consume the last
+ * housekeeping CPU. Invalidating the whole chain returns the CPUs to a root
+ * partition instead.
+ */
+static struct cpuset *invalidate_isolated_ancestor(struct cpuset *cs,
+ struct tmpmasks *tmp)
+{
+ struct cpuset *ancestor = parent_cs(cs);
+ int err;
+
+ lockdep_assert_held(&cpuset_mutex);
+ while (!is_remote_partition(ancestor) &&
+ (parent_cs(ancestor)->partition_root_state == PRS_ISOLATED))
+ ancestor = parent_cs(ancestor);
That can be dangerous & may cause NULL pointer dereference.
parent_cs(cs) returns NULL if cs is top_cpuset. So you should always
check if ancestor is NULL or parent_cs(ancestor) is NULL. Assuming that
the given cs is never the top_cpuset so the initial ancestor will not be
NULL. Each ancestor reassignment can become NULL. So don't mix ancestor
and parent_cs(ancestor) testing in the same compound if statement.
+
+ WRITE_ONCE(ancestor->prs_err, PERR_HKEEPING);
+ if (is_remote_partition(ancestor)) {
+ remote_partition_disable(ancestor, tmp);
+ } else {
+ err = update_parent_effective_cpumask(ancestor,
+ partcmd_invalidate, NULL,
tmp);
+ WARN_ON_ONCE(err);
+ }
+
+ return ancestor;
+}
+
/**
* update_prstate - update partition_root_state
* @cs: the cpuset to update
@@ -2929,6 +2988,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
{
int err = PERR_NONE, old_prs = cs->partition_root_state;
struct cpuset *parent = parent_cs(cs);
+ struct cpuset *invalidated = NULL;
struct tmpmasks tmpmask;
bool isolcpus_updated = false;
@@ -2985,11 +3045,14 @@ static int update_prstate(struct cpuset *cs, int new_prs)
} else if (old_prs && new_prs) {
/*
* A change in load balance state only, no change in cpumasks.
- * Need to update isolated_cpus.
+ * Need to update isolated_cpus for CPUs owned by this
partition,
+ * excluding CPUs distributed to valid child partitions.
*/
+ compute_partition_owned_cpumask(cs, cs->effective_xcpus,
+ tmpmask.new_cpus);
if (((new_prs == PRS_ISOLATED) &&
- !isolated_cpus_can_update(cs->effective_xcpus, NULL)) ||
- prstate_housekeeping_conflict(new_prs, cs->effective_xcpus))
+ !isolated_cpus_can_update(tmpmask.new_cpus, NULL)) ||
+ prstate_housekeeping_conflict(new_prs, tmpmask.new_cpus))
err = PERR_HKEEPING;
else
isolcpus_updated = true;
@@ -2998,6 +3061,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
* Switching back to member is always allowed even if it
* disables child partitions.
*/
+ if (old_prs == PRS_ROOT &&
+ parent->partition_root_state == PRS_ISOLATED &&
+ !isolated_cpus_can_update(cs->effective_xcpus, NULL))
+ invalidated = invalidate_isolated_ancestor(cs,
&tmpmask);
+ if (invalidated)
+ goto out;
+
You are adding a new exception here. You should update the comment above
to talk about the exception.
if (is_remote_partition(cs))
remote_partition_disable(cs, &tmpmask);
else
@@ -3025,11 +3095,17 @@ static int update_prstate(struct cpuset *cs, int
new_prs)
if (!is_partition_valid(cs))
reset_partition_data(cs);
else if (isolcpus_updated)
- isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
+ isolated_cpus_update(old_prs, new_prs, tmpmask.new_cpus);
As I have mentioned in my comment to your v1 series, this code can be
reached from multiple places beside switching from root to isolated and
vice versa. So tmpmask.new_cpus may not be the "owned xcpus". So
additional guard should be needed to decide if effective_xcpus or
tmpmask.new_cpus should be used.
spin_unlock_irq(&callback_lock);
/* Force update if switching back to member & update effective_xcpus */
- update_cpumasks_hier(cs, &tmpmask, !new_prs);
+ if (invalidated) {
+ update_cpumasks_hier(invalidated, &tmpmask, false);
+ update_partition_sd_lb(invalidated, PRS_ISOLATED);
+ notify_partition_change(invalidated, PRS_ISOLATED);
+ } else {
+ update_cpumasks_hier(cs, &tmpmask, !new_prs);
+ }
/* A newly created partition must have effective_xcpus set */
WARN_ON_ONCE(!old_prs && (new_prs > 0)
Cheers,
Longman
