[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <90939118-df03-4c26-9351-1a4ec67116eb@huaweicloud.com>
Date: Thu, 13 Nov 2025 15:16:58 +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 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@...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.
>
> 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
Powered by blists - more mailing lists