linux-kernel - Re: [PATCH -next RFC 03/16] cpuset: factor out partition

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <10bd36ca-aaf9-45c5-9240-453886a92ce4@huaweicloud.com>
Date: Tue, 21 Oct 2025 08:52:42 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Waiman Long <llong@...hat.com>, tj@...nel.org, hannes@...xchg.org,
 mkoutny@...e.com
Cc: cgups@...r.kernel.org, linux-kernel@...r.kernel.org,
 lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH -next RFC 03/16] cpuset: factor out partition_enable()
 function



On 2025/10/21 3:42, Waiman Long wrote:
> On 10/20/25 3:48 AM, Chen Ridong wrote:
>>
>> On 2025/10/20 10:39, Waiman Long wrote:
>>> On 9/28/25 3:12 AM, Chen Ridong wrote:
>>>> From: Chen Ridong <chenridong@...wei.com>
>>>>
>>>> Extract the core partition enablement logic into a dedicated
>>>> partition_enable() function. This refactoring centralizes updates to key
>>>> cpuset data structures including remote_sibling, effective_xcpus,
>>>> partition_root_state, and prs_err.
>>>>
>>>> The function handles the complete partition enablement workflow:
>>>> - Adding exclusive CPUs via partition_xcpus_add()
>>>> - Managing remote sibling relationships
>>>> - Synchronizing effective exclusive CPUs mask
>>>> - Updating partition state and error status
>>>> - Triggering required scheduler domain rebuilds
>>>>
>>>> This creates a coherent interface for partition operations and establishes
>>>> a foundation for future local partition support while maintaining existing
>>>> remote partition behavior.
>>>>
>>>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
>>>> ---
>>>>    kernel/cgroup/cpuset.c | 55 +++++++++++++++++++++++++++++++++---------
>>>>    1 file changed, 44 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>> index 0787904321a9..43ce62f4959c 100644
>>>> --- a/kernel/cgroup/cpuset.c
>>>> +++ b/kernel/cgroup/cpuset.c
>>>> @@ -1515,6 +1515,49 @@ static inline bool is_local_partition(struct cpuset *cs)
>>>>        return is_partition_valid(cs) && !is_remote_partition(cs);
>>>>    }
>>>>    +static void partition_state_update(struct cpuset *cs, int new_prs,
>>>> +                      enum prs_errcode prs_err)
>>>> +{
>>>> +    lockdep_assert_held(&callback_lock);
>>>> +
>>>> +    cs->partition_root_state = new_prs;
>>>> +    WRITE_ONCE(cs->prs_err, prs_err);
>>>> +    if (!is_partition_valid(cs))
>>>> +        reset_partition_data(cs);
>>>> +}
>>>> +
>>>> +/**
>>>> + * partition_enable - Transitions a cpuset to a partition root
>>>> + * @cs: The cpuset to enable partition for
>>>> + * @parent: Parent cpuset of @cs, NULL for remote parent
>>>> + * @new_prs: New partition root state to set
>>>> + * @new_excpus: New exclusive CPUs mask for the partition
>>>> + *
>>>> + * Transitions a cpuset to a partition root, only for v2.
>>>> + */
>>>> +static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>>>> +                 int new_prs, struct cpumask *new_excpus)
>>>> +{
>>>> +    bool isolcpus_updated;
>>>> +
>>>> +    lockdep_assert_held(&cpuset_mutex);
>>>> +    WARN_ON_ONCE(new_prs <= 0);
>>>> +    WARN_ON_ONCE(!cpuset_v2());
>>>> +
>>>> +    if (cs->partition_root_state == new_prs)
>>>> +        return;
>>>> +
>>>> +    spin_lock_irq(&callback_lock);
>>>> +    /* enable partition should only add exclusive cpus */
>>>> +    isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
>>>> +    list_add(&cs->remote_sibling, &remote_children);
>>>> +    cpumask_copy(cs->effective_xcpus, new_excpus);
>>>> +    partition_state_update(cs, new_prs, PERR_NONE);
>>>> +    spin_unlock_irq(&callback_lock);
>>>> +    update_unbound_workqueue_cpumask(isolcpus_updated);
>>>> +    cpuset_force_rebuild();
>>>> +}
>>>> +
>>> partition_enable() is supposed to be a common helper used for the creation of both local and remote
>>> partitions. The one in this patch does work for remote partition but not for local partition. I
>>> would prefer to make it good for both cases when you introduce it instead adding code in patch 6 to
>>> make it work for local partition later in the series. It will make it easier to review instead of
>>> jumping back and forth to make sure that it will do the right thing.
>>>
>>> Cheers,
>>> Longman
>>>
>> Thank you, Longman.
>>
>> My original intention was to keep the changes easier to review. Patches 3–5 are meant to be pure
>> refactoring moves of code from the remote partition logic, without altering any behavior.
>>
>> Would it be clearer to proceed in the following stages:
>>
>> 1. Introduce partition_enable(), partition_disable(), and partition_update() with their complete
>> logic first.
>> 2. Replace the corresponding logic in remote partitions with these new helpers.
>> 3. Then, replace the logic in local partitions with the same helpers.
>>
> Yes, that should make it easier to review.
> 
> Thanks,
> Longman

Thank you for your feedback.
I will update in next version.

-- 
Best regards,
Ridong