Re: [PATCH-next v3 08/11] cgroup/cpuset: Release invalid isolated CPUs in compute_partition_effective_cpumask()

[Guopeng Zhang]

在 2026/10/26 05:44, Waiman Long 写道:
> It is found by sashiko that when a child isolated partition with parent
> root partition is invalidated in compute_partition_effective_cpumask(),
> the switching of the isolated exclusive CPUs from child to parent will
> not be reflected in isolated_cpus leading to CPU leakage. Fix that by
> calling isolated_cpus_update() when that happens.
> 
> Also simplify compute_partition_effective_cpumask() by doing
> child partition invalidation only for the special case of no more
> effective_cpus left in a populated parent. The other case doesn't
> actually warrant invalidation at this stage and subsequent iteration
> in update_cpumasks_hier() and update_parent_effective_cpumask() for
> that child should handle that.
> 
> Also drop the make_partition_invalid() helper by inlining it into
> compute_partition_effective_cpumask() as it is the only caller.
> 
> A new test case is add to test_cpuset_prs.sh to verify this.
> 
> Fixes: 11e5f407b64a ("cgroup/cpuset: Keep track of CPUs in isolated partitions")
> Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
> ---
>  kernel/cgroup/cpuset.c                        | 46 +++++++------------
>  .../selftests/cgroup/test_cpuset_prs.sh       |  4 ++
>  2 files changed, 21 insertions(+), 29 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 53fa3970946e..6d224e614e74 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -254,15 +254,6 @@ static inline bool cs_is_member(const struct cpuset *cs)
>  	return cs->partition_root_state == PRS_MEMBER;
>  }
>  
> -/*
> - * Callers should hold callback_lock to modify partition_root_state.
> - */
> -static inline void make_partition_invalid(struct cpuset *cs)
> -{
> -	if (cs->partition_root_state > 0)
> -		cs->partition_root_state = -cs->partition_root_state;
> -}
> -
>  /*
>   * Send notification event of whenever partition_root_state changes.
>   */
> @@ -2153,8 +2144,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   * or update_cpumasks_hier() where parent and children are modified
>   * successively, we don't need to call update_parent_effective_cpumask()
>   * and the child's effective_cpus will be updated in later iterations.
> - *
> - * Note that rcu_read_lock() is assumed to be held.
>   */
>  static void compute_partition_effective_cpumask(struct cpuset *cs,
>  						struct cpumask *new_ecpus)
> @@ -2166,15 +2155,16 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>  	WARN_ON_ONCE(!is_partition_valid(cs));
>  
>  	/*
> -	 * Check child partition roots to see if they should be
> -	 * invalidated when
> -	 *  1) child effective_xcpus not a subset of new
> -	 *     excluisve_cpus
> -	 *  2) All the effective_cpus will be used up and cp
> -	 *     has tasks
> +	 * Check child partition roots to see if they should be invalidated
> +	 * when all the effective_cpus will be used up and cs is populated.
> +	 * It is better to sacrifice a child partition root than invalidating
> +	 * the whole subtree.
> +	 *
> +	 * Return early if the effective_cpus is already empty.
>  	 */
>  	compute_excpus(cs, new_ecpus);
> -	cpumask_and(new_ecpus, new_ecpus, cpu_active_mask);
> +	if (!cpumask_and(new_ecpus, new_ecpus, cpu_active_mask))
> +		return;
>  
>  	rcu_read_lock();
>  	cpuset_for_each_child(child, css, cs) {
> @@ -2186,22 +2176,20 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>  		 * partition root.
>  		 */
>  		WARN_ON_ONCE(is_remote_partition(child));
> -		WRITE_ONCE(child->prs_err, 0);
> -		if (!cpumask_subset(child->effective_xcpus,
> -				    cs->effective_xcpus))
> -			WRITE_ONCE(child->prs_err, PERR_INVCPUS);
> -		else if (populated &&
> -			 cpumask_subset(new_ecpus, child->effective_xcpus))
> -			WRITE_ONCE(child->prs_err, PERR_NOCPUS);
> -
> -		if (child->prs_err) {
> +		if (populated && cpumask_subset(new_ecpus, child->effective_xcpus)) {
>  			int old_prs = child->partition_root_state;
> +			int parent_prs = cs->partition_root_state;
>  
>  			/*
> -			 * Invalidate child partition
> +			 * Invalidate child partition and update isolated_cpus
> +			 * if necessary.
>  			 */
>  			spin_lock_irq(&callback_lock);
> -			make_partition_invalid(child);
> +			WRITE_ONCE(child->prs_err, PERR_NOCPUS);
> +			if ((old_prs == PRS_ISOLATED) && (parent_prs == PRS_ROOT))
> +				isolated_cpus_update(PRS_ISOLATED, PRS_ROOT,
> +						     child->effective_xcpus);
> +			child->partition_root_state = -old_prs;
>  			spin_unlock_irq(&callback_lock);
>  			notify_partition_change(child, old_prs);
>  			continue;
> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index f0d98da3cfd8..d7d0bef0a17a 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -518,6 +518,10 @@ TEST_MATRIX=(
>  	" CX0-4:P2 CX2-4:P2 CX3-4:P2 .     .     X2     .      .     0 A1:0-1,3-4|A2:2|A3:2 \
>  								       A1:P2|A2:P2|A3:P-2 0-4"
>  
> +	# When an isolated partition child with root partition parent is invalidated,
> +	# its effective xcpus will be removed from isolated_cpus.
> +	" C1-4:P1 C3-4:P2   .      .    T:C3-4   .      .      .     0 A1:3-4|A2:3-4 A1:P1|A2:P-2 ."
> +
>  	#
>  	#  old-A1 old-A2 old-A3 old-B1 new-A1 new-A2 new-A3 new-B1 fail ECPUs Pstate ISOLCPUS
>  	#  ------ ------ ------ ------ ------ ------ ------ ------ ---- ----- ------ --------

Reviewed-by: Guopeng Zhang <zhangguopeng@xxxxxxxxxx>
Tested-by: Guopeng Zhang <zhangguopeng@xxxxxxxxxx>

Thanks,
Guopeng