Re: [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask

[Chen Ridong]

On 2025/10/20 11:02, Waiman Long wrote:
> 
> On 9/28/25 3:13 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@xxxxxxxxxx>
>>
>> Replace the partition invalidation logic in the
>> compute_partition_effective_cpumask() with a call to partition_disable().
>>
>> This centralizes partition state management and ensures consistent
>> handling of partition disable operations throughout the cpuset subsystem.
>>
>> Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
>> ---
>>   kernel/cgroup/cpuset.c | 28 +++++++---------------------
>>   1 file changed, 7 insertions(+), 21 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 6625b803ba02..20288dbd6ccf 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -170,15 +170,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.
>>    */
>> @@ -2073,6 +2064,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>       struct cgroup_subsys_state *css;
>>       struct cpuset *child;
>>       bool populated = partition_is_populated(cs, NULL);
>> +    enum prs_errcode prs_err;
>>         /*
>>        * Check child partition roots to see if they should be
>> @@ -2095,26 +2087,20 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>            * partition root.
>>            */
>>           WARN_ON_ONCE(is_remote_partition(child));
>> -        child->prs_err = 0;
>> +        prs_err = 0;
>>           if (!cpumask_subset(child->effective_xcpus,
>>                       cs->effective_xcpus))
>> -            child->prs_err = PERR_INVCPUS;
>> +            prs_err = PERR_INVCPUS;
>>           else if (populated &&
>>                cpumask_subset(new_ecpus, child->effective_xcpus))
>> -            child->prs_err = PERR_NOCPUS;
>> -
>> -        if (child->prs_err) {
>> -            int old_prs = child->partition_root_state;
>> +            prs_err = PERR_NOCPUS;
>>   +        if (prs_err) {
>>               /*
>>                * Invalidate child partition
>>                */
>> -            spin_lock_irq(&callback_lock);
>> -            make_partition_invalid(child);
>> -            cs->nr_subparts--;
>> -            child->nr_subparts = 0;
>> -            spin_unlock_irq(&callback_lock);
>> -            notify_partition_change(child, old_prs);
>> +            partition_disable(child, parent_cs(child),
> 
> The parent of child should just be cs. You don't need to use parent_cs() to get it.
> 
> Cheers,
> Longman
> 

Thank you, Longman,

Will update.

> 
>> +                      -child->partition_root_state, prs_err);
>>               continue;
>>           }
>>           cpumask_andnot(new_ecpus, new_ecpus,

-- 
Best regards,
Ridong