Message-ID: <7a42945a-9d82-47ad-9175-cfc3c0c311c3@redhat.com>
Date: Mon, 15 Sep 2025 15:05:21 -0400
From: Waiman Long <llong@...hat.com>
To: Chen Ridong <chenridong@...weicloud.com>, tj@...nel.org,
 hannes@...xchg.org, mkoutny@...e.com
Cc: cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
 lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH -next RFC -v2 08/11] cpuset: refactor
 cpus_allowed_validate_change

On 9/8/25 11:32 PM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@...wei.com>
>
> Refactor cpus_allowed_validate_change to handle the special case where
> cpuset.cpus can be set even when violating partition sibling CPU
> exclusivity rules. This differs from the general validation logic in
> validate_change. Add a wrapper function to properly handle this
> exceptional case.

That special rule is needed as setting v2 cpuset.cpus will never fail. 
For v1, it may only fails if the exclusivity rule set by the exclusive 
flag is violated. For v2, write failure may only happen on 
cpuset.cpus.exclusive. As a result, existing partitions may have to be 
invalidated if it will break exclusivity rule.

>
> The trialcs->prs_err field is cleared before performing validation checks
> for both CPU changes and partition errors. If cpus_allowed_validate_change
> fails its validation, trialcs->prs_err is set to PERR_NOTEXCL. If partition
> validation fails, the specific error code returned by validate_partition
> is assigned to trialcs->prs_err.
>
> With the partition validation status now directly available through
> trialcs->prs_err, the local boolean variable 'invalidate' becomes
> redundant and can be safely removed.
>
> Signed-off-by: Chen Ridong <chenridong@...wei.com>
> ---
>   kernel/cgroup/cpuset.c | 84 ++++++++++++++++++++++--------------------
>   1 file changed, 45 insertions(+), 39 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 770b33e30576..6aa93bd9d5dd 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2416,6 +2416,42 @@ static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *tri
>   	return PERR_NONE;
>   }
>   
> +static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset *trialcs,
> +					struct tmpmasks *tmp)
> +{
> +	int retval;
> +	struct cpuset *parent = parent_cs(cs);
> +
> +	retval = validate_change(cs, trialcs);
> +
> +	if ((retval == -EINVAL) && cpuset_v2()) {
> +		struct cgroup_subsys_state *css;
> +		struct cpuset *cp;
> +
> +		/*
> +		 * The -EINVAL error code indicates that partition sibling
> +		 * CPU exclusivity rule has been violated. We still allow
> +		 * the cpumask change to proceed while invalidating the
> +		 * partition. However, any conflicting sibling partitions
> +		 * have to be marked as invalid too.
> +		 */
> +		trialcs->prs_err = PERR_NOTEXCL;
> +		rcu_read_lock();
> +		cpuset_for_each_child(cp, css, parent) {
> +			struct cpumask *xcpus = user_xcpus(trialcs);
> +
> +			if (is_partition_valid(cp) &&
> +			    cpumask_intersects(xcpus, cp->effective_xcpus)) {
> +				rcu_read_unlock();
> +				update_parent_effective_cpumask(cp, partcmd_invalidate, NULL, tmp);
> +				rcu_read_lock();
> +			}
> +		}
> +		rcu_read_unlock();
> +		retval = 0;
> +	}
> +	return retval;
> +}
>   
>   /**
>    * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it
> @@ -2428,8 +2464,6 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>   {
>   	int retval;
>   	struct tmpmasks tmp;
> -	struct cpuset *parent = parent_cs(cs);
> -	bool invalidate = false;
>   	bool force = false;
>   	int old_prs = cs->partition_root_state;
>   	enum prs_errcode prs_err;
> @@ -2446,12 +2480,10 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>   		return -ENOMEM;
>   
>   	compute_trialcs_excpus(trialcs, cs);
> +	trialcs->prs_err = PERR_NONE;
>   
> -	prs_err = validate_partition(cs, trialcs);
> -	if (prs_err) {
> -		invalidate = true;
> -		cs->prs_err = prs_err;
> -	}
> +	if (cpus_allowed_validate_change(cs, trialcs, &tmp) < 0)
> +		goto out_free;
>   
>   	/*
>   	 * Check all the descendants in update_cpumasks_hier() if
> @@ -2459,40 +2491,14 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>   	 */
>   	force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus);
>   
> -	retval = validate_change(cs, trialcs);
> -
> -	if ((retval == -EINVAL) && cpuset_v2()) {
> -		struct cgroup_subsys_state *css;
> -		struct cpuset *cp;
> -
> -		/*
> -		 * The -EINVAL error code indicates that partition sibling
> -		 * CPU exclusivity rule has been violated. We still allow
> -		 * the cpumask change to proceed while invalidating the
> -		 * partition. However, any conflicting sibling partitions
> -		 * have to be marked as invalid too.
> -		 */
> -		invalidate = true;
> -		rcu_read_lock();
> -		cpuset_for_each_child(cp, css, parent) {
> -			struct cpumask *xcpus = user_xcpus(trialcs);
> -
> -			if (is_partition_valid(cp) &&
> -			    cpumask_intersects(xcpus, cp->effective_xcpus)) {
> -				rcu_read_unlock();
> -				update_parent_effective_cpumask(cp, partcmd_invalidate, NULL, &tmp);
> -				rcu_read_lock();
> -			}
> -		}
> -		rcu_read_unlock();
> -		retval = 0;
> +	prs_err = validate_partition(cs, trialcs);
> +	if (prs_err) {
> +		trialcs->prs_err = prs_err;
> +		cs->prs_err = prs_err;
>   	}
>   
> -	if (retval < 0)
> -		goto out_free;
> -
>   	if (is_partition_valid(cs) ||
> -	   (is_partition_invalid(cs) && !invalidate)) {
> +	   (is_partition_invalid(cs) && !trialcs->prs_err)) {
>   		struct cpumask *xcpus = trialcs->effective_xcpus;
>   
>   		if (cpumask_empty(xcpus) && is_partition_invalid(cs))
> @@ -2503,7 +2509,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>   		 */
>   		if (is_remote_partition(cs))
>   			remote_cpus_update(cs, NULL, xcpus, &tmp);
> -		else if (invalidate)
> +		else if (trialcs->prs_err)
>   			update_parent_effective_cpumask(cs, partcmd_invalidate,
>   							NULL, &tmp);
>   		else
Reviewed-by: Waiman Long <longman@...hat.com>

Powered by blists - more mailing lists