Re: [PATCH-next v3 04/11] cgroup/cpuset: Minor fixes to prstate_housekeeping_conflict()

[Guopeng Zhang <guopeng.zhang@linux.dev>]

在 2026/10/26 05:44, Waiman Long 写道:
> Commit 9527ff78a413 ("cgroup/cpuset: Consider all the exclusive CPUs when
> doing housekeeping check") changes the way prstate_housekeeping_conflict()
> is being used by considering all the exclusive xcpus to be owned by the
> partition when doing housekeeping check. However there are two minor
> issues reported by sashiko with that patch:
> 
>  1) In partition_cpus_change(), prstate_housekeeping_conflict() can
>  be called with an invalid partition root. In that case, there are
>  no isolated CPUs that are going to be returned back to the system to
>  become housekeeping CPUs, so the calculation of the del_cpus cpumask
>  is meaningless.
> 
>  2) The checking of del_cpus in prstate_housekeeping_conflict() to see
>  if it can contain an active housekeeping CPU that can be returned to the
>  system ignores the fact that an active CPU not in HK_TYPE_DOMAIN_BOOT HK
>  cpumask will not become an active housekeeping CPU when it is released
>  by an isolated partition.
> 
> Update the existing code to handle these two issues properly to make
> prstate_housekeeping_conflict() produce more accurate result.
> 
> Fixes: 9527ff78a413 ("cgroup/cpuset: Consider all the exclusive CPUs when doing housekeeping check")
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  kernel/cgroup/cpuset.c | 45 +++++++++++++++++++++++++++++-------------
>  1 file changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 37bc79d0c647..57e5334eb919 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1446,6 +1446,9 @@ static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
>  	    !cpumask_subset(new_cpus, housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT)))
>  		return true;
>  
> +	if (del_cpus && cpumask_empty(del_cpus))
> +		del_cpus = NULL;
> +
>  	/*
>  	 * Exit 2nd check early and return false if
>  	 * 1) there is no boot time isolated nohz_full CPU
> @@ -1454,8 +1457,7 @@ static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
>  	 *    be returned back to the parent.
>  	 */
>  	if (!housekeeping_enabled(HK_TYPE_KERNEL_NOISE) ||
> -	    (new_prs == parent_prs) ||
> -	    ((new_prs == PRS_ROOT) && (!del_cpus || cpumask_empty(del_cpus))))
> +	    (new_prs == parent_prs) || (!del_cpus && (new_prs == PRS_ROOT)))
>  		return false;
>  
>  	/*
> @@ -1466,20 +1468,29 @@ static bool prstate_housekeeping_conflict(int new_prs, int parent_prs,
>  	if (parent_prs == PRS_ISOLATED)
>  		swap(new_cpus, del_cpus);
>  
> -	if (del_cpus &&
> -	   (cpumask_first_and_and(del_cpus, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
> -				   cpu_active_mask) < nr_cpu_ids))
> -		return false;
> -
>  	if (!alloc_cpumask_var(&full_hk_cpus, GFP_KERNEL))
>  		return true;
>  
> +	/*
> +	 * Check if del_cpus may contain a CPU that can become an active
> +	 * housekeeping CPU again.
> +	 */
> +	if (del_cpus) {
> +		cpumask_and(full_hk_cpus, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
> +			    housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT));
> +		if (cpumask_first_and_and(del_cpus, full_hk_cpus,
> +					  cpu_active_mask) < nr_cpu_ids) {
> +			res = false;
> +			goto out;
> +		}
> +	}
> +
>  	cpumask_and(full_hk_cpus, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
>  		    housekeeping_cpumask(HK_TYPE_DOMAIN));
>  	cpumask_andnot(full_hk_cpus, full_hk_cpus, isolated_cpus);
>  	cpumask_and(full_hk_cpus, full_hk_cpus, cpu_active_mask);
>  	res = !cpumask_weight_andnot(full_hk_cpus, new_cpus);
> -
> +out:
>  	free_cpumask_var(full_hk_cpus);
>  	return res;
>  }
> @@ -2486,23 +2497,29 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>  				  struct tmpmasks *tmp)
>  {
>  	enum prs_errcode prs_err;
> +	int parent_prs = is_remote_partition(cs)
> +		       ? PRS_ROOT : parent_cs(cs)->partition_root_state;
>  
>  	if (cs_is_member(cs))
>  		return;
>  
>  	prs_err = validate_partition(cs, trialcs);
> -	if (!prs_err && !cpumask_empty(trialcs->effective_xcpus)) {
> +	if (!prs_err && (parent_prs > 0) &&
> +	    !cpumask_empty(trialcs->effective_xcpus)) {
>  		int prs = cs->partition_root_state;
> -		int parent_prs = is_remote_partition(cs)
> -			       ? PRS_ROOT : parent_cs(cs)->partition_root_state;
> +
>  		/*
>  		 * Check for housekeeping conflict by passing the whole
>  		 * effective_xcpus as well as those CPUs that are to be
>  		 * removed from cs as delmask.
>  		 */
> -		cpumask_andnot(tmp->delmask, cs->effective_xcpus, trialcs->effective_xcpus);
> -		if (parent_prs > 0 &&
> -		    prstate_housekeeping_conflict(prs, parent_prs,
> +		if (prs > 0)
> +			cpumask_andnot(tmp->delmask, cs->effective_xcpus,
> +				       trialcs->effective_xcpus);
> +		else
> +			cpumask_clear(tmp->delmask);
> +
> +		if (prstate_housekeeping_conflict(prs, parent_prs,
>  						  trialcs->effective_xcpus, tmp->delmask))
>  			prs_err = PERR_HKEEPING;
>  	}

Reviewed-by: Guopeng Zhang <zhangguopeng@kylinos.cn>

Thanks,
Guopeng