Re: [PATCH v2 1/4] cgroup/cpuset: Reset partition data when an invalid partition stays invalid

[Waiman Long <longman@redhat.com>]

On 11/6/26 5:55 AM, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
>
> When cpuset.cpus or cpuset.cpus.exclusive of a partition root is
> changed, update_cpumask() and update_exclusive_cpumask() copy the new
> effective_xcpus computed for the trial cpuset into the cpuset, and call
> reset_partition_data() only if the partition was valid before and is
> invalid now:
>
> 	if ((old_prs > 0) && !is_partition_valid(cs))
> 		reset_partition_data(cs);
>
> An invalid partition root (old_prs < 0) that stays invalid is not
> covered. Its effective_xcpus is then left with the trial value, which
> is computed from user_xcpus() and so falls back to cpuset.cpus when
> cpuset.cpus.exclusive is cleared. As an invalid partition root does not
> own any exclusive CPUs, and rm_siblings_excl_cpus() treats a non-empty
> effective_xcpus of a sibling without cpuset.cpus.exclusive as owned,
> those CPUs can then no longer be claimed by a sibling:
>
>    # A1: cpuset.cpus=2-3, cpuset.cpus.exclusive=2-3, root, with tasks
>    # offline CPUs 2-3 -> A1 becomes invalid
>    echo > A1/cpuset.cpus.exclusive
>    cat A1/cpuset.cpus.exclusive.effective    # 2-3, expected empty
>    echo 3 > B1/cpuset.cpus.exclusive         # -EINVAL
>
> Reset the partition data whenever the cpuset is a partition root, valid
> or not, and ends up invalid. As effective_xcpus may be cleared by that,
> also force update_cpumasks_hier() to update the subtree, so that the
> effective_xcpus of the descendants doesn't keep the released CPUs. Add
> test cases for it.
>
> Fixes: de9f15e21c55 ("cpuset: use parse_cpulist for setting cpus.exclusive")
> Fixes: c5866c9a007d ("cpuset: refactor CPU mask buffer parsing logic")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20261101114206.2434398-1-ridong.chen%40linux.dev?part=15
> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> ---
>   kernel/cgroup/cpuset.c                            | 10 ++++++++--
>   tools/testing/selftests/cgroup/test_cpuset_prs.sh |  6 ++++++
>   2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 3052413e41e7..cf38e8708ddd 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -2607,8 +2607,11 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>   	spin_lock_irq(&callback_lock);
>   	cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
>   	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
> -	if ((old_prs > 0) && !is_partition_valid(cs))
> +	if (old_prs && !is_partition_valid(cs)) {
>   		reset_partition_data(cs);
> +		/* effective_xcpus may be cleared, update the subtree too */
> +		force = true;
> +	}
>   	spin_unlock_irq(&callback_lock);
>   
>   	/* effective_cpus/effective_xcpus will be updated here */
> @@ -2672,8 +2675,11 @@ static int update_exclusive_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>   	spin_lock_irq(&callback_lock);
>   	cpumask_copy(cs->exclusive_cpus, trialcs->exclusive_cpus);
>   	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
> -	if ((old_prs > 0) && !is_partition_valid(cs))
> +	if (old_prs && !is_partition_valid(cs)) {
>   		reset_partition_data(cs);
> +		/* effective_xcpus may be cleared, update the subtree too */
> +		force = true;
> +	}
>   	spin_unlock_irq(&callback_lock);
>   
>   	/*
> diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> index bdca6c471f87..32e1e44fa0b1 100755
> --- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> +++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
> @@ -434,6 +434,12 @@ TEST_MATRIX=(
>   	# cpuset.cpus is allowed, but those extra CPUs are ignored.
>   	"  C2-3:P1   C3:P1  .      .      .     C2-4    .      .     0 A1:|A2:2-3 A1:P1|A2:P1"
>   
> +	# Clearing cpuset.cpus.exclusive of an invalid partition releases its
> +	# exclusive CPUs, including those of its descendants, so a sibling
> +	# can claim them.
> +	" CX2-3:P1    .     .     C3  T:O2=0:O3=0:X  .     .      X3   0 XA1:|XB1:3 A1:P-1"
> +	" CX2-3:P1   X3     .     C3  T:O2=0:O3=0:X  .     .      X3   0 XA1:|XA2:|XB1:3 A1:P-1"
> +
>   	# A child cannot recover until its exclusive CPUs are available again.
>   	" CX1-4:P1 CX3-4:P1 .     .     X1-2     .      .      .     0 A1:1-2|A2:1-2|XA2: A1:P1|A2:P-1"
>   	" CX1-4:P1 CX3-4:P1 .     .  X1-2:X1-4   .      .      .     0 A1:1-2|A2:3-4|XA2:3-4 A1:P1|A2:P1"

LGTM

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