Re: [PATCH 1/2] cgroup/cpuset: Fix cpuset.cpus update in a remote partition

[Waiman Long <longman@redhat.com>]

On 10/10/26 3:59 AM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> When the CPU list of a remote partition is reduced, a removed CPU may
> remain unavailable to the top cpuset.
>
> This can be reproduced on a cgroup v2 system with CPUs 0-7 online:
>
>      cd /sys/fs/cgroup
>      echo +cpuset > cgroup.subtree_control
>      mkdir A
>      echo 1-7 > A/cpuset.cpus
>      echo 1-7 > A/cpuset.cpus.exclusive
>      echo +cpuset > A/cgroup.subtree_control
>      mkdir A/B
>      echo 1-4 > A/B/cpuset.cpus
>      echo root > A/B/cpuset.cpus.partition
>      echo 1-3 > A/B/cpuset.cpus
>      cat cpuset.cpus.effective
>
> The last command should show 0,4-7, but CPU 4 is missing.
>
> update_cpumask() calls partition_cpus_change() while cs->cpus_allowed
> still contains the old mask. remote_cpus_update() first releases CPU 4,
> but the hierarchy update revisits B and compute_excpus() still sees the
> old mask, so CPU 4 is removed from the top cpuset again.
>
> Move the cpus_allowed update before partition_cpus_change() so that
> the hierarchy update sees the new mask when it revisits the cpuset.
>
> Fixes: f62a5d39368e ("cgroup/cpuset: Remove remote_partition_check() & make update_cpumasks_hier() handle remote partition")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
>   kernel/cgroup/cpuset.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 0ebba2d646c3..de7553459adc 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2478,10 +2478,13 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>   	 */
>   	force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus);
>   
> +	spin_lock_irq(&callback_lock);
> +	cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
> +	spin_unlock_irq(&callback_lock);
> +
>   	partition_cpus_change(cs, trialcs, &tmp);
>   
>   	spin_lock_irq(&callback_lock);
> -	cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
>   	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
>   	if ((old_prs > 0) && !is_partition_valid(cs))
>   		reset_partition_data(cs);

Sorry for the late reply.

Thanks for reporting this problem. I suspect similar problem may exist 
with change to cpuset.cpus.exclusive as well. The root cause of this 
problem is the recursive call to remote_cpus_update() with the second 
call missing information leading to the revert of the first call. I have 
posted another patch to address this recursive call problem. Please take 
a look to see if you agree with that or not.

Thanks,
Longman