Re: [PATCH-next 2/6] cgroup/cpuset: Properly compute effective xcpus in update_parent_effective_cpumask()

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

On 10/22/2026 11:16 AM, Waiman Long wrote:
> Since the introduction of cpuset.cpus.exclusive in commit
> 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().
> 
> 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@redhat.com>
> ---
>   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;
>   

update_parent_effective_cpumask is only used for the local partition. So I'd 
suggest adding a local_partition_validate helper, which can be used for both 
partition enable and update. For now this logic is spread out, and many corner 
cases would easily be missed.

For the remote partition, I have also made suggestions when reviewing Guopeng's 
patches.

>   		/*
> @@ -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"

-- 
Best regards
Ridong