linux-kernel - Re: [PATCH -next RFC 09/11] cpuset: refactor partition_cpus

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: <5a8b0ebe-95a5-4144-a1d5-b1a96c2312b9@huaweicloud.com>
Date: Wed, 3 Sep 2025 08:51:04 +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 -next RFC 09/11] cpuset: refactor partition_cpus_change



On 2025/9/2 21:30, Waiman Long wrote:
> On 8/29/25 10:01 PM, Chen Ridong wrote:
>>
>> On 2025/8/30 4:32, Waiman Long wrote:
>>> On 8/28/25 8:56 AM, Chen Ridong wrote:
>>>> From: Chen Ridong <chenridong@...wei.com>
>>>>
>>>> Refactor the partition_cpus_change function to handle both regular CPU
>>>> set updates and exclusive CPU modifications, either of which may trigger
>>>> partition state changes. This generalized function will also be utilized
>>>> for exclusive CPU updates in subsequent patches.
>>>>
>>>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
>>>> ---
>>>>    kernel/cgroup/cpuset.c | 59 ++++++++++++++++++++++++++----------------
>>>>    1 file changed, 36 insertions(+), 23 deletions(-)
>>>>
>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>> index 75ad18ab40ae..e3eb87a33b12 100644
>>>> --- a/kernel/cgroup/cpuset.c
>>>> +++ b/kernel/cgroup/cpuset.c
>>>> @@ -2447,6 +2447,41 @@ static int acpus_validate_change(struct cpuset *cs, struct cpuset *trialcs,
>>>>        return retval;
>>>>    }
>>>>    +/**
>>>> + * partition_cpus_change - Handle partition state changes due to CPU mask updates
>>>> + * @cs: The target cpuset being modified
>>>> + * @trialcs: The trial cpuset containing proposed configuration changes
>>>> + * @tmp: Temporary masks for intermediate calculations
>>>> + *
>>>> + * This function handles partition state transitions triggered by CPU mask changes.
>>>> + * CPU modifications may cause a partition to be disabled or require state updates.
>>>> + */
>>>> +static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>>>> +                    struct tmpmasks *tmp)
>>>> +{
>>>> +    if (cs_is_member(cs))
>>>> +        return;
>>>> +
>>>> +    invalidate_cs_partition(trialcs);
>>>> +    if (trialcs->prs_err)
>>>> +        cs->prs_err = trialcs->prs_err;
>>>> +
>>>> +    if (is_remote_partition(cs)) {
>>>> +        if (trialcs->prs_err)
>>>> +            remote_partition_disable(cs, tmp);
>>>> +        else
>>>> +            remote_cpus_update(cs, trialcs->exclusive_cpus,
>>>> +                       trialcs->effective_xcpus, tmp);
>>>> +    } else {
>>>> +        if (trialcs->prs_err)
>>>> +            update_parent_effective_cpumask(cs, partcmd_invalidate,
>>>> +                            NULL, tmp);
>>>> +        else
>>>> +            update_parent_effective_cpumask(cs, partcmd_update,
>>>> +                            trialcs->effective_xcpus, tmp);
>>>> +    }
>>>> +}
>>>> +
>>>>    /**
>>>>     * update_cpumask - update the cpus_allowed mask of a cpuset and all tasks in it
>>>>     * @cs: the cpuset to consider
>>>> @@ -2483,29 +2518,7 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
>>>>         */
>>>>        force = !cpumask_equal(cs->effective_xcpus, trialcs->effective_xcpus);
>>>>    -    invalidate_cs_partition(trialcs);
>>>> -    if (trialcs->prs_err)
>>>> -        cs->prs_err = trialcs->prs_err;
>>>> -
>>>> -    if (is_partition_valid(cs) ||
>>>> -       (is_partition_invalid(cs) && !trialcs->prs_err)) {
>>>> -        struct cpumask *xcpus = trialcs->effective_xcpus;
>>>> -
>>>> -        if (cpumask_empty(xcpus) && is_partition_invalid(cs))
>>>> -            xcpus = trialcs->cpus_allowed;
>>> This if statement was added in commit 46c521bac592 ("cgroup/cpuset: Enable invalid to valid local
>>> partition transition") that is missing in your new partition_cpus_change() function. Have you run
>>> the test_cpuset_prs.sh selftest with a patched kernel to make sure that there is no test failure?
>>>
>>> Cheers,
>>> Longman
>> Thank you Longman,
>>
>> I did run the self-test for every patch, and I appreciate the test script test_cpuset_prs.sh you
>> provided.
>>
>> The trialcs->effective_xcpus will be updated using compute_trialcs_excpus, which was introduced in
>> Patch 4. The corresponding logic was then added in Patch 5:
>>
>> -    cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus);
>> +    /* trialcs is member, cpuset.cpus has no impact to excpus */
>> +    if (cs_is_member(cs))
>> +        cpumask_and(excpus, trialcs->exclusive_cpus,
>> +                parent->effective_xcpus);
>> +    else
>> +        cpumask_and(excpus, user_xcpus(trialcs), parent->effective_xcpus);
>> +
>>
>> Therefore, as long as excpus is computed correctly, I believe this implementation can handle the
>> scenario appropriately.
> 
> It will be helpful to put down a note in the commit log that the missing logic will be re-introduced
> in a subsequent patch.
> 
> Thanks,
> Longman

Thank you Longman,

I will update the commit message.

-- 
Best regards,
Ridong