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] [thread-next>] [day] [month] [year] [list]
Message-ID: <7fdd05cd-4923-4a3f-89e9-d5f5099500ae@huaweicloud.com>
Date: Mon, 20 Oct 2025 15:30:07 +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 01/16] cpuset: use update_partition_sd_lb in
 update_cpumasks_hier



On 2025/10/20 10:37, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@...wei.com>
>>
>> For cgroup v2, when a cpuset is not a valid partition root, it inherits
>> the CS_SCHED_LOAD_BALANCE flag from its parent. The existing logic in
>> update_cpumasks_hier() manually handled this inheritance condition.
>>
>> This patch replaces the inline implementation with a call to the dedicated
>> update_partition_sd_lb() helper function, which already encapsulates the
>> same logic. The helper function comprehensively handles both the load
>> balance flag update and the necessary scheduling domain rebuild decision.
>>
>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 16 +++-------------
>>   1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 52468d2c178a..052f9e0c7a65 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1257,6 +1257,8 @@ static void update_partition_sd_lb(struct cpuset *cs, int old_prs)
>>       bool rebuild_domains = (new_prs > 0) || (old_prs > 0);
>>       bool new_lb;
>>   +    if (!cpuset_v2())
>> +        return;
>>       /*
>>        * If cs is not a valid partition root, the load balance state
>>        * will follow its parent.
>> @@ -2276,19 +2278,7 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
>>               !cpumask_equal(cp->cpus_allowed, cp->effective_cpus));
>>             cpuset_update_tasks_cpumask(cp, cp->effective_cpus);
>> -
>> -        /*
>> -         * On default hierarchy, inherit the CS_SCHED_LOAD_BALANCE
>> -         * from parent if current cpuset isn't a valid partition root
>> -         * and their load balance states differ.
>> -         */
>> -        if (cpuset_v2() && !is_partition_valid(cp) &&
>> -            (is_sched_load_balance(parent) != is_sched_load_balance(cp))) {
>> -            if (is_sched_load_balance(parent))
>> -                set_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
>> -            else
>> -                clear_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
>> -        }
>> +        update_partition_sd_lb(cp, old_prs);
>>             /*
>>            * On legacy hierarchy, if the effective cpumask of any non-
> Calling update_partition_sd_lb() directly from update_cpumasks_hier() may incorrectly force
> rebuidling sched domain when it is not really necessary.
> 
> Cheers,
> Longman

Thank you Longman,

You are correct. Besides the change for the CS_SCHED_LOAD_BALANCE flag, which can lead to rebuilding
the sched domain, the condition of new_prs or old_prs being greater than 0 may also trigger a domain
rebuild, which is unnecessary. I will drop this patch in next version.

-- 
Best regards,
Ridong


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ