linux-kernel - Re: [PATCH -next 1/4] cpuset: remove redundant CS

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: <cdef61eb-7a70-4106-b2a0-b92cac95972d@huaweicloud.com>
Date: Wed, 13 Aug 2025 14:28:39 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Waiman Long <llong@...hat.com>, tj@...nel.org, hannes@...xchg.org,
 mkoutny@...e.com, mingo@...hat.com, peterz@...radead.org,
 juri.lelli@...hat.com, vincent.guittot@...aro.org, dietmar.eggemann@....com,
 rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de, vschneid@...hat.com
Cc: cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
 lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH -next 1/4] cpuset: remove redundant CS_ONLINE flag



On 2025/8/13 9:33, Waiman Long wrote:
> 
> On 8/12/25 9:20 PM, Chen Ridong wrote:
>>
>> On 2025/8/13 9:00, Waiman Long wrote:
>>> On 8/12/25 8:54 PM, Chen Ridong wrote:
>>>> On 2025/8/12 22:44, Waiman Long wrote:
>>>>> On 8/8/25 5:25 AM, Chen Ridong wrote:
>>>>>> From: Chen Ridong <chenridong@...wei.com>
>>>>>>
>>>>>> The CS_ONLINE flag was introduced prior to the CSS_ONLINE flag in the
>>>>>> cpuset subsystem. Currently, the flag setting sequence is as follows:
>>>>>>
>>>>>> 1. cpuset_css_online() sets CS_ONLINE
>>>>>> 2. css->flags gets CSS_ONLINE set
>>>>>> ...
>>>>>> 3. cgroup->kill_css sets CSS_DYING
>>>>>> 4. cpuset_css_offline() clears CS_ONLINE
>>>>>> 5. css->flags clears CSS_ONLINE
>>>>>>
>>>>>> The is_cpuset_online() check currently occurs between steps 1 and 3.
>>>>>> However, it would be equally safe to perform this check between steps 2
>>>>>> and 3, as CSS_ONLINE provides the same synchronization guarantee as
>>>>>> CS_ONLINE.
>>>>>>
>>>>>> Since CS_ONLINE is redundant with CSS_ONLINE and provides no additional
>>>>>> synchronization benefits, we can safely remove it to simplify the code.
>>>>>>
>>>>>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
>>>>>> ---
>>>>>>     include/linux/cgroup.h          | 5 +++++
>>>>>>     kernel/cgroup/cpuset-internal.h | 3 +--
>>>>>>     kernel/cgroup/cpuset.c          | 4 +---
>>>>>>     3 files changed, 7 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
>>>>>> index b18fb5fcb38e..ae73dbb19165 100644
>>>>>> --- a/include/linux/cgroup.h
>>>>>> +++ b/include/linux/cgroup.h
>>>>>> @@ -354,6 +354,11 @@ static inline bool css_is_dying(struct cgroup_subsys_state *css)
>>>>>>         return css->flags & CSS_DYING;
>>>>>>     }
>>>>>>     +static inline bool css_is_online(struct cgroup_subsys_state *css)
>>>>>> +{
>>>>>> +    return css->flags & CSS_ONLINE;
>>>>>> +}
>>>>>> +
>>>>>>     static inline bool css_is_self(struct cgroup_subsys_state *css)
>>>>>>     {
>>>>>>         if (css == &css->cgroup->self) {
>>>>>> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
>>>>>> index 383963e28ac6..75b3aef39231 100644
>>>>>> --- a/kernel/cgroup/cpuset-internal.h
>>>>>> +++ b/kernel/cgroup/cpuset-internal.h
>>>>>> @@ -38,7 +38,6 @@ enum prs_errcode {
>>>>>>       /* bits in struct cpuset flags field */
>>>>>>     typedef enum {
>>>>>> -    CS_ONLINE,
>>>>>>         CS_CPU_EXCLUSIVE,
>>>>>>         CS_MEM_EXCLUSIVE,
>>>>>>         CS_MEM_HARDWALL,
>>>>>> @@ -202,7 +201,7 @@ static inline struct cpuset *parent_cs(struct cpuset *cs)
>>>>>>     /* convenient tests for these bits */
>>>>>>     static inline bool is_cpuset_online(struct cpuset *cs)
>>>>>>     {
>>>>>> -    return test_bit(CS_ONLINE, &cs->flags) && !css_is_dying(&cs->css);
>>>>>> +    return css_is_online(&cs->css) && !css_is_dying(&cs->css);
>>>>>>     }
>>>>>>       static inline int is_cpu_exclusive(const struct cpuset *cs)
>>>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>>>> index f74d04429a29..cf7cd2255265 100644
>>>>>> --- a/kernel/cgroup/cpuset.c
>>>>>> +++ b/kernel/cgroup/cpuset.c
>>>>>> @@ -207,7 +207,7 @@ static inline void notify_partition_change(struct cpuset *cs, int old_prs)
>>>>>>      * parallel, we may leave an offline CPU in cpu_allowed or some other masks.
>>>>>>      */
>>>>>>     static struct cpuset top_cpuset = {
>>>>>> -    .flags = BIT(CS_ONLINE) | BIT(CS_CPU_EXCLUSIVE) |
>>>>>> +    .flags = BIT(CS_CPU_EXCLUSIVE) |
>>>>>>              BIT(CS_MEM_EXCLUSIVE) | BIT(CS_SCHED_LOAD_BALANCE),
>>>>>>         .partition_root_state = PRS_ROOT,
>>>>>>         .relax_domain_level = -1,
>>>>> top_cpuset.css is not initialized like the one in the children. If you modify
>>>>> is_cpuset_online() to
>>>>> test the css.flags, you will probably need to set the CSS_ONLINE flag in top_cpuset.css.flags.
>>>>> I do
>>>>> doubt that we will apply the is_cpuset_online() test on top_cpuset. To be consistent, we should
>>>>> support that.
>>>>>
>>>>> BTW, other statically allocated css'es in the cgroup root may have similar problem. If you make
>>>>> the
>>>>> css_is_online() helper available to all other controllers, you will have to document that
>>>>> limitation.
>>>>>
>>>>> Cheers,
>>>>> Longman
>>>> Hi, Longman, thank you for your response.
>>>>
>>>> If I understand correctly, the CSS_ONLINE flag should be set in top_cpuset.css during the following
>>>> process:
>>>>
>>>> css_create
>>>>     css = ss->css_alloc(parent_css);  // cgroup root is static, unlike children
>>>>     online_css(css);
>>>>        ret = ss->css_online(css);     // css root may differ from children
>>>>        css->flags |= CSS_ONLINE;      // css.flags is set with CSS_ONLINE, including the root css
>>>>
>>>> I think css online must be successful, and it's CSS_ONLINE flag must be set. Do I missing anything?
>>> I am talking about just the top_cpuset which is statically allocated. It is not created by the
>>> css_create() call and so the CSS_ONLINE will not be set.
>>>
>>> Cheers,
>>> Longman
>> Hi Longman,
>>
>> Apologies for the call stack earlier. Thank you for your patience in clarifying this matter.
>>
>> The CSS root is brought online through the following initialization flow:
>>
>> cgroup_init_subsys
>>    css = ss->css_alloc(NULL);       // css root is static, unlike children
>>    online_css(css)
>>      ret = ss->css_online(css);     // css root may differ from children
>>      css->flags |= CSS_ONLINE;      // css.flags is set with CSS_ONLINE, including the root css
>>
>> My key point is that:
>> - The root CSS should be online by design.
>> - Root css CSS_ONLINE flag should be properly set during initialization.
> 
> Yes, you are right. I missed css_online() call for the root css for each controller. Thanks for the
> clarification.
> 
> With that, I am OK with this patch. Though the other ones are not good.
> 
> Acked-by: Waiman Long <longman@...hat.com>

Thank you.

I will send v2 to fix the others.

Best regard,
Ridong