Re: [PATCH -next 1/4] cpuset: remove redundant CS_ONLINE flag

[Waiman Long <llong@redhat.com>]

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