[PATCH -next 0/4] some optimization for cpuset

[PATCH -next 0/4] some optimization for cpuset

[Chen Ridong]

From: Chen Ridong <chenridong@huawei.com>

This patch series includes the following improvements:

1. Removes the redundant CS_ONLINE flag.
2. Introduces new helper functions guard_cpuset() and
   guard_cpus_read_and_cpuset().
3. Replaces manual locking patterns with the new guard helpers.

Chen Ridong (4):
  cpuset: remove redundant CS_ONLINE flag
  cpuset: add helpers for cpuset related locks
  cpuset: use guard_cpus_read_and_cpuset to make code concise
  cpuset: replace cpuset_lock() with guard_cpuset()

 include/linux/cgroup.h          |  5 ++++
 include/linux/cpuset.h          |  3 +-
 kernel/cgroup/cpuset-internal.h |  5 ++--
 kernel/cgroup/cpuset-v1.c       | 22 +++++---------
 kernel/cgroup/cpuset.c          | 52 ++++++++++-----------------------
 kernel/sched/syscalls.c         | 15 ++++------
 6 files changed, 37 insertions(+), 65 deletions(-)

-- 
2.34.1

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

[Chen Ridong]

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.1

[PATCH -next 2/4] cpuset: add helpers for cpuset related locks

[Chen Ridong]

From: Chen Ridong <chenridong@huawei.com>

Add guard_cpus_read_and_cpuset and guard_cpuset helpers for cpuset, which
will be user for subsequent patched to make code concise;

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 include/linux/cpuset.h          |  1 +
 kernel/cgroup/cpuset-internal.h |  2 ++
 kernel/cgroup/cpuset.c          | 11 +++++++++++
 3 files changed, 14 insertions(+)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index 2ddb256187b5..6153de28acf0 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -74,6 +74,7 @@ extern void inc_dl_tasks_cs(struct task_struct *task);
 extern void dec_dl_tasks_cs(struct task_struct *task);
 extern void cpuset_lock(void);
 extern void cpuset_unlock(void);
+extern void guard_cpuset(void);
 extern void cpuset_cpus_allowed(struct task_struct *p, struct cpumask *mask);
 extern bool cpuset_cpus_allowed_fallback(struct task_struct *p);
 extern bool cpuset_cpu_is_isolated(int cpu);
diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index 75b3aef39231..084e19fe33d5 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -277,6 +277,8 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
 				    char *buf, size_t nbytes, loff_t off);
 int cpuset_common_seq_show(struct seq_file *sf, void *v);
 
+void guard_cpus_read_and_cpuset(void);
+
 /*
  * cpuset-v1.c
  */
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index cf7cd2255265..f6cdb5cdffe8 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -260,6 +260,17 @@ void cpuset_unlock(void)
 	mutex_unlock(&cpuset_mutex);
 }
 
+void guard_cpuset(void)
+{
+	guard(mutex)(&cpuset_mutex);
+}
+
+void guard_cpus_read_and_cpuset(void)
+{
+	guard(cpus_read_lock)();
+	guard(mutex)(&cpuset_mutex);
+}
+
 static DEFINE_SPINLOCK(callback_lock);
 
 void cpuset_callback_lock_irq(void)
-- 
2.34.1

[PATCH -next 3/4] cpuset: use guard_cpus_read_and_cpuset to make code concise

[Chen Ridong]

From: Chen Ridong <chenridong@huawei.com>

Several functions currently implement manual locking sequences for both
cpus_read_lock() and cpuset_mutex. Replace these repetitive patterns
with the new guard_cpus_read_and_cpuset() helper.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset-v1.c | 22 +++++++---------------
 kernel/cgroup/cpuset.c    | 39 ++++++++++-----------------------------
 2 files changed, 17 insertions(+), 44 deletions(-)

diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index b69a7db67090..70685b90759c 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -169,10 +169,10 @@ static int cpuset_write_s64(struct cgroup_subsys_state *css, struct cftype *cft,
 	cpuset_filetype_t type = cft->private;
 	int retval = -ENODEV;
 
-	cpus_read_lock();
-	cpuset_lock();
+	guard_cpus_read_and_cpuset();
+
 	if (!is_cpuset_online(cs))
-		goto out_unlock;
+		return retval;
 
 	switch (type) {
 	case FILE_SCHED_RELAX_DOMAIN_LEVEL:
@@ -183,9 +183,6 @@ static int cpuset_write_s64(struct cgroup_subsys_state *css, struct cftype *cft,
 		retval = -EINVAL;
 		break;
 	}
-out_unlock:
-	cpuset_unlock();
-	cpus_read_unlock();
 	return retval;
 }
 
@@ -454,12 +451,9 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 	cpuset_filetype_t type = cft->private;
 	int retval = 0;
 
-	cpus_read_lock();
-	cpuset_lock();
-	if (!is_cpuset_online(cs)) {
-		retval = -ENODEV;
-		goto out_unlock;
-	}
+	guard_cpus_read_and_cpuset();
+	if (!is_cpuset_online(cs))
+		return -ENODEV;
 
 	switch (type) {
 	case FILE_CPU_EXCLUSIVE:
@@ -497,9 +491,7 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 		retval = -EINVAL;
 		break;
 	}
-out_unlock:
-	cpuset_unlock();
-	cpus_read_unlock();
+
 	return retval;
 }
 
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index f6cdb5cdffe8..110d2b93ff96 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -3235,15 +3235,14 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
 	int retval = -ENODEV;
 
 	buf = strstrip(buf);
-	cpus_read_lock();
-	mutex_lock(&cpuset_mutex);
+	guard_cpus_read_and_cpuset();
 	if (!is_cpuset_online(cs))
-		goto out_unlock;
+		goto out;
 
 	trialcs = alloc_trial_cpuset(cs);
 	if (!trialcs) {
 		retval = -ENOMEM;
-		goto out_unlock;
+		goto out;
 	}
 
 	switch (of_cft(of)->private) {
@@ -3264,9 +3263,7 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
 	free_cpuset(trialcs);
 	if (force_sd_rebuild)
 		rebuild_sched_domains_locked();
-out_unlock:
-	mutex_unlock(&cpuset_mutex);
-	cpus_read_unlock();
+out:
 	flush_workqueue(cpuset_migrate_mm_wq);
 	return retval ?: nbytes;
 }
@@ -3370,12 +3367,9 @@ static ssize_t cpuset_partition_write(struct kernfs_open_file *of, char *buf,
 		return -EINVAL;
 
 	css_get(&cs->css);
-	cpus_read_lock();
-	mutex_lock(&cpuset_mutex);
+	guard_cpus_read_and_cpuset();
 	if (is_cpuset_online(cs))
 		retval = update_prstate(cs, val);
-	mutex_unlock(&cpuset_mutex);
-	cpus_read_unlock();
 	css_put(&cs->css);
 	return retval ?: nbytes;
 }
@@ -3506,8 +3500,7 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 	if (!parent)
 		return 0;
 
-	cpus_read_lock();
-	mutex_lock(&cpuset_mutex);
+	guard_cpus_read_and_cpuset();
 
 	if (is_spread_page(parent))
 		set_bit(CS_SPREAD_PAGE, &cs->flags);
@@ -3529,7 +3522,7 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 	spin_unlock_irq(&callback_lock);
 
 	if (!test_bit(CGRP_CPUSET_CLONE_CHILDREN, &css->cgroup->flags))
-		goto out_unlock;
+		return 0;
 
 	/*
 	 * Clone @parent's configuration if CGRP_CPUSET_CLONE_CHILDREN is
@@ -3548,7 +3541,7 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 	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;
+			return 0;
 		}
 	}
 	rcu_read_unlock();
@@ -3559,9 +3552,6 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 	cpumask_copy(cs->cpus_allowed, parent->cpus_allowed);
 	cpumask_copy(cs->effective_cpus, parent->cpus_allowed);
 	spin_unlock_irq(&callback_lock);
-out_unlock:
-	mutex_unlock(&cpuset_mutex);
-	cpus_read_unlock();
 	return 0;
 }
 
@@ -3576,16 +3566,12 @@ static void cpuset_css_offline(struct cgroup_subsys_state *css)
 {
 	struct cpuset *cs = css_cs(css);
 
-	cpus_read_lock();
-	mutex_lock(&cpuset_mutex);
+	guard_cpus_read_and_cpuset();
 
 	if (!cpuset_v2() && is_sched_load_balance(cs))
 		cpuset_update_flag(CS_SCHED_LOAD_BALANCE, cs, 0);
 
 	cpuset_dec();
-
-	mutex_unlock(&cpuset_mutex);
-	cpus_read_unlock();
 }
 
 /*
@@ -3597,16 +3583,11 @@ static void cpuset_css_killed(struct cgroup_subsys_state *css)
 {
 	struct cpuset *cs = css_cs(css);
 
-	cpus_read_lock();
-	mutex_lock(&cpuset_mutex);
+	guard_cpus_read_and_cpuset();
 
 	/* Reset valid partition back to member */
 	if (is_partition_valid(cs))
 		update_prstate(cs, PRS_MEMBER);
-
-	mutex_unlock(&cpuset_mutex);
-	cpus_read_unlock();
-
 }
 
 static void cpuset_css_free(struct cgroup_subsys_state *css)
-- 
2.34.1

[PATCH -next 4/4] cpuset: replace cpuset_lock() with guard_cpuset()

[Chen Ridong]

From: Chen Ridong <chenridong@huawei.com>

The guard_cpuset() is introduced, we can replace all cpuset_lock()
usage with it and remove the cpuset_lock().

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 include/linux/cpuset.h  |  2 --
 kernel/cgroup/cpuset.c  | 10 ----------
 kernel/sched/syscalls.c | 15 +++++----------
 3 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
index 6153de28acf0..1baf12f4be19 100644
--- a/include/linux/cpuset.h
+++ b/include/linux/cpuset.h
@@ -72,8 +72,6 @@ extern void cpuset_force_rebuild(void);
 extern void cpuset_update_active_cpus(void);
 extern void inc_dl_tasks_cs(struct task_struct *task);
 extern void dec_dl_tasks_cs(struct task_struct *task);
-extern void cpuset_lock(void);
-extern void cpuset_unlock(void);
 extern void guard_cpuset(void);
 extern void cpuset_cpus_allowed(struct task_struct *p, struct cpumask *mask);
 extern bool cpuset_cpus_allowed_fallback(struct task_struct *p);
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 110d2b93ff96..04ed73d0887e 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -250,16 +250,6 @@ static struct cpuset top_cpuset = {
 
 static DEFINE_MUTEX(cpuset_mutex);
 
-void cpuset_lock(void)
-{
-	mutex_lock(&cpuset_mutex);
-}
-
-void cpuset_unlock(void)
-{
-	mutex_unlock(&cpuset_mutex);
-}
-
 void guard_cpuset(void)
 {
 	guard(mutex)(&cpuset_mutex);
diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 77ae87f36e84..954f6e9af41b 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -577,8 +577,10 @@ int __sched_setscheduler(struct task_struct *p,
 	 * information.
 	 */
 	if (dl_policy(policy) || dl_policy(p->policy)) {
-		cpuset_locked = true;
-		cpuset_lock();
+		if (!cpuset_locked) {
+			guard_cpuset();
+			cpuset_locked = true;
+		}
 	}
 
 	/*
@@ -660,8 +662,6 @@ int __sched_setscheduler(struct task_struct *p,
 	if (unlikely(oldpolicy != -1 && oldpolicy != p->policy)) {
 		policy = oldpolicy = -1;
 		task_rq_unlock(rq, p, &rf);
-		if (cpuset_locked)
-			cpuset_unlock();
 		goto recheck;
 	}
 
@@ -733,11 +733,8 @@ int __sched_setscheduler(struct task_struct *p,
 	head = splice_balance_callbacks(rq);
 	task_rq_unlock(rq, p, &rf);
 
-	if (pi) {
-		if (cpuset_locked)
-			cpuset_unlock();
+	if (pi)
 		rt_mutex_adjust_pi(p);
-	}
 
 	/* Run balance callbacks after we've adjusted the PI chain: */
 	balance_callbacks(rq, head);
@@ -747,8 +744,6 @@ int __sched_setscheduler(struct task_struct *p,
 
 unlock:
 	task_rq_unlock(rq, p, &rf);
-	if (cpuset_locked)
-		cpuset_unlock();
 	return retval;
 }
 
-- 
2.34.1

Re: [PATCH -next 4/4] cpuset: replace cpuset_lock() with guard_cpuset()

[kernel test robot]

Hi Chen,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20250808]

url:    https://github.com/intel-lab-lkp/linux/commits/Chen-Ridong/cpuset-remove-redundant-CS_ONLINE-flag/20250808-174245
base:   next-20250808
patch link:    https://lore.kernel.org/r/20250808092515.764820-5-chenridong%40huaweicloud.com
patch subject: [PATCH -next 4/4] cpuset: replace cpuset_lock() with guard_cpuset()
config: x86_64-buildonly-randconfig-001-20250809 (https://download.01.org/0day-ci/archive/20250809/202508090557.XjdGVjX4-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250809/202508090557.XjdGVjX4-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508090557.XjdGVjX4-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from kernel/sched/build_policy.c:65:
   kernel/sched/syscalls.c: In function '__sched_setscheduler':
>> kernel/sched/syscalls.c:581:25: error: implicit declaration of function 'guard_cpuset' [-Werror=implicit-function-declaration]
     581 |                         guard_cpuset();
         |                         ^~~~~~~~~~~~
   In file included from kernel/sched/build_policy.c:52:
   kernel/sched/rt.c: At top level:
   kernel/sched/rt.c:12:18: warning: 'max_rt_runtime' defined but not used [-Wunused-const-variable=]
      12 | static const u64 max_rt_runtime = MAX_BW;
         |                  ^~~~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/guard_cpuset +581 kernel/sched/syscalls.c

   512	
   513	int __sched_setscheduler(struct task_struct *p,
   514				 const struct sched_attr *attr,
   515				 bool user, bool pi)
   516	{
   517		int oldpolicy = -1, policy = attr->sched_policy;
   518		int retval, oldprio, newprio, queued, running;
   519		const struct sched_class *prev_class, *next_class;
   520		struct balance_callback *head;
   521		struct rq_flags rf;
   522		int reset_on_fork;
   523		int queue_flags = DEQUEUE_SAVE | DEQUEUE_MOVE | DEQUEUE_NOCLOCK;
   524		struct rq *rq;
   525		bool cpuset_locked = false;
   526	
   527		/* The pi code expects interrupts enabled */
   528		BUG_ON(pi && in_interrupt());
   529	recheck:
   530		/* Double check policy once rq lock held: */
   531		if (policy < 0) {
   532			reset_on_fork = p->sched_reset_on_fork;
   533			policy = oldpolicy = p->policy;
   534		} else {
   535			reset_on_fork = !!(attr->sched_flags & SCHED_FLAG_RESET_ON_FORK);
   536	
   537			if (!valid_policy(policy))
   538				return -EINVAL;
   539		}
   540	
   541		if (attr->sched_flags & ~(SCHED_FLAG_ALL | SCHED_FLAG_SUGOV))
   542			return -EINVAL;
   543	
   544		/*
   545		 * Valid priorities for SCHED_FIFO and SCHED_RR are
   546		 * 1..MAX_RT_PRIO-1, valid priority for SCHED_NORMAL,
   547		 * SCHED_BATCH and SCHED_IDLE is 0.
   548		 */
   549		if (attr->sched_priority > MAX_RT_PRIO-1)
   550			return -EINVAL;
   551		if ((dl_policy(policy) && !__checkparam_dl(attr)) ||
   552		    (rt_policy(policy) != (attr->sched_priority != 0)))
   553			return -EINVAL;
   554	
   555		if (user) {
   556			retval = user_check_sched_setscheduler(p, attr, policy, reset_on_fork);
   557			if (retval)
   558				return retval;
   559	
   560			if (attr->sched_flags & SCHED_FLAG_SUGOV)
   561				return -EINVAL;
   562	
   563			retval = security_task_setscheduler(p);
   564			if (retval)
   565				return retval;
   566		}
   567	
   568		/* Update task specific "requested" clamps */
   569		if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) {
   570			retval = uclamp_validate(p, attr);
   571			if (retval)
   572				return retval;
   573		}
   574	
   575		/*
   576		 * SCHED_DEADLINE bandwidth accounting relies on stable cpusets
   577		 * information.
   578		 */
   579		if (dl_policy(policy) || dl_policy(p->policy)) {
   580			if (!cpuset_locked) {
 > 581				guard_cpuset();
   582				cpuset_locked = true;
   583			}
   584		}
   585	
   586		/*
   587		 * Make sure no PI-waiters arrive (or leave) while we are
   588		 * changing the priority of the task:
   589		 *
   590		 * To be able to change p->policy safely, the appropriate
   591		 * runqueue lock must be held.
   592		 */
   593		rq = task_rq_lock(p, &rf);
   594		update_rq_clock(rq);
   595	
   596		/*
   597		 * Changing the policy of the stop threads its a very bad idea:
   598		 */
   599		if (p == rq->stop) {
   600			retval = -EINVAL;
   601			goto unlock;
   602		}
   603	
   604		retval = scx_check_setscheduler(p, policy);
   605		if (retval)
   606			goto unlock;
   607	
   608		/*
   609		 * If not changing anything there's no need to proceed further,
   610		 * but store a possible modification of reset_on_fork.
   611		 */
   612		if (unlikely(policy == p->policy)) {
   613			if (fair_policy(policy) &&
   614			    (attr->sched_nice != task_nice(p) ||
   615			     (attr->sched_runtime != p->se.slice)))
   616				goto change;
   617			if (rt_policy(policy) && attr->sched_priority != p->rt_priority)
   618				goto change;
   619			if (dl_policy(policy) && dl_param_changed(p, attr))
   620				goto change;
   621			if (attr->sched_flags & SCHED_FLAG_UTIL_CLAMP)
   622				goto change;
   623	
   624			p->sched_reset_on_fork = reset_on_fork;
   625			retval = 0;
   626			goto unlock;
   627		}
   628	change:
   629	
   630		if (user) {
   631	#ifdef CONFIG_RT_GROUP_SCHED
   632			/*
   633			 * Do not allow real-time tasks into groups that have no runtime
   634			 * assigned.
   635			 */
   636			if (rt_group_sched_enabled() &&
   637					rt_bandwidth_enabled() && rt_policy(policy) &&
   638					task_group(p)->rt_bandwidth.rt_runtime == 0 &&
   639					!task_group_is_autogroup(task_group(p))) {
   640				retval = -EPERM;
   641				goto unlock;
   642			}
   643	#endif /* CONFIG_RT_GROUP_SCHED */
   644			if (dl_bandwidth_enabled() && dl_policy(policy) &&
   645					!(attr->sched_flags & SCHED_FLAG_SUGOV)) {
   646				cpumask_t *span = rq->rd->span;
   647	
   648				/*
   649				 * Don't allow tasks with an affinity mask smaller than
   650				 * the entire root_domain to become SCHED_DEADLINE. We
   651				 * will also fail if there's no bandwidth available.
   652				 */
   653				if (!cpumask_subset(span, p->cpus_ptr) ||
   654				    rq->rd->dl_bw.bw == 0) {
   655					retval = -EPERM;
   656					goto unlock;
   657				}
   658			}
   659		}
   660	
   661		/* Re-check policy now with rq lock held: */
   662		if (unlikely(oldpolicy != -1 && oldpolicy != p->policy)) {
   663			policy = oldpolicy = -1;
   664			task_rq_unlock(rq, p, &rf);
   665			goto recheck;
   666		}
   667	
   668		/*
   669		 * If setscheduling to SCHED_DEADLINE (or changing the parameters
   670		 * of a SCHED_DEADLINE task) we need to check if enough bandwidth
   671		 * is available.
   672		 */
   673		if ((dl_policy(policy) || dl_task(p)) && sched_dl_overflow(p, policy, attr)) {
   674			retval = -EBUSY;
   675			goto unlock;
   676		}
   677	
   678		p->sched_reset_on_fork = reset_on_fork;
   679		oldprio = p->prio;
   680	
   681		newprio = __normal_prio(policy, attr->sched_priority, attr->sched_nice);
   682		if (pi) {
   683			/*
   684			 * Take priority boosted tasks into account. If the new
   685			 * effective priority is unchanged, we just store the new
   686			 * normal parameters and do not touch the scheduler class and
   687			 * the runqueue. This will be done when the task deboost
   688			 * itself.
   689			 */
   690			newprio = rt_effective_prio(p, newprio);
   691			if (newprio == oldprio)
   692				queue_flags &= ~DEQUEUE_MOVE;
   693		}
   694	
   695		prev_class = p->sched_class;
   696		next_class = __setscheduler_class(policy, newprio);
   697	
   698		if (prev_class != next_class && p->se.sched_delayed)
   699			dequeue_task(rq, p, DEQUEUE_SLEEP | DEQUEUE_DELAYED | DEQUEUE_NOCLOCK);
   700	
   701		queued = task_on_rq_queued(p);
   702		running = task_current_donor(rq, p);
   703		if (queued)
   704			dequeue_task(rq, p, queue_flags);
   705		if (running)
   706			put_prev_task(rq, p);
   707	
   708		if (!(attr->sched_flags & SCHED_FLAG_KEEP_PARAMS)) {
   709			__setscheduler_params(p, attr);
   710			p->sched_class = next_class;
   711			p->prio = newprio;
   712		}
   713		__setscheduler_uclamp(p, attr);
   714		check_class_changing(rq, p, prev_class);
   715	
   716		if (queued) {
   717			/*
   718			 * We enqueue to tail when the priority of a task is
   719			 * increased (user space view).
   720			 */
   721			if (oldprio < p->prio)
   722				queue_flags |= ENQUEUE_HEAD;
   723	
   724			enqueue_task(rq, p, queue_flags);
   725		}
   726		if (running)
   727			set_next_task(rq, p);
   728	
   729		check_class_changed(rq, p, prev_class, oldprio);
   730	
   731		/* Avoid rq from going away on us: */
   732		preempt_disable();
   733		head = splice_balance_callbacks(rq);
   734		task_rq_unlock(rq, p, &rf);
   735	
   736		if (pi)
   737			rt_mutex_adjust_pi(p);
   738	
   739		/* Run balance callbacks after we've adjusted the PI chain: */
   740		balance_callbacks(rq, head);
   741		preempt_enable();
   742	
   743		return 0;
   744	
   745	unlock:
   746		task_rq_unlock(rq, p, &rf);
   747		return retval;
   748	}
   749	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH -next 2/4] cpuset: add helpers for cpuset related locks

[Christophe JAILLET]

Le 08/08/2025 à 11:25, Chen Ridong a écrit :
> From: Chen Ridong <chenridong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> 
> Add guard_cpus_read_and_cpuset and guard_cpuset helpers for cpuset, which
> will be user for subsequent patched to make code concise;
> 
> Signed-off-by: Chen Ridong <chenridong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>   include/linux/cpuset.h          |  1 +
>   kernel/cgroup/cpuset-internal.h |  2 ++
>   kernel/cgroup/cpuset.c          | 11 +++++++++++
>   3 files changed, 14 insertions(+)
> 
> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
> index 2ddb256187b5..6153de28acf0 100644
> --- a/include/linux/cpuset.h
> +++ b/include/linux/cpuset.h
> @@ -74,6 +74,7 @@ extern void inc_dl_tasks_cs(struct task_struct *task);
>   extern void dec_dl_tasks_cs(struct task_struct *task);
>   extern void cpuset_lock(void);
>   extern void cpuset_unlock(void);
> +extern void guard_cpuset(void);
>   extern void cpuset_cpus_allowed(struct task_struct *p, struct cpumask *mask);
>   extern bool cpuset_cpus_allowed_fallback(struct task_struct *p);
>   extern bool cpuset_cpu_is_isolated(int cpu);
> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
> index 75b3aef39231..084e19fe33d5 100644
> --- a/kernel/cgroup/cpuset-internal.h
> +++ b/kernel/cgroup/cpuset-internal.h
> @@ -277,6 +277,8 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>   				    char *buf, size_t nbytes, loff_t off);
>   int cpuset_common_seq_show(struct seq_file *sf, void *v);
>   
> +void guard_cpus_read_and_cpuset(void);
> +
>   /*
>    * cpuset-v1.c
>    */
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index cf7cd2255265..f6cdb5cdffe8 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -260,6 +260,17 @@ void cpuset_unlock(void)
>   	mutex_unlock(&cpuset_mutex);
>   }
>   
> +void guard_cpuset(void)
> +{
> +	guard(mutex)(&cpuset_mutex);
> +}
> +
> +void guard_cpus_read_and_cpuset(void)
> +{
> +	guard(cpus_read_lock)();
> +	guard(mutex)(&cpuset_mutex);
> +}
> +

Not sure that it works like that.

I think that these 2 functions are just no-op because whatever is 
"garded", it will be release when the function exits.

So, if correct, all this serie does is removing some existing 
synchronisation mechanism.

Do I miss something obvious?

CJ


>   static DEFINE_SPINLOCK(callback_lock);
>   
>   void cpuset_callback_lock_irq(void)

Re: [PATCH -next 2/4] cpuset: add helpers for cpuset related locks

[Chen Ridong]

On 2025/8/9 23:56, Christophe JAILLET wrote:
> Le 08/08/2025 à 11:25, Chen Ridong a écrit :
>> From: Chen Ridong <chenridong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>
>> Add guard_cpus_read_and_cpuset and guard_cpuset helpers for cpuset, which
>> will be user for subsequent patched to make code concise;
>>
>> Signed-off-by: Chen Ridong <chenridong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>   include/linux/cpuset.h          |  1 +
>>   kernel/cgroup/cpuset-internal.h |  2 ++
>>   kernel/cgroup/cpuset.c          | 11 +++++++++++
>>   3 files changed, 14 insertions(+)
>>
>> diff --git a/include/linux/cpuset.h b/include/linux/cpuset.h
>> index 2ddb256187b5..6153de28acf0 100644
>> --- a/include/linux/cpuset.h
>> +++ b/include/linux/cpuset.h
>> @@ -74,6 +74,7 @@ extern void inc_dl_tasks_cs(struct task_struct *task);
>>   extern void dec_dl_tasks_cs(struct task_struct *task);
>>   extern void cpuset_lock(void);
>>   extern void cpuset_unlock(void);
>> +extern void guard_cpuset(void);
>>   extern void cpuset_cpus_allowed(struct task_struct *p, struct cpumask *mask);
>>   extern bool cpuset_cpus_allowed_fallback(struct task_struct *p);
>>   extern bool cpuset_cpu_is_isolated(int cpu);
>> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
>> index 75b3aef39231..084e19fe33d5 100644
>> --- a/kernel/cgroup/cpuset-internal.h
>> +++ b/kernel/cgroup/cpuset-internal.h
>> @@ -277,6 +277,8 @@ ssize_t cpuset_write_resmask(struct kernfs_open_file *of,
>>                       char *buf, size_t nbytes, loff_t off);
>>   int cpuset_common_seq_show(struct seq_file *sf, void *v);
>>   +void guard_cpus_read_and_cpuset(void);
>> +
>>   /*
>>    * cpuset-v1.c
>>    */
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index cf7cd2255265..f6cdb5cdffe8 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -260,6 +260,17 @@ void cpuset_unlock(void)
>>       mutex_unlock(&cpuset_mutex);
>>   }
>>   +void guard_cpuset(void)
>> +{
>> +    guard(mutex)(&cpuset_mutex);
>> +}
>> +
>> +void guard_cpus_read_and_cpuset(void)
>> +{
>> +    guard(cpus_read_lock)();
>> +    guard(mutex)(&cpuset_mutex);
>> +}
>> +
> 
> Not sure that it works like that.
> 
> I think that these 2 functions are just no-op because whatever is "garded", it will be release when
> the function exits.
> 
> So, if correct, all this serie does is removing some existing synchronisation mechanism.
> 
> Do I miss something obvious?
> 
> CJ
> 

Thank you for catching this issue. You're absolutely right - I made a critical mistake in the guard
function implementation.

After further investigation, I realized that when I ran the self-tests with CONFIG_LOCKDEP disabled,
I missed the lock protection warnings that would have revealed this problem earlier. Had Lockdep
been enabled, it would have immediately flagged the incorrect locking behavior.

Please ignore this patch series, Thanks.

Best regards,
Ridong

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

[Waiman Long]

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

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

[Chen Ridong]

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

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

[Waiman Long]

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

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

[Chen Ridong]

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

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

[Waiman Long]

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>

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

[Chen Ridong]

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