Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation

[Guopeng Zhang]

在 2026/9/4 03:11, Waiman Long 写道:
> On 9/2/26 6:26 AM, Guopeng Zhang wrote:
>> From: Guopeng Zhang <zhangguopeng@xxxxxxxxxx>
>>
>> compute_partition_effective_cpumask() checks whether each valid child
>> partition remains covered by the parent exclusive CPU mask and whether it
>> would consume all remaining CPUs of a populated parent.
>>
>> Factor these two checks into child_partition_error() so the same rules can
>> be reused when evaluating a proposed parent configuration. This is a
>> preparatory refactoring with no intended functional change.
>>
>> Signed-off-by: Guopeng Zhang <zhangguopeng@xxxxxxxxxx>
>> ---
>>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++---------
>>   1 file changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 8f24171b6055..6994dc75d940 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>       return 0;
>>   }
>>   +/*
>> + * Return the error that will invalidate a child partition under a proposed
>> + * parent partition configuration.
>> + */
>> +static enum prs_errcode
>> +child_partition_error(struct cpuset *child,
>> +              const struct cpumask *partition_cpus,
>> +              const struct cpumask *remaining_cpus,
>> +              bool parent_populated)
>> +{
>> +    if (!cpumask_subset(child->effective_xcpus, partition_cpus))
>> +        return PERR_INVCPUS;
>> +
>> +    if (parent_populated &&
>> +        cpumask_subset(remaining_cpus, child->effective_xcpus))
>> +        return PERR_NOCPUS;
>> +
>> +    return PERR_NONE;
>> +}
>> +
>>   /**
>>    * compute_partition_effective_cpumask - compute effective_cpus for partition
>>    * @cs: partition root cpuset
>> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>         rcu_read_lock();
>>       cpuset_for_each_child(child, css, cs) {
>> +        enum prs_errcode child_err;
>> +
>>           if (!is_partition_valid(child))
>>               continue;
>>   @@ -2129,15 +2151,13 @@ 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))
> 
> Looking at the patch again, I now see that the current code is incorrect. It should be "if (!cpumask_subset(child->effective_xcpus, new_ecpus))" So you only need to pass the "parent_excpus" (for instance) to the new helper instead of passing two.
> 

Longman, thanks for your review.

Yes, you are right that using `cs->effective_xcpus` here is incorrect.

In v1, I had a change for this issue, and based on your comment [1], I moved it to another fix. I also followed Ridong's suggestion to split the original 17-patch series into smaller series. I split it into three parts locally, and the current series is the second part, so this fix is not included here.

I plan to address this in the third series, together with the CPU ownership and isolation accounting issues following child partition invalidation.

In that series, the two masks have different meanings: the newly computed unfiltered exclusive mask, including offline CPUs, is used for the child containment check, while the active CPU mask remaining after child ownership is accounted for is used for the `PERR_NOCPUS` check. So I would prefer to keep the two-mask interface here and clarify their meanings with comments.

Does this approach make sense to you?

[1] https://lore.kernel.org/all/3bf50da2-f13a-4cef-bfb8-cd592f862164@xxxxxxxxxx/

Thanks,
Guopeng