Re: [PATCH RFC v2 11/22] cpuset: introduce local_partition_disable()

[Waiman Long <llong@redhat.com>]

On 10/25/25 2:48 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> The partition_disable() function introduced earlier can be extended to
> handle local partition disablement.
>
> The local_partition_disable() functions is introduced, which extracts the
> local partition disable logic from update_parent_effective_cpumask(). It
> calls partition_disable() to complete the disablement process.
>
> This refactoring establishes a clear separation between local and remote
> partition operations while promoting code reuse through the shared
> partition_disable() infrastructure.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 55 ++++++++++++++++++++++++++++++------------
>   1 file changed, 39 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index b308d9f80eef..f36d17a4d8cd 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1877,6 +1877,43 @@ static int local_partition_enable(struct cpuset *cs,
>   	return 0;
>   }
>   
> +/**
> + * local_partition_disable - Disable a local partition
> + * @cs: Target cpuset (local partition root) to disable
> + * @part_error: partition error when @cs is disabled
> + * @tmp: Temporary masks for CPU calculations
> + */
> +static void local_partition_disable(struct cpuset *cs, enum prs_errcode part_error,
> +				    struct tmpmasks *tmp)
> +{
> +	struct cpuset *parent = parent_cs(cs);
> +	bool cpumask_updated = false;
> +	int new_prs;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
> +
> +	if (!is_partition_valid(cs))
> +		return;
> +
> +	new_prs = part_error ? -cs->partition_root_state : 0;
> +	/*
> +	 * May need to add cpus back to parent's effective_cpus
> +	 * (and maybe removed from subpartitions_cpus/isolated_cpus)
> +	 * for valid partition root. xcpus may contain CPUs that
> +	 * shouldn't be removed from the two global cpumasks.
> +	 */
> +	if (is_partition_valid(parent))
> +		cpumask_updated = !cpumask_empty(cs->effective_xcpus);

If cs is a valid local partition, parent must be a valid partition. So 
the is_partition_valid(parent) check is meaningless. Also the 
effective_xcpus must not be empty. IOW, cpumask_updated must be true.

Cheers,
Longman

> +
> +	partition_disable(cs, parent, new_prs, part_error);
> +
> +	if (cpumask_updated) {
> +		cpuset_update_tasks_cpumask(parent, tmp->addmask);
> +		update_sibling_cpumasks(parent, cs, tmp);
> +	}
> +}
> +
>   /**
>    * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
>    * @cs:      The cpuset that requests change in partition root state
> @@ -1967,19 +2004,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   
>   	nocpu = tasks_nocpu_error(parent, cs, xcpus);
>   
> -	if (cmd == partcmd_disable) {
> -		/*
> -		 * May need to add cpus back to parent's effective_cpus
> -		 * (and maybe removed from subpartitions_cpus/isolated_cpus)
> -		 * for valid partition root. xcpus may contain CPUs that
> -		 * shouldn't be removed from the two global cpumasks.
> -		 */
> -		if (is_partition_valid(cs)) {
> -			cpumask_copy(tmp->addmask, cs->effective_xcpus);
> -			adding = true;
> -		}
> -		new_prs = PRS_MEMBER;
> -	} else if (newmask) {
> +	if (newmask) {
>   		/*
>   		 * Empty cpumask is not allowed
>   		 */
> @@ -3110,9 +3135,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   		if (is_remote_partition(cs))
>   			remote_partition_disable(cs, &tmpmask);
>   		else
> -			update_parent_effective_cpumask(cs, partcmd_disable,
> -							NULL, &tmpmask);
> -
> +			local_partition_disable(cs, PERR_NONE, &tmpmask);
>   		/*
>   		 * Invalidation of child partitions will be done in
>   		 * update_cpumasks_hier().