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

[Chen Ridong]

On 2025/11/13 10:58, Waiman Long wrote:
> 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@xxxxxxxxxx>
>>>>
>>>> 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@xxxxxxxxxx>
>>>> ---
>>>>    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
> 

Thanks, I will try to introduce a new helper to handle it.

-- 
Best regards,
Ridong