Re: [PATCH RFC v2 10/22] cpuset: introduce local_partition_enable()

[Waiman Long <llong@redhat.com>]

On 11/12/25 9:49 PM, Chen Ridong wrote:
>
> On 2025/11/13 5:47, Waiman Long wrote:
>> On 10/25/25 2:48 AM, Chen Ridong wrote:
>>> From: Chen Ridong <chenridong@huawei.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@huawei.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.

For state switching, I would suggest to use a separate helper just for 
state switching as such switching can be done to both local and remote 
partition.

Cheers,
Longman