Re: [PATCH -next 1/4] cpuset: remove redundant CS_ONLINE flag
From: Waiman Long
Date: Tue Aug 12 2025 - 21:33:56 EST
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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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@xxxxxxxxxx>