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

[Waiman Long <longman@redhat.com>]

On 5/27/26 2:43 AM, Sun Shaojie wrote:
> 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")
> Suggested-by: Zhang Guopeng <zhangguopeng@kylinos.cn>
> Signed-off-by: Sun Shaojie <sunshaojie@kylinos.cn>
> ---
>   kernel/cgroup/cpuset.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 1335e437098e..2395c5aec871 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1807,9 +1807,9 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   		 * Compute add/delete mask to/from effective_cpus
>   		 *
>   		 * For valid partition:
> -		 *   addmask = exclusive_cpus & ~newmask
> +		 *   addmask = effective_xcpus & ~newmask
>   		 *			      & parent->effective_xcpus
> -		 *   delmask = newmask & ~exclusive_cpus
> +		 *   delmask = newmask & ~effective_xcpus
>   		 *		       & parent->effective_xcpus
>   		 *
>   		 * For invalid partition:
> @@ -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;
Reviewed-by: Waiman Long <longman@redhat.com>