From: Ridong Chen <ridong.chen@linux.dev>
To: Guopeng Zhang <guopeng.zhang@linux.dev>,
Waiman Long <longman@redhat.com>
Cc: "Tejun Heo" <tj@kernel.org>,
"Johannes Weiner" <hannes@cmpxchg.org>,
"Michal Koutný" <mkoutny@suse.com>,
cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
"Guopeng Zhang" <zhangguopeng@kylinos.cn>
Subject: Re: [PATCH v2] cgroup/cpuset: Invalidate remote partition on housekeeping conflict
Date: Mon, 28 Sep 2026 09:29:15 +0800 [thread overview]
Message-ID: <4e63ea4d-8cb9-4a36-87d8-43796b42d98f@linux.dev> (raw)
In-Reply-To: <20260927095722.70660-1-guopeng.zhang@linux.dev>
On 9/27/2026 5:57 PM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> Widening an ancestor's exclusive CPU mask can add a boot-isolated CPU
> to a valid remote partition root without touching the partition's own
> control files. The partition then load balances that CPU, silently
> defeating isolcpus=domain for it.
>
> This can be reproduced on a 32-CPU system booted with
> isolcpus=domain,4:
>
> cd /sys/fs/cgroup
> echo +cpuset > cgroup.subtree_control
> mkdir -p A/B
> echo +cpuset > A/cgroup.subtree_control
> echo 2-4 > A/cpuset.cpus
> echo 2-3 > A/cpuset.cpus.exclusive
> echo 2-4 > A/B/cpuset.cpus
> echo 2-4 > A/B/cpuset.cpus.exclusive
> echo root > A/B/cpuset.cpus.partition
> cat A/B/cpuset.cpus.effective # 2-3
> echo 2-4 > A/cpuset.cpus.exclusive
> cat A/B/cpuset.cpus.partition # root
> cat A/B/cpuset.cpus.effective # 2-4
>
> The last write returns 0 and leaves the hierarchy in this state:
>
> root (cpuset.cpus.effective=0-1,5-31)
> |
> \-- A (member): cpuset.cpus=2-4
> | cpuset.cpus.exclusive=2-4
> \-- B (root, remote): cpuset.cpus=2-4
> cpuset.cpus.effective=2-4
>
> B is a remote partition: it takes its CPUs directly from the root
> cpuset, and A only passes its exclusive list down. Before the last
> write, that list is 2-3, so B holds 2-3 and CPU 4 stays in the
> root cpuset as a boot-isolated CPU. The write widens A's exclusive
> list to 2-4, which additionally grants CPU 4 to B. Nothing rejects
> the grant: B remains a valid root partition, and CPU 4 is still
> listed in cpuset.cpus.isolated while sitting in a load-balanced
> partition.
>
> remote_partition_enable() already rejects such grants through
> prstate_housekeeping_conflict(). remote_cpus_update(), which applies
> ancestor changes to a remote partition, does not.
>
> Both paths also open-code remote partition validation. Move those
> checks into validate_remote_partition(), and check the resulting
> effective exclusive CPU mask for a housekeeping conflict there. The
> existing prs_err path then invalidates the remote partition instead of
> assigning it a boot-isolated CPU.
>
> Fixes: f62a5d39368e ("cgroup/cpuset: Remove remote_partition_check() & make update_cpumasks_hier() handle remote partition")
> Suggested-by: Ridong Chen <ridong.chen@linux.dev>
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
> Changes since v1:
> - Consolidate remote partition validation in a common helper, as
> suggested by Ridong.
> - Check housekeeping conflicts against the resulting effective exclusive
> CPU mask.
> - Rebase onto cgroup/for-7.3-fixes.
>
> kernel/cgroup/cpuset.c | 71 +++++++++++++++++++++++++++++-------------
> 1 file changed, 49 insertions(+), 22 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 1fcec89a28b9..5adf47217e59 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1561,6 +1561,45 @@ static inline bool is_local_partition(struct cpuset *cs)
> return is_partition_valid(cs) && !is_remote_partition(cs);
> }
>
> +/**
> + * validate_remote_partition - Validate a remote partition CPU change
> + * @cs: cpuset being enabled or updated
> + * @prs: partition root state to validate
> + * @excpus: resulting effective exclusive CPU mask
> + * @addcpus: exclusive CPUs to be added
> + * @delcpus: exclusive CPUs to be deleted, can be NULL
> + *
> + * Return: PERR_NONE if valid, otherwise an appropriate error code
> + */
> +static enum prs_errcode
> +validate_remote_partition(struct cpuset *cs, int prs,
> + struct cpumask *excpus,
> + struct cpumask *addcpus,
> + struct cpumask *delcpus)
> +{
> + bool updating = is_remote_partition(cs);
> +
> + if (!capable(CAP_SYS_ADMIN))
> + return PERR_ACCESS;
> +
> + if (!updating &&
> + (!cpumask_intersects(excpus, cpu_active_mask) ||
> + cpumask_subset(top_cpuset.effective_cpus, addcpus)))
> + return PERR_INVCPUS;
> +
> + if (cpumask_intersects(addcpus, subpartitions_cpus) ||
> + (updating &&
> + cpumask_subset(top_cpuset.effective_cpus, addcpus)))
> + return PERR_NOCPUS;
> +
> + if ((prs == PRS_ISOLATED &&
> + !isolated_cpus_can_update(addcpus, delcpus)) ||
> + prstate_housekeeping_conflict(prs, excpus))
> + return PERR_HKEEPING;
> +
> + return PERR_NONE;
> +}
> +
validate_partition is used for both local and remote partitions, so I think this
check may be added here in the future. For now, it looks good to me.
> /*
> * remote_partition_enable - Enable current cpuset as a remote partition root
> * @cs: the cpuset to update
> @@ -1574,11 +1613,7 @@ static inline bool is_local_partition(struct cpuset *cs)
> static int remote_partition_enable(struct cpuset *cs, int new_prs,
> struct tmpmasks *tmp)
> {
> - /*
> - * The user must have sysadmin privilege.
> - */
> - if (!capable(CAP_SYS_ADMIN))
> - return PERR_ACCESS;
> + enum prs_errcode err;
>
> /*
> * The requested exclusive_cpus must not be allocated to other
> @@ -1591,15 +1626,10 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
> * above it or remote partition root underneath it is not allowed.
> */
> compute_excpus(cs, tmp->new_cpus);
> - if (!cpumask_intersects(tmp->new_cpus, cpu_active_mask) ||
> - cpumask_subset(top_cpuset.effective_cpus, tmp->new_cpus))
> - return PERR_INVCPUS;
> - if (cpumask_intersects(tmp->new_cpus, subpartitions_cpus))
> - return PERR_NOCPUS;
> - if (((new_prs == PRS_ISOLATED) &&
> - !isolated_cpus_can_update(tmp->new_cpus, NULL)) ||
> - prstate_housekeeping_conflict(new_prs, tmp->new_cpus))
> - return PERR_HKEEPING;
> + err = validate_remote_partition(cs, new_prs,
> + tmp->new_cpus, tmp->new_cpus, NULL);
> + if (err)
> + return err;
>
> spin_lock_irq(&callback_lock);
> partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
> @@ -1672,6 +1702,7 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
> static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
> struct cpumask *excpus, struct tmpmasks *tmp)
> {
> + enum prs_errcode err;
> bool adding, deleting;
> int prs = cs->partition_root_state;
>
> @@ -1695,14 +1726,10 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
> */
> if (adding) {
> WARN_ON_ONCE(cpumask_intersects(tmp->addmask, subpartitions_cpus));
> - if (!capable(CAP_SYS_ADMIN))
> - WRITE_ONCE(cs->prs_err, PERR_ACCESS);
> - else if (cpumask_intersects(tmp->addmask, subpartitions_cpus) ||
> - cpumask_subset(top_cpuset.effective_cpus, tmp->addmask))
> - WRITE_ONCE(cs->prs_err, PERR_NOCPUS);
> - else if ((prs == PRS_ISOLATED) &&
> - !isolated_cpus_can_update(tmp->addmask, tmp->delmask))
> - WRITE_ONCE(cs->prs_err, PERR_HKEEPING);
> + err = validate_remote_partition(cs, prs, excpus,
> + tmp->addmask, tmp->delmask);
> + if (err)
> + WRITE_ONCE(cs->prs_err, err);
> if (cs->prs_err)
> goto invalidate;
> }
>
> base-commit: 31c88350b7dd1522792f726f79607f31bb55c50f
Reviewed-by: Ridong Chen <ridong.chen@linux.dev>
Thanks.
--
Best regards
Ridong
next prev parent reply other threads:[~2026-09-28 1:29 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-27 9:57 [PATCH v2] cgroup/cpuset: Invalidate remote partition on housekeeping conflict Guopeng Zhang
2026-09-28 0:00 ` Waiman Long
2026-09-28 1:29 ` Ridong Chen [this message]
2026-09-28 18:44 ` Tejun Heo
2026-09-28 23:56 ` Waiman Long
2026-09-29 2:33 ` Guopeng Zhang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4e63ea4d-8cb9-4a36-87d8-43796b42d98f@linux.dev \
--to=ridong.chen@linux.dev \
--cc=cgroups@vger.kernel.org \
--cc=guopeng.zhang@linux.dev \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=mkoutny@suse.com \
--cc=tj@kernel.org \
--cc=zhangguopeng@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.