[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8bb19728-8611-4266-aa8e-6dd4783c4702@redhat.com>
Date: Wed, 12 Nov 2025 21:58:08 -0500
From: Waiman Long <llong@...hat.com>
To: Chen Ridong <chenridong@...weicloud.com>, 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 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
Powered by blists - more mailing lists