[PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations

Chen Ridong posted 1 patch 1 month, 3 weeks ago
kernel/cgroup/cpuset-internal.h |  2 ++
kernel/cgroup/cpuset-v1.c       | 46 +++++++++++++++++++++++++++++++++
kernel/cgroup/cpuset.c          | 39 +---------------------------
3 files changed, 49 insertions(+), 38 deletions(-)
[PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Chen Ridong 1 month, 3 weeks ago
From: Chen Ridong <chenridong@huawei.com>

This commit introduces the cpuset1_online_css helper to centralize
v1-specific handling during cpuset online. It performs operations such as
updating the CS_SPREAD_PAGE, CS_SPREAD_SLAB, and CGRP_CPUSET_CLONE_CHILDREN
flags, which are unique to the cpuset v1 control group interface.

The helper is now placed in cpuset-v1.c to maintain clear separation
between v1 and v2 logic.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset-internal.h |  2 ++
 kernel/cgroup/cpuset-v1.c       | 46 +++++++++++++++++++++++++++++++++
 kernel/cgroup/cpuset.c          | 39 +---------------------------
 3 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index 01976c8e7d49..d3bde90ac6f3 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -293,6 +293,7 @@ void cpuset1_hotplug_update_tasks(struct cpuset *cs,
 			    struct cpumask *new_cpus, nodemask_t *new_mems,
 			    bool cpus_updated, bool mems_updated);
 int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial);
+void cpuset1_online_css(struct cgroup_subsys_state *css);
 #else
 static inline void fmeter_init(struct fmeter *fmp) {}
 static inline void cpuset1_update_task_spread_flags(struct cpuset *cs,
@@ -303,6 +304,7 @@ static inline void cpuset1_hotplug_update_tasks(struct cpuset *cs,
 			    bool cpus_updated, bool mems_updated) {}
 static inline int cpuset1_validate_change(struct cpuset *cur,
 				struct cpuset *trial) { return 0; }
+static inline void  cpuset1_online_css(struct cgroup_subsys_state *css) {}
 #endif /* CONFIG_CPUSETS_V1 */
 
 #endif /* __CPUSET_INTERNAL_H */
diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 12e76774c75b..1d83c2b26ff0 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -499,6 +499,52 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 	return retval;
 }
 
+/* v1-specific operation — caller must hold cpuset_full_lock. */
+void cpuset1_online_css(struct cgroup_subsys_state *css)
+{
+	struct cpuset *tmp_cs;
+	struct cgroup_subsys_state *pos_css;
+	struct cpuset *cs = css_cs(css);
+	struct cpuset *parent = parent_cs(cs);
+
+	if (is_spread_page(parent))
+		set_bit(CS_SPREAD_PAGE, &cs->flags);
+	if (is_spread_slab(parent))
+		set_bit(CS_SPREAD_SLAB, &cs->flags);
+
+	if (!test_bit(CGRP_CPUSET_CLONE_CHILDREN, &css->cgroup->flags))
+		return;
+
+	/*
+	 * Clone @parent's configuration if CGRP_CPUSET_CLONE_CHILDREN is
+	 * set.  This flag handling is implemented in cgroup core for
+	 * historical reasons - the flag may be specified during mount.
+	 *
+	 * Currently, if any sibling cpusets have exclusive cpus or mem, we
+	 * refuse to clone the configuration - thereby refusing the task to
+	 * be entered, and as a result refusing the sys_unshare() or
+	 * clone() which initiated it.  If this becomes a problem for some
+	 * users who wish to allow that scenario, then this could be
+	 * changed to grant parent->cpus_allowed-sibling_cpus_exclusive
+	 * (and likewise for mems) to the new cgroup.
+	 */
+	rcu_read_lock();
+	cpuset_for_each_child(tmp_cs, pos_css, parent) {
+		if (is_mem_exclusive(tmp_cs) || is_cpu_exclusive(tmp_cs)) {
+			rcu_read_unlock();
+			return;
+		}
+	}
+	rcu_read_unlock();
+
+	cpuset_callback_lock_irq();
+	cs->mems_allowed = parent->mems_allowed;
+	cs->effective_mems = parent->mems_allowed;
+	cpumask_copy(cs->cpus_allowed, parent->cpus_allowed);
+	cpumask_copy(cs->effective_cpus, parent->cpus_allowed);
+	cpuset_callback_unlock_irq();
+}
+
 /*
  * for the common functions, 'private' gives the type of file
  */
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index fea577b4016a..ba645ba09a25 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3611,17 +3611,11 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 {
 	struct cpuset *cs = css_cs(css);
 	struct cpuset *parent = parent_cs(cs);
-	struct cpuset *tmp_cs;
-	struct cgroup_subsys_state *pos_css;
 
 	if (!parent)
 		return 0;
 
 	cpuset_full_lock();
-	if (is_spread_page(parent))
-		set_bit(CS_SPREAD_PAGE, &cs->flags);
-	if (is_spread_slab(parent))
-		set_bit(CS_SPREAD_SLAB, &cs->flags);
 	/*
 	 * For v2, clear CS_SCHED_LOAD_BALANCE if parent is isolated
 	 */
@@ -3636,39 +3630,8 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 		cs->effective_mems = parent->effective_mems;
 	}
 	spin_unlock_irq(&callback_lock);
+	cpuset1_online_css(css);
 
-	if (!test_bit(CGRP_CPUSET_CLONE_CHILDREN, &css->cgroup->flags))
-		goto out_unlock;
-
-	/*
-	 * Clone @parent's configuration if CGRP_CPUSET_CLONE_CHILDREN is
-	 * set.  This flag handling is implemented in cgroup core for
-	 * historical reasons - the flag may be specified during mount.
-	 *
-	 * Currently, if any sibling cpusets have exclusive cpus or mem, we
-	 * refuse to clone the configuration - thereby refusing the task to
-	 * be entered, and as a result refusing the sys_unshare() or
-	 * clone() which initiated it.  If this becomes a problem for some
-	 * users who wish to allow that scenario, then this could be
-	 * changed to grant parent->cpus_allowed-sibling_cpus_exclusive
-	 * (and likewise for mems) to the new cgroup.
-	 */
-	rcu_read_lock();
-	cpuset_for_each_child(tmp_cs, pos_css, parent) {
-		if (is_mem_exclusive(tmp_cs) || is_cpu_exclusive(tmp_cs)) {
-			rcu_read_unlock();
-			goto out_unlock;
-		}
-	}
-	rcu_read_unlock();
-
-	spin_lock_irq(&callback_lock);
-	cs->mems_allowed = parent->mems_allowed;
-	cs->effective_mems = parent->mems_allowed;
-	cpumask_copy(cs->cpus_allowed, parent->cpus_allowed);
-	cpumask_copy(cs->effective_cpus, parent->cpus_allowed);
-	spin_unlock_irq(&callback_lock);
-out_unlock:
 	cpuset_full_unlock();
 	return 0;
 }
-- 
2.34.1

Re: [PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Michal Koutný 1 month, 3 weeks ago
On Tue, Dec 16, 2025 at 01:28:45AM +0000, Chen Ridong <chenridong@huaweicloud.com> wrote:
> From: Chen Ridong <chenridong@huawei.com>
> 
> This commit introduces the cpuset1_online_css helper to centralize
> v1-specific handling during cpuset online. It performs operations such as
> updating the CS_SPREAD_PAGE, CS_SPREAD_SLAB, and CGRP_CPUSET_CLONE_CHILDREN
> flags, which are unique to the cpuset v1 control group interface.
> 
> The helper is now placed in cpuset-v1.c to maintain clear separation
> between v1 and v2 logic.

It makes sense to me.

> +/* v1-specific operation — caller must hold cpuset_full_lock. */
> +void cpuset1_online_css(struct cgroup_subsys_state *css)
> +{
> +	struct cpuset *tmp_cs;
> +	struct cgroup_subsys_state *pos_css;
> +	struct cpuset *cs = css_cs(css);
> +	struct cpuset *parent = parent_cs(cs);
> +

+	lockdep_assert_held(&cpuset_mutex);
+	lockdep_assert_cpus_held();

When it's carved out from under cpuset_full_lock().


> @@ -3636,39 +3630,8 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
>  		cs->effective_mems = parent->effective_mems;
>  	}
>  	spin_unlock_irq(&callback_lock);
> +	cpuset1_online_css(css);

guard with !is_in_v2mode()

Thanks,
Michal
Re: [PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Chen Ridong 1 month, 3 weeks ago

On 2025/12/16 17:51, Michal Koutný wrote:
> On Tue, Dec 16, 2025 at 01:28:45AM +0000, Chen Ridong <chenridong@huaweicloud.com> wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> This commit introduces the cpuset1_online_css helper to centralize
>> v1-specific handling during cpuset online. It performs operations such as
>> updating the CS_SPREAD_PAGE, CS_SPREAD_SLAB, and CGRP_CPUSET_CLONE_CHILDREN
>> flags, which are unique to the cpuset v1 control group interface.
>>
>> The helper is now placed in cpuset-v1.c to maintain clear separation
>> between v1 and v2 logic.
> 
> It makes sense to me.
> 
>> +/* v1-specific operation — caller must hold cpuset_full_lock. */
>> +void cpuset1_online_css(struct cgroup_subsys_state *css)
>> +{
>> +	struct cpuset *tmp_cs;
>> +	struct cgroup_subsys_state *pos_css;
>> +	struct cpuset *cs = css_cs(css);
>> +	struct cpuset *parent = parent_cs(cs);
>> +
> 
> +	lockdep_assert_held(&cpuset_mutex);
> +	lockdep_assert_cpus_held();
> 

Thanks for the feedback, Michal.

Regarding the lock assertions: cpuset_mutex is defined in cpuset.c and is not visible in
cpuset-v1.c. Given that cpuset v1 is deprecated, would you prefer that we add a helper to assert
cpuset_mutex is locked? Is that worth?

> When it's carved out from under cpuset_full_lock().
> 
> 
>> @@ -3636,39 +3630,8 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
>>  		cs->effective_mems = parent->effective_mems;
>>  	}
>>  	spin_unlock_irq(&callback_lock);
>> +	cpuset1_online_css(css);
> 
> guard with !is_in_v2mode()
> 

Should we guard with !cpuset_v2() or !is_in_v2mode()?

In cgroup v1, if the cpuset is operating in v2 mode, are these flags still valid?

-- 
Best regards,
Ridong

Re: [PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Michal Koutný 1 month, 3 weeks ago
On Tue, Dec 16, 2025 at 08:13:53PM +0800, Chen Ridong <chenridong@huaweicloud.com> wrote:
> Regarding the lock assertions: cpuset_mutex is defined in cpuset.c and is not visible in
> cpuset-v1.c. Given that cpuset v1 is deprecated, would you prefer that we add a helper to assert
> cpuset_mutex is locked? Is that worth?

It could be un-static'd and defined in cpuset-internal.h. (Hopefully, we
should not end up with random callers of the helper but it's IMO worth
it for docs and greater safety.)

> Should we guard with !cpuset_v2() or !is_in_v2mode()?
> 
> In cgroup v1, if the cpuset is operating in v2 mode, are these flags still valid?

I have no experience with this transitional option so that made me look
at the docs and there we specify it only affects behaviors of CPU masks,
not the extra flags. So I wanted to suggest !cpuset_v2(), correct?

Thanks,
Michal
Re: [PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Waiman Long 1 month, 3 weeks ago
On 12/16/25 9:03 AM, Michal Koutný wrote:
> On Tue, Dec 16, 2025 at 08:13:53PM +0800, Chen Ridong <chenridong@huaweicloud.com> wrote:
>> Regarding the lock assertions: cpuset_mutex is defined in cpuset.c and is not visible in
>> cpuset-v1.c. Given that cpuset v1 is deprecated, would you prefer that we add a helper to assert
>> cpuset_mutex is locked? Is that worth?
> It could be un-static'd and defined in cpuset-internal.h. (Hopefully, we
> should not end up with random callers of the helper but it's IMO worth
> it for docs and greater safety.)
I would suggest defining a "assert_cpuset_lock_held(void)" helper 
function and put the declaration in include/linux/cpuset.h together with 
cpuset_lock/unlock() to complete the full set. This will allow other 
kernel subsystems to acquire the cpuset_mutex and assert that the mutex 
was held.
>
>> Should we guard with !cpuset_v2() or !is_in_v2mode()?
>>
>> In cgroup v1, if the cpuset is operating in v2 mode, are these flags still valid?
> I have no experience with this transitional option so that made me look
> at the docs and there we specify it only affects behaviors of CPU masks,
> not the extra flags. So I wanted to suggest !cpuset_v2(), correct?

The "cpuset_v2_mode" mount flag is used for making the behavior of 
cpuset.{cpus,mems}.effective in v1 behave like in v2. It has no effect 
on other v1 specific control files. So cpuset1_online_css() should only 
be called if "!cpuset_v2()".

Cheers,
Longman

>
> Thanks,
> Michal

Re: [PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Chen Ridong 1 month, 3 weeks ago

On 2025/12/16 22:58, Waiman Long wrote:
> On 12/16/25 9:03 AM, Michal Koutný wrote:
>> On Tue, Dec 16, 2025 at 08:13:53PM +0800, Chen Ridong <chenridong@huaweicloud.com> wrote:
>>> Regarding the lock assertions: cpuset_mutex is defined in cpuset.c and is not visible in
>>> cpuset-v1.c. Given that cpuset v1 is deprecated, would you prefer that we add a helper to assert
>>> cpuset_mutex is locked? Is that worth?
>> It could be un-static'd and defined in cpuset-internal.h. (Hopefully, we
>> should not end up with random callers of the helper but it's IMO worth
>> it for docs and greater safety.)
> I would suggest defining a "assert_cpuset_lock_held(void)" helper function and put the declaration
> in include/linux/cpuset.h together with cpuset_lock/unlock() to complete the full set. This will
> allow other kernel subsystems to acquire the cpuset_mutex and assert that the mutex was held.

Considering potential use by other subsystems, this is worthwhile. I will add the helper.

>>
>>> Should we guard with !cpuset_v2() or !is_in_v2mode()?
>>>
>>> In cgroup v1, if the cpuset is operating in v2 mode, are these flags still valid?
>> I have no experience with this transitional option so that made me look
>> at the docs and there we specify it only affects behaviors of CPU masks,
>> not the extra flags. So I wanted to suggest !cpuset_v2(), correct?
> 
> The "cpuset_v2_mode" mount flag is used for making the behavior of cpuset.{cpus,mems}.effective in
> v1 behave like in v2. It has no effect on other v1 specific control files. So cpuset1_online_css()
> should only be called if "!cpuset_v2()".
> 

If cpuset1_online_css() is only called under the condition !cpuset_v2(), a compile-time option might
suffice? When CONFIG_CPUSETS_V1=n, cpuset1_online_css could be defined as an empty inline function.

-- 
Best regards,
Ridong

Re: [PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Waiman Long 1 month, 3 weeks ago
On 12/16/25 7:53 PM, Chen Ridong wrote:
>
> On 2025/12/16 22:58, Waiman Long wrote:
>> On 12/16/25 9:03 AM, Michal Koutný wrote:
>>> On Tue, Dec 16, 2025 at 08:13:53PM +0800, Chen Ridong <chenridong@huaweicloud.com> wrote:
>>>> Regarding the lock assertions: cpuset_mutex is defined in cpuset.c and is not visible in
>>>> cpuset-v1.c. Given that cpuset v1 is deprecated, would you prefer that we add a helper to assert
>>>> cpuset_mutex is locked? Is that worth?
>>> It could be un-static'd and defined in cpuset-internal.h. (Hopefully, we
>>> should not end up with random callers of the helper but it's IMO worth
>>> it for docs and greater safety.)
>> I would suggest defining a "assert_cpuset_lock_held(void)" helper function and put the declaration
>> in include/linux/cpuset.h together with cpuset_lock/unlock() to complete the full set. This will
>> allow other kernel subsystems to acquire the cpuset_mutex and assert that the mutex was held.
> Considering potential use by other subsystems, this is worthwhile. I will add the helper.
>
>>>> Should we guard with !cpuset_v2() or !is_in_v2mode()?
>>>>
>>>> In cgroup v1, if the cpuset is operating in v2 mode, are these flags still valid?
>>> I have no experience with this transitional option so that made me look
>>> at the docs and there we specify it only affects behaviors of CPU masks,
>>> not the extra flags. So I wanted to suggest !cpuset_v2(), correct?
>> The "cpuset_v2_mode" mount flag is used for making the behavior of cpuset.{cpus,mems}.effective in
>> v1 behave like in v2. It has no effect on other v1 specific control files. So cpuset1_online_css()
>> should only be called if "!cpuset_v2()".
>>
> If cpuset1_online_css() is only called under the condition !cpuset_v2(), a compile-time option might
> suffice? When CONFIG_CPUSETS_V1=n, cpuset1_online_css could be defined as an empty inline function.

cpuset_v2() includes "!IS_ENABLED(CONFIG_CPUSETS_V1)", so the compiler 
should compile out the call to cpuset1_online_css() if CONFIG_CPUSETS_V1 
isn't defined. If you want to make cpuset1_online_css() conditional on 
CONFIG_CPUSETS_V1, I am fine with that too.

Cheers,
Longman

Re: [PATCH -next] cpuset: add cpuset1_online_css helper for v1-specific operations
Posted by Chen Ridong 1 month, 3 weeks ago

On 2025/12/17 10:03, Waiman Long wrote:
> 
> On 12/16/25 7:53 PM, Chen Ridong wrote:
>>
>> On 2025/12/16 22:58, Waiman Long wrote:
>>> On 12/16/25 9:03 AM, Michal Koutný wrote:
>>>> On Tue, Dec 16, 2025 at 08:13:53PM +0800, Chen Ridong <chenridong@huaweicloud.com> wrote:
>>>>> Regarding the lock assertions: cpuset_mutex is defined in cpuset.c and is not visible in
>>>>> cpuset-v1.c. Given that cpuset v1 is deprecated, would you prefer that we add a helper to assert
>>>>> cpuset_mutex is locked? Is that worth?
>>>> It could be un-static'd and defined in cpuset-internal.h. (Hopefully, we
>>>> should not end up with random callers of the helper but it's IMO worth
>>>> it for docs and greater safety.)
>>> I would suggest defining a "assert_cpuset_lock_held(void)" helper function and put the declaration
>>> in include/linux/cpuset.h together with cpuset_lock/unlock() to complete the full set. This will
>>> allow other kernel subsystems to acquire the cpuset_mutex and assert that the mutex was held.
>> Considering potential use by other subsystems, this is worthwhile. I will add the helper.
>>
>>>>> Should we guard with !cpuset_v2() or !is_in_v2mode()?
>>>>>
>>>>> In cgroup v1, if the cpuset is operating in v2 mode, are these flags still valid?
>>>> I have no experience with this transitional option so that made me look
>>>> at the docs and there we specify it only affects behaviors of CPU masks,
>>>> not the extra flags. So I wanted to suggest !cpuset_v2(), correct?
>>> The "cpuset_v2_mode" mount flag is used for making the behavior of cpuset.{cpus,mems}.effective in
>>> v1 behave like in v2. It has no effect on other v1 specific control files. So cpuset1_online_css()
>>> should only be called if "!cpuset_v2()".
>>>
>> If cpuset1_online_css() is only called under the condition !cpuset_v2(), a compile-time option might
>> suffice? When CONFIG_CPUSETS_V1=n, cpuset1_online_css could be defined as an empty inline function.
> 
> cpuset_v2() includes "!IS_ENABLED(CONFIG_CPUSETS_V1)", so the compiler should compile out the call
> to cpuset1_online_css() if CONFIG_CPUSETS_V1 isn't defined. If you want to make cpuset1_online_css()
> conditional on CONFIG_CPUSETS_V1, I am fine with that too.
> 

The cpuset1_online_css() resides in cpuset-v1.c and is only compiled when CONFIG_CPUSETS_V1=y. For
cases where CONFIG_CPUSETS_V1=n, I have defined it as an empty inline function.

-- 
Best regards,
Ridong