From: Chen Ridong <chenridong@huawei.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@huawei.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,
@@ -3498,7 +3498,6 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
cpus_read_lock();
mutex_lock(&cpuset_mutex);
- set_bit(CS_ONLINE, &cs->flags);
if (is_spread_page(parent))
set_bit(CS_SPREAD_PAGE, &cs->flags);
if (is_spread_slab(parent))
@@ -3573,7 +3572,6 @@ static void cpuset_css_offline(struct cgroup_subsys_state *css)
cpuset_update_flag(CS_SCHED_LOAD_BALANCE, cs, 0);
cpuset_dec();
- clear_bit(CS_ONLINE, &cs->flags);
mutex_unlock(&cpuset_mutex);
cpus_read_unlock();
--
2.34.1On 8/8/25 5:25 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.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@huawei.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
On 2025/8/12 22:44, Waiman Long wrote:
> On 8/8/25 5:25 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.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@huawei.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?
Best regards,
Ridong
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@huawei.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@huawei.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
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@huawei.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@huawei.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.
Best regards,
Ridong
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@huawei.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@huawei.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@redhat.com>
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@huawei.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@huawei.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@redhat.com>
Thank you.
I will send v2 to fix the others.
Best regard,
Ridong
© 2016 - 2025 Red Hat, Inc.