Re: [PATCH v3 2/3] cgroup/cpuset: Allow top-level partition CPU replacement

[Ridong Chen <ridong.chen@linux.dev>]

On 11/10/2026 1:03 PM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
> 
> When replacing a partition's CPUs, tasks_nocpu_error() checks whether
> the parent's current effective_cpus is a subset of the new exclusive
> CPU mask. The parent mask does not yet include CPUs being returned by
> the update, so this can invalidate an otherwise valid partition.
> 
> For example, on an 8-CPU system without boot isolation:
> 
>    echo 2-7 > A/cpuset.cpus
>    echo isolated > A/cpuset.cpus.partition
>    echo 0-4 > A/cpuset.cpus
>    cat A/cpuset.cpus.partition
>    isolated invalid (Parent unable to distribute cpu downstream)
> 
> The update should return CPUs 5-7 to the parent and leave A as a
> valid isolated partition.
> 
> Skip this subset check for top_cpuset. top_cpuset_sd_nocpu() already
> accounts for returned CPUs and ensures that an active boot domain
> housekeeping CPU remains.
> 
> Add a regression test for this case.
> 
> Fixes: 0c7f293efc87 ("cgroup/cpuset: Add cpuset.cpus.exclusive.effective for v2")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>

Hi Guopeng,

Patch 2 depends on patch 1. Skipping the cpumask_subset() check for
top_cpuset is only safe because top_cpuset_sd_nocpu() added by patch 1
now covers it. Without patch 1, patch 2 alone would let a partition
take away all the CPUs of top_cpuset. So patch 2 can't be backported
on its own, and the two have different Fixes tags. I think it would be
better to fold patch 2 into patch 1.

Also, the fix in patch 2 seems incomplete. The same false PERR_NOCPUS
happens when the parent is a local partition, as cpumask_subset()
there doesn't take the CPUs given back by the update into account
either. On an 8-CPU system:

	cd /sys/fs/cgroup
	echo +cpuset > cgroup.subtree_control
	mkdir A1
	echo 1-7 > A1/cpuset.cpus
	echo root > A1/cpuset.cpus.partition
	echo +cpuset > A1/cgroup.subtree_control
	mkdir A1/A2
	echo 3-7 > A1/A2/cpuset.cpus
	echo isolated > A1/A2/cpuset.cpus.partition
	sleep 1000 &
	echo $! > A1/cgroup.procs
	echo 1-4 > A1/A2/cpuset.cpus
	cat A1/A2/cpuset.cpus.partition
	isolated invalid (Parent unable to distribute cpu downstream)

A2 takes CPUs 1-2 from A1 and gives CPUs 5-7 back, so A1 still has
CPUs for its task. Instead of skipping the check for top_cpuset only,
how about making the check in tasks_nocpu_error() account for the CPUs
given back by cs for all parents? The hotplug caller would then need
to not count cs->effective_xcpus as given back, similar to what patch 3
does for top-level partitions.

> ---
>   kernel/cgroup/cpuset.c                            | 4 +++-
>   tools/testing/selftests/cgroup/test_cpuset_prs.sh | 3 +++
>   2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index c405719ccd12..8b6831983803 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1232,8 +1232,10 @@ static bool tasks_nocpu_error(struct cpuset *parent, struct cpuset *cs,
>   
>   	/*
>   	 * A populated partition (cs or parent) can't have empty effective_cpus
> +	 * The top_cpuset check above also accounts for CPUs being returned.
>   	 */
> -	return (cpumask_subset(parent->effective_cpus, xcpus) &&
> +	return (parent != &top_cpuset &&
> +		cpumask_subset(parent->effective_cpus, xcpus) &&
>   		partition_is_populated(parent, cs)) ||
>   	       (!cpumask_intersects(xcpus, cpu_active_mask) &&
>   		partition_is_populated(cs, NULL));
> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index 0288af1c2eb3..ee4d57b7e35b 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -554,6 +554,9 @@ TEST_MATRIX=(
>   	" C2-6:X2-5:P1 CX3-6:P2 \
>   	                    .      .   T:X4-6    .      .      .     0 A1:4-6|A2:4-6 A1:P1|A2:P-2 ."
>   
> +	# A CPU replacement must account for CPUs returned to the top cpuset.
> +	" C2-${CPULIST#0-}:P2 . . . C0-4 . . . 0 A1:0-4 A1:P2 0-4"
> +
>   	#
>   	#  old-A1 old-A2 old-A3 old-B1 new-A1 new-A2 new-A3 new-B1 fail ECPUs Pstate ISOLCPUS
>   	#  ------ ------ ------ ------ ------ ------ ------ ------ ---- ----- ------ --------

-- 
Best regards
Ridong