Re: [PATCH] cgroup/cpuset: Use effective_xcpus in partcmd_update add/del mask calculation

[Guopeng Zhang <zhangguopeng@kylinos.cn>]

在 2026/5/22 15:53, Sun Shaojie 写道:
> When sibling CPU exclusion occurs, a partition's user_xcpus may contain
> CPUs that were never actually granted to it. These CPUs are present in
> user_xcpus(cs) but not in cs->effective_xcpus.
> 
> The partcmd_update path in update_parent_effective_cpumask() uses
> user_xcpus(cs) (via the local variable xcpus) to compute the addmask
> (CPUs to return to parent) and delmask (CPUs to request from parent).
> This is incorrect:
> 
>  1) When newmask removes a CPU that was previously excluded by a
>     sibling, addmask incorrectly includes that CPU and tries to return
>     it to the parent even though the partition never actually owned it,
>     causing CPU overlap with sibling partitions and triggering warnings
>     in generate_sched_domains().
> 
>  2) When newmask adds a previously excluded CPU that is now available,
>     delmask fails to request it from the parent because user_xcpus(cs)
>     already includes it.
> 
> Fix this by using cs->effective_xcpus instead of user_xcpus(cs) in all
> partcmd_update paths that calculate addmask or delmask, including the
> PERR_NOCPUS error handling paths.
> 
> Reproducers:
> 
>   Example 1 - Removing a sibling-excluded CPU incorrectly returns it:
> 
>     # cd /sys/fs/cgroup
>     # echo "0-1" > a1/cpuset.cpus
>     # echo "root" > a1/cpuset.cpus.partition
>     # echo "0-2" > b1/cpuset.cpus
>     # echo "root" > b1/cpuset.cpus.partition
>     # echo "2" > b1/cpuset.cpus
>     # cat cpuset.cpus.effective
>     # Actual: 0-1,3    Expected: 3
> 
>   Example 2 - Expanding to a previously excluded CPU fails to request it:
> 
>     # cd /sys/fs/cgroup
>     # echo "0-1" > a1/cpuset.cpus
>     # echo "root" > a1/cpuset.cpus.partition
>     # echo "0-2" > b1/cpuset.cpus
>     # echo "root" > b1/cpuset.cpus.partition
>     # echo "member" > a1/cpuset.cpus.partition
>     # echo "1-2" > b1/cpuset.cpus
>     # cat cpuset.cpus.effective
>     # Actual: 0-1,3    Expected: 0,3
> 
> Fixes: 2a3602030d80 ("cgroup/cpuset: Don't invalidate sibling partitions on cpuset.cpus conflict")
> Signed-off-by: Sun Shaojie <sunshaojie@kylinos.cn>
> ---
>  kernel/cgroup/cpuset.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 1335e437098e..5a5fa2481467 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1821,11 +1821,11 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  			deleting = cpumask_and(tmp->delmask,
>  					newmask, parent->effective_xcpus);
>  		} else {
> -			cpumask_andnot(tmp->addmask, xcpus, newmask);
> +			cpumask_andnot(tmp->addmask, cs->effective_xcpus, newmask);
>  			adding = cpumask_and(tmp->addmask, tmp->addmask,
>  					     parent->effective_xcpus);
>  
> -			cpumask_andnot(tmp->delmask, newmask, xcpus);
> +			cpumask_andnot(tmp->delmask, newmask, cs->effective_xcpus);
>  			deleting = cpumask_and(tmp->delmask, tmp->delmask,
>  					       parent->effective_xcpus);
>  		}
> @@ -1864,7 +1864,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  			part_error = PERR_NOCPUS;
>  			deleting = false;
>  			adding = cpumask_and(tmp->addmask,
> -					     xcpus, parent->effective_xcpus);
> +					     cs->effective_xcpus, parent->effective_xcpus);
>  		}
>  	} else {
>  		/*
> @@ -1886,7 +1886,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  			part_error = PERR_NOCPUS;
>  			if (is_partition_valid(cs))
>  				adding = cpumask_and(tmp->addmask,
> -						xcpus, parent->effective_xcpus);
> +						     cs->effective_xcpus,
> +						     parent->effective_xcpus);
>  		} else if (is_partition_invalid(cs) && !cpumask_empty(xcpus) &&
>  			   cpumask_subset(xcpus, parent->effective_xcpus)) {
>  			struct cgroup_subsys_state *css;
Hi, Shaojie

The code change looks reasonable to me, but I think the comment above
the partcmd_update calculation should be updated as well.

Maybe it can be updated like this:

		 * Compute add/delete mask to/from effective_cpus
		 *
		 * For valid partition:
-		 *   addmask = exclusive_cpus & ~newmask
+		 *   addmask = cs->effective_xcpus & ~newmask
		 *			      & parent->effective_xcpus
-		 *   delmask = newmask & ~exclusive_cpus
+		 *   delmask = newmask & ~cs->effective_xcpus
		 *		       & parent->effective_xcpus
		 *
		 * For invalid partition:

Does this look reasonable to you?

Tested-by: Guopeng Zhang <zhangguopeng@kylinos.cn>

Thanks.