Re: [PATCH v2 2/3] cgroup/cpuset: Preserve boot-isolated CPUs on partition release

[Waiman Long <longman@redhat.com>]

On 8/23/26 10:01 PM, Guopeng Zhang wrote:
> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>
> isolated_cpus tracks CPUs isolated with isolcpus= as well as CPUs in
> isolated cpuset partitions. When an isolated partition is released,
> isolated_cpus_update() removes its whole CPU mask. This also clears CPUs
> which were already isolated at boot.
>
> This can be reproduced on a cgroup v2 system booted with
> isolcpus=domain,15:
>
>      cd /sys/fs/cgroup
>      echo +cpuset > cgroup.subtree_control
>      mkdir cpuset-repro
>      echo 15 > cpuset-repro/cpuset.cpus
>      echo isolated > cpuset-repro/cpuset.cpus.partition
>      echo member > cpuset-repro/cpuset.cpus.partition
>      cat cpuset.cpus.isolated
>
> CPU 15 is absent before the change. It must remain in
> cpuset.cpus.isolated after the partition is released.
>
> Update isolated_cpus one CPU at a time and keep CPUs outside the
> boot-time domain housekeeping mask isolated.
>
> Fixes: c188f33c864e ("cgroup/cpuset: Account for boot time isolated CPUs")
> Signed-off-by: Guopeng Zhang <zhangguopeng@kylinos.cn>
> ---
>   kernel/cgroup/cpuset.c | 39 +++++++++++++++++++++++++++++----------
>   1 file changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index d100634fa12b..2538faac9aba 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1259,6 +1259,28 @@ static void reset_partition_data(struct cpuset *cs)
>   		cpumask_copy(cs->effective_cpus, parent->effective_cpus);
>   }
>   
> +/* Return true if isolated_cpus changes. */
> +static bool isolated_cpu_update(int new_prs, int cpu)
> +{
> +	lockdep_assert_held(&callback_lock);
> +	lockdep_assert_held(&cpuset_mutex);
> +
> +	if (new_prs == PRS_ISOLATED) {
> +		if (cpumask_test_cpu(cpu, isolated_cpus))
> +			return false;
> +		cpumask_set_cpu(cpu, isolated_cpus);
> +		return true;
> +	}
> +
> +	/* CPUs isolated at boot must remain isolated. */
> +	if (!cpumask_test_cpu(cpu,
> +			      housekeeping_cpumask(HK_TYPE_DOMAIN_BOOT)) ||
> +	    !cpumask_test_cpu(cpu, isolated_cpus))
> +		return false;
> +	cpumask_clear_cpu(cpu, isolated_cpus);
> +	return true;
> +}
> +
>   /*
>    * isolated_cpus_update - Update the isolated_cpus mask
>    * @old_prs: old partition_root_state
> @@ -1267,19 +1289,16 @@ static void reset_partition_data(struct cpuset *cs)
>    */
>   static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus)
>   {
> +	bool updated = false;
> +	int cpu;
> +
>   	WARN_ON_ONCE(old_prs == new_prs);
>   	lockdep_assert_held(&callback_lock);
>   	lockdep_assert_held(&cpuset_mutex);
> -	if (new_prs == PRS_ISOLATED) {
> -		if (cpumask_subset(xcpus, isolated_cpus))
> -			return;
> -		cpumask_or(isolated_cpus, isolated_cpus, xcpus);
> -	} else {
> -		if (!cpumask_intersects(xcpus, isolated_cpus))
> -			return;
> -		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
> -	}
> -	update_housekeeping = true;
> +	for_each_cpu(cpu, xcpus)
> +		updated |= isolated_cpu_update(new_prs, cpu);
> +	if (updated)
> +		update_housekeeping = true;
>   }
>   
>   /*

The code is technically correct. However, handling it one CPU at a time 
is less efficient from my point of view. It can be handled more 
efficiently from the cpumask level. I do notice that the new 
isolated_cpu_update() helper is being used in a later patch in your 
larger series. Maybe that is the reason why you do it this way.

I am OK with this change.

Acked-by: Waiman Long <longman@redhat.com>