Re: [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate()

[Waiman Long <llong@redhat.com>]

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Build on the partition_disable() infrastructure introduced in the previous
> patch to handle local partition invalidation.
>
> The local_partition_invalidate() function factors out the local partition
> invalidation logic from update_parent_effective_cpumask(), which delegates
> to partition_disable() to complete the invalidation process.
>
> Additionally, correct the transition logic in cpuset_hotplug_update_tasks()
> when determining whether to transition an invalid partition root, the check
> should be based on non-empty user_cpus rather than non-empty
> effective_xcpus. This correction addresses the scenario where
> exclusive_cpus is not set but cpus_allowed is configured - in this case,
> effective_xcpus may be empty even though the partition should be considered
> for re-enablement. The user_cpus-based check ensures proper partition state
> transitions under these conditions.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 66 +++++++++++++++++++++++++++---------------
>   1 file changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 87ba43e93540..e460d03286ba 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1911,6 +1911,39 @@ static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
>   	}
>   }
>   
> +/**
> + * local_partition_invalidate - Invalidate a local partition
> + * @cs: Target cpuset (local partition root) to invalidate
> + * @tmp: Temporary masks
> + */
> +static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
> +{
> +	struct cpumask *xcpus = user_xcpus(cs);
> +	struct cpuset *parent = parent_cs(cs);
> +	int new_prs = cs->partition_root_state;
> +	bool cpumask_updated = false;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
> +
> +	if (is_partition_invalid(cs))
> +		return;
You should change the check to if (!is_partition_valid(cs)). You can 
avoid the case that partition_disable() is called with a member.

> +	/*
> +	 * Make the current partition invalid.
> +	 */
> +	if (is_partition_valid(parent))
> +		cpumask_updated = cpumask_and(tmp->addmask,
> +					      xcpus, parent->effective_xcpus);
> +	if (cs->partition_root_state > 0)
> +		new_prs = -cs->partition_root_state;
> +
> +	partition_disable(cs, parent, new_prs, cs->prs_err);
> +	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
> @@ -1972,23 +2005,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   	adding = deleting = false;
>   	old_prs = new_prs = cs->partition_root_state;
>   
> -	if (cmd == partcmd_invalidate) {
> -		if (is_partition_invalid(cs))
> -			return 0;
> -
> -		/*
> -		 * Make the current partition invalid.
> -		 */
> -		if (is_partition_valid(parent))
> -			adding = cpumask_and(tmp->addmask,
> -					     xcpus, parent->effective_xcpus);
> -		if (old_prs > 0) {
> -			new_prs = -old_prs;
> -			subparts_delta--;
> -		}
> -		goto write_error;
> -	}
> -
>   	/*
>   	 * The parent must be a partition root.
>   	 * The new cpumask, if present, or the current cpus_allowed must
> @@ -2552,7 +2568,7 @@ static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset *trialc
>   			if (is_partition_valid(cp) &&
>   			    cpumask_intersects(xcpus, cp->effective_xcpus)) {
>   				rcu_read_unlock();
> -				update_parent_effective_cpumask(cp, partcmd_invalidate, NULL, tmp);
> +				local_partition_invalidate(cp, tmp);
>   				rcu_read_lock();
>   			}
>   		}
> @@ -2592,8 +2608,7 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>   					   trialcs->effective_xcpus, tmp);
>   	} else {
>   		if (trialcs->prs_err)
> -			update_parent_effective_cpumask(cs, partcmd_invalidate,
> -							NULL, tmp);
> +			local_partition_invalidate(cs, tmp);
>   		else
>   			update_parent_effective_cpumask(cs, partcmd_update,
>   							trialcs->effective_xcpus, tmp);
> @@ -4037,18 +4052,21 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
>   	 *    partitions.
>   	 */
>   	if (is_local_partition(cs) && (!is_partition_valid(parent) ||
> -				tasks_nocpu_error(parent, cs, &new_cpus)))
> +				tasks_nocpu_error(parent, cs, &new_cpus))) {
>   		partcmd = partcmd_invalidate;
> +		local_partition_invalidate(cs, tmp);
> +	}
>   	/*
>   	 * On the other hand, an invalid partition root may be transitioned
> -	 * back to a regular one with a non-empty effective xcpus.
> +	 * back to a regular one with a non-empty user xcpus.
>   	 */
>   	else if (is_partition_valid(parent) && is_partition_invalid(cs) &&
> -		 !cpumask_empty(cs->effective_xcpus))
> +		 !cpumask_empty(user_xcpus(cs))) {

I believe the effective_xcpus is not cleared currently when a partition 
is invalidated. Anyway, this change is also OK especially if 
effective_xcpus will be cleared in a later patch.

Cheers,
Longman

>   		partcmd = partcmd_update;
> +		update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
> +	}
>   
>   	if (partcmd >= 0) {
> -		update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
>   		if ((partcmd == partcmd_invalidate) || is_partition_valid(cs)) {
>   			compute_partition_effective_cpumask(cs, &new_cpus);
>   			cpuset_force_rebuild();