Message-ID: <47c20c78-9f7f-4134-8835-3c4f5bff4c30@huaweicloud.com>
Date: Thu, 13 Nov 2025 10:49:52 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Waiman Long <llong@...hat.com>, tj@...nel.org, hannes@...xchg.org,
 mkoutny@...e.com
Cc: cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
 lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH RFC v2 10/22] cpuset: introduce local_partition_enable()



On 2025/11/13 5:47, Waiman Long wrote:
> On 10/25/25 2:48 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@...wei.com>
>>
>> The partition_enable() function introduced in the previous patch can be
>> reused to enable local partitions.
>>
>> The local_partition_enable() function is introduced, which factors out the
>> local partition enablement logic from update_parent_effective_cpumask().
>> After passing local partition validation checks, it delegates to
>> partition_enable() to complete the partition setup.
>>
>> This refactoring creates a clear separation between local and remote
>> partition operations while maintaining code reuse through the shared
>> partition_enable() infrastructure.
>>
>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 94 ++++++++++++++++++++++++++----------------
>>   1 file changed, 59 insertions(+), 35 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 5b57c5370641..b308d9f80eef 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1822,6 +1822,61 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>>       remote_partition_disable(cs, tmp);
>>   }
>>   +/**
>> + * local_partition_enable - Enable local partition for a cpuset
>> + * @cs: Target cpuset to become a local partition root
>> + * @new_prs: New partition root state to apply
>> + * @tmp: Temporary masks for CPU calculations
>> + *
>> + * This function enables local partition root capability for a cpuset by
>> + * validating prerequisites, computing exclusive CPUs, and updating the
>> + * partition hierarchy.
>> + *
>> + * Return: 0 on success, error code on failure
>> + */
>> +static int local_partition_enable(struct cpuset *cs,
>> +                int new_prs, struct tmpmasks *tmp)
>> +{
>> +    struct cpuset *parent = parent_cs(cs);
>> +    enum prs_errcode part_error;
>> +    bool cpumask_updated = false;
>> +
>> +    lockdep_assert_held(&cpuset_mutex);
>> +    WARN_ON_ONCE(is_remote_partition(cs));    /* For local partition only */
>> +
>> +    /*
>> +     * The parent must be a partition root.
>> +     * The new cpumask, if present, or the current cpus_allowed must
>> +     * not be empty.
>> +     */
>> +    if (!is_partition_valid(parent)) {
>> +        return is_partition_invalid(parent)
>> +            ? PERR_INVPARENT : PERR_NOTPART;
>> +    }
>> +
>> +    /*
>> +     * Need to call compute_excpus() in case
>> +     * exclusive_cpus not set. Sibling conflict should only happen
>> +     * if exclusive_cpus isn't set.
>> +     */
>> +    if (compute_excpus(cs, tmp->new_cpus))
>> +        WARN_ON_ONCE(!cpumask_empty(cs->exclusive_cpus));
>> +
>> +    part_error = validate_partition(cs, new_prs, tmp->new_cpus);
>> +    if (part_error)
>> +        return part_error;
>> +
>> +    cpumask_updated = cpumask_andnot(tmp->addmask, tmp->new_cpus,
>> +                     parent->effective_cpus);
> 
> What is the purpose of this cpumask_andnot() operation? Is it just to create the cpumask_updated
> boolean? At this point, cpumask_updated should always be true. If not, we have to add validation
> check to return an error.
> 
> Cheers,
> Longman
> 

I want to support switching the root partition’s state between "root" and "isolated"—for example, an
isolated partition switching to root without changing its CPU mask.

Adding a comment to clarify this behavior would be helpful.

>> +    partition_enable(cs, parent, new_prs, tmp->new_cpus);
>> +
>> +    if (cpumask_updated) {
>> +        cpuset_update_tasks_cpumask(parent, tmp->addmask);
>> +        update_sibling_cpumasks(parent, cs, tmp);
>> +    }
>> +    return 0;
>> +}
>> +
>>   /**
>>    * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
>>    * @cs:      The cpuset that requests change in partition root state
>> @@ -1912,34 +1967,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>         nocpu = tasks_nocpu_error(parent, cs, xcpus);
>>   -    if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
>> -        /*
>> -         * Need to call compute_excpus() in case
>> -         * exclusive_cpus not set. Sibling conflict should only happen
>> -         * if exclusive_cpus isn't set.
>> -         */
>> -        xcpus = tmp->delmask;
>> -        if (compute_excpus(cs, xcpus))
>> -            WARN_ON_ONCE(!cpumask_empty(cs->exclusive_cpus));
>> -        new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
>> -
>> -        part_error = validate_partition(cs, new_prs, xcpus);
>> -        if (part_error)
>> -            return part_error;
>> -        /*
>> -         * This function will only be called when all the preliminary
>> -         * checks have passed. At this point, the following condition
>> -         * should hold.
>> -         *
>> -         * (cs->effective_xcpus & cpu_active_mask) ⊆ parent->effective_cpus
>> -         *
>> -         * Warn if it is not the case.
>> -         */
>> -        cpumask_and(tmp->new_cpus, xcpus, cpu_active_mask);
>> -        WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
>> -
>> -        deleting = true;
>> -    } else if (cmd == partcmd_disable) {
>> +    if (cmd == partcmd_disable) {
>>           /*
>>            * May need to add cpus back to parent's effective_cpus
>>            * (and maybe removed from subpartitions_cpus/isolated_cpus)
>> @@ -3062,14 +3090,10 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>            * If parent is valid partition, enable local partiion.
>>            * Otherwise, enable a remote partition.
>>            */
>> -        if (is_partition_valid(parent)) {
>> -            enum partition_cmd cmd = (new_prs == PRS_ROOT)
>> -                           ? partcmd_enable : partcmd_enablei;
>> -
>> -            err = update_parent_effective_cpumask(cs, cmd, NULL, &tmpmask);
>> -        } else {
>> +        if (is_partition_valid(parent))
>> +            err = local_partition_enable(cs, new_prs, &tmpmask);
>> +        else
>>               err = remote_partition_enable(cs, new_prs, &tmpmask);
>> -        }
>>       } else if (old_prs && new_prs) {
>>           /*
>>            * A change in load balance state only, no change in cpumasks.
> 

-- 
Best regards,
Ridong

Powered by blists - more mailing lists