Re: [PATCH 08/17] cgroup/cpuset: Release CPUs when a type change is rejected

[Waiman Long <longman@redhat.com>]

On 8/20/26 8:41 AM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> When a housekeeping check rejects a root-to-isolated or isolated-to-root
> change, update_prstate() marks the partition invalid but leaves its
> effective_xcpus allocated. Those CPUs remain missing from the partition
> which owns the invalidated subtree.
>
> Disable a rejected remote partition with remote_partition_disable().
> For a local partition, return effective_xcpus to its parent before
> recording the invalid state. Use the complete mask because descendants
> are invalidated along with the parent and their CPUs return at the same
> time.
>
> Fixes: 103b08709e8a ("cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
>   kernel/cgroup/cpuset.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 853942c1afeb..a616012927be 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -3020,10 +3020,21 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   		compute_partition_owned_cpumask(cs, tmpmask.new_cpus);
>   		if (((new_prs == PRS_ISOLATED) &&
>   		     !isolated_cpus_can_update(tmpmask.new_cpus, NULL)) ||
> -		    prstate_housekeeping_conflict(new_prs, tmpmask.new_cpus))
> +		    prstate_housekeeping_conflict(new_prs, tmpmask.new_cpus)) {
>   			err = PERR_HKEEPING;
> -		else
> +			if (is_remote_partition(cs)) {
> +				WRITE_ONCE(cs->prs_err, err);
This WRITE_ONCE() is not necessary as it will be done later in the 
function.
> +				remote_partition_disable(cs, &tmpmask);
> +			} else {
> +				spin_lock_irq(&callback_lock);
> +				partition_xcpus_del(old_prs, parent, cs->effective_xcpus);
> +				spin_unlock_irq(&callback_lock);
> +				cpuset_update_tasks_cpumask(parent, tmpmask.new_cpus);
> +				update_sibling_cpumasks(parent, cs, &tmpmask);
> +			}

This code to disable the partition is duplicating what is being done in 
the last else clause of the outer if, but in a somewhat different way. 
That is not good programming practice. You can either jump to the 
partition disabling code below or separate out the partition disabling 
code controlled by a separate control flag and set the flag to do it.

Cheers,
Longman

> +		} else {
>   			isolcpus_updated = true;
> +		}
>   	} else {
>   		/*
>   		 * Switching back to member is always allowed even if it