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

[Ridong Chen <ridong.chen@linux.dev>]

On 9/4/2026 2:33 AM, Waiman Long wrote:
> On 9/2/26 6:26 AM, Guopeng Zhang wrote:
>> From: Guopeng Zhang <zhangguopeng@kylinos.cn>
>>
>> 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@kylinos.cn>
>> ---
>>   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)
> I think you should add some functional comments on what "partition_cpus" and 
> "remaining_cpus" are supposed to be so that caller knows what to pass into this 
> helper.

Would it help to rename them to excpus and local_excpus?
local already implies "excluding children" (just like cgroup.stat.local), so I 
think that makes the intent clearer.

>> +{
>> +    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))
>> -            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) {
>> +        WRITE_ONCE(child->prs_err, PERR_NONE);
>> +        child_err = child_partition_error(child, cs->effective_xcpus,
>> +                          new_ecpus, populated);
>> +        if (child_err)
>> +            WRITE_ONCE(child->prs_err, child_err);
>> +
> 
> You can ignore the inital PERR_NONE write and always write the child_err value 
> into child->prs_err.
> 
> Not big issue, just some nits.
> 
> Cheers,
> Longman
> 
>> +        if (child_err) {
>>               int old_prs = child->partition_root_state;
>>               /*
> 

-- 
Best regards
Ridong