Re: [PATCH -next RFC -v2 09/11] cpuset: refactor partition_cpus_change

[Chen Ridong]

On 2025/9/16 3:34, Waiman Long wrote:
> On 9/8/25 11:32 PM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@xxxxxxxxxx>
>>
>> Refactor the partition_cpus_change function to handle both regular CPU
>> set updates and exclusive CPU modifications, either of which may trigger
>> partition state changes. This generalized function will also be utilized
>> for exclusive CPU updates in subsequent patches.
>>
>> With the introduction of compute_trialcs_excpus in a previous patch,
>> the trialcs->effective_xcpus field is now consistently computed and
>> maintained. Consequently, the legacy logic which assigned
>> **trialcs->allowed_cpus to a local 'xcpus' variable** when
>> trialcs->effective_xcpus was empty has been removed.
>>
>> This removal is safe because when trialcs is not a partition member,
>> trialcs->effective_xcpus is now correctly populated with the intersection
>> of user_xcpus and the parent's effective_xcpus. This calculation inherently
>> covers the scenario previously handled by the removed code.
>>
>> Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
>> ---
>>   kernel/cgroup/cpuset.c | 66 +++++++++++++++++++++++++-----------------
>>   1 file changed, 40 insertions(+), 26 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 6aa93bd9d5dd..de61520f1e44 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -2453,6 +2453,45 @@ static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset
>> *trialc
>>       return retval;
>>   }
>>   +/**
>> + * partition_cpus_change - Handle partition state changes due to CPU mask updates
>> + * @cs: The target cpuset being modified
>> + * @trialcs: The trial cpuset containing proposed configuration changes
>> + * @tmp: Temporary masks for intermediate calculations
>> + *
>> + * This function handles partition state transitions triggered by CPU mask changes.
>> + * CPU modifications may cause a partition to be disabled or require state updates.
>> + */
>> +static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>> +                    struct tmpmasks *tmp)
>> +{
>> +    enum prs_errcode prs_err;
>> +
>> +    if (cs_is_member(cs))
>> +        return;
>> +
>> +    prs_err = validate_partition(cs, trialcs);
>> +    if (prs_err) {
>> +        trialcs->prs_err = prs_err;
>> +        cs->prs_err = prs_err;
>> +    }
> 
> The assignment can be simplified into
> 
>     trialcs->prs_err = cs->prs_err = prs_err;
> 

Thank you, will update.

>> +
>> +    if (is_remote_partition(cs)) {
>> +        if (trialcs->prs_err)
>> +            remote_partition_disable(cs, tmp);
>> +        else
>> +            remote_cpus_update(cs, trialcs->exclusive_cpus,
>> +                       trialcs->effective_xcpus, tmp);
>> +    } else {
>> +        if (trialcs->prs_err)
>> +            update_parent_effective_cpumask(cs, partcmd_invalidate,
>> +                            NULL, tmp);
>> +        else
>> +            update_parent_effective_cpumask(cs, partcmd_update,
>> +                            trialcs->effective_xcpus, tmp);
>> +    }
>> +}
>> +
>>   /**
>>    * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it
>>    * @cs: the cpuset to consider
>> @@ -2466,7 +2505,6 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>>       struct tmpmasks tmp;
>>       bool force = false;
>>       int old_prs = cs->partition_root_state;
>> -    enum prs_errcode prs_err;
>>         retval = parse_cpuset_cpulist(buf, trialcs->cpus_allowed);
>>       if (retval < 0)
>> @@ -2491,31 +2529,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>>        */
>>       force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus);
>>   -    prs_err = validate_partition(cs, trialcs);
>> -    if (prs_err) {
>> -        trialcs->prs_err = prs_err;
>> -        cs->prs_err = prs_err;
>> -    }
>> -
>> -    if (is_partition_valid(cs) ||
>> -       (is_partition_invalid(cs) && !trialcs->prs_err)) {
>> -        struct cpumask *xcpus = trialcs->effective_xcpus;
>> -
>> -        if (cpumask_empty(xcpus) && is_partition_invalid(cs))
>> -            xcpus = trialcs->cpus_allowed;
>> -
>> -        /*
>> -         * Call remote_cpus_update() to handle valid remote partition
>> -         */
>> -        if (is_remote_partition(cs))
>> -            remote_cpus_update(cs, NULL, xcpus, &tmp);
>> -        else if (trialcs->prs_err)
>> -            update_parent_effective_cpumask(cs, partcmd_invalidate,
>> -                            NULL, &tmp);
>> -        else
>> -            update_parent_effective_cpumask(cs, partcmd_update,
>> -                            xcpus, &tmp);
>> -    }
>> +    partition_cpus_change(cs, trialcs, &tmp);
>>         spin_lock_irq(&callback_lock);
>>       cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
> Reviewed-by: Waiman Long <longman@xxxxxxxxxx>

-- 
Best regards,
Ridong