mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH -next RFC 00/16] cpuset: rework local partition logic
@ 2025-09-28  7:12 Chen Ridong
  2025-09-28  7:12 ` [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier Chen Ridong
                   ` (17 more replies)
  0 siblings, 18 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

The current local partition implementation consolidates all operations
(enable, disable, invalidate, and update) within the large
update_parent_effective_cpumask() function, which exceeds 300 lines.
This monolithic approach has become increasingly difficult to understand
and maintain. Additionally, partition-related fields are updated in
multiple locations, leading to redundant code and potential corner case
oversights.

This patch series refactors the local partition logic by separating
operations into dedicated functions: local_partition_enable(),
local_partition_disable(), and local_partition_update(), creating
symmetry with the existing remote partition infrastructure.

The series is organized as follows:

1. Infrastructure Preparation (Patches 1-2):
   - Code cleanup and preparation for the refactoring work

2. Core Partition Operations (Patches 3-5):
   - Factor out partition_enable(), partition_disable(), and
     partition_update() functions from remote partition operations

3. Local Partition Implementation (Patches 6-9):
   - Separate update_parent_effective_cpumask() into dedicated functions:
     * local_partition_enable()
     * local_partition_disable()
     * local_partition_invalidate()
     * local_partition_update()

4. Optimization and Cleanup (Patches 10-16):
   - Remove redundant partition-related operations
   - Additional optimizations based on the new architecture

Key improvements:
- Centralized management of partition-related fields (partition_root_state,
  prs_err, nr_subparts, remote_sibling, effective_xcpus) within the
  partition_enable/disable/update functions
- Consistent operation patterns for both local and remote partitions
  with type-specific validation checks
- Fixed bug where isolcpus remained in root partition after isolated
  partition transitioned to root

Chen Ridong (16):
  cpuset: use update_partition_sd_lb in update_cpumasks_hier
  cpuset: generalize validate_partition() interface
  cpuset: factor out partition_enable() function
  cpuset: factor out partition_disable() function
  cpuset: factor out partition_update() function
  cpuset: introduce local_partition_enable()
  cpuset: introduce local_partition_disable()
  cpuset: introduce local_partition_invalidate()
  cpuset: introduce local_partition_update()
  cpuset: remove redundant partition field updates
  cpuset: simplify partition update logic for hotplug tasks
  cpuset: unify local partition disable and invalidate
  cpuset: use partition_disable for compute_partition_effective_cpumask
  cpuset: fix isolcpus stay in root when isolated partition changes to
    root
  cpuset: use partition_disable for update_prstate
  cpuset: remove prs_err clear when notify_partition_change

 kernel/cgroup/cpuset.c | 907 ++++++++++++++++++-----------------------
 1 file changed, 408 insertions(+), 499 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-10-20  2:37   ` Waiman Long
  2025-09-28  7:12 ` [PATCH -next RFC 02/16] cpuset: generalize validate_partition() interface Chen Ridong
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

For cgroup v2, when a cpuset is not a valid partition root, it inherits
the CS_SCHED_LOAD_BALANCE flag from its parent. The existing logic in
update_cpumasks_hier() manually handled this inheritance condition.

This patch replaces the inline implementation with a call to the dedicated
update_partition_sd_lb() helper function, which already encapsulates the
same logic. The helper function comprehensively handles both the load
balance flag update and the necessary scheduling domain rebuild decision.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 52468d2c178a..052f9e0c7a65 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1257,6 +1257,8 @@ static void update_partition_sd_lb(struct cpuset *cs, int old_prs)
 	bool rebuild_domains = (new_prs > 0) || (old_prs > 0);
 	bool new_lb;
 
+	if (!cpuset_v2())
+		return;
 	/*
 	 * If cs is not a valid partition root, the load balance state
 	 * will follow its parent.
@@ -2276,19 +2278,7 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
 			!cpumask_equal(cp->cpus_allowed, cp->effective_cpus));
 
 		cpuset_update_tasks_cpumask(cp, cp->effective_cpus);
-
-		/*
-		 * On default hierarchy, inherit the CS_SCHED_LOAD_BALANCE
-		 * from parent if current cpuset isn't a valid partition root
-		 * and their load balance states differ.
-		 */
-		if (cpuset_v2() && !is_partition_valid(cp) &&
-		    (is_sched_load_balance(parent) != is_sched_load_balance(cp))) {
-			if (is_sched_load_balance(parent))
-				set_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
-			else
-				clear_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
-		}
+		update_partition_sd_lb(cp, old_prs);
 
 		/*
 		 * On legacy hierarchy, if the effective cpumask of any non-
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 02/16] cpuset: generalize validate_partition() interface
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
  2025-09-28  7:12 ` [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-09-28  7:12 ` [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function Chen Ridong
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Refactor validate_partition() to accept a more generic parameter set,
making the interface flexible enough to handle local partition enablement
validation scenarios. This prepares the function for broader use cases
beyond its current validation scope while maintaining backward
compatibility with existing callers.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 77 ++++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 44 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 052f9e0c7a65..0787904321a9 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1702,6 +1702,34 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
 	return false;
 }
 
+/**
+ * validate_partition - Validate a cpuset partition configuration
+ * @cs: The cpuset to validate
+ * @new_prs: The proposed new partition root state
+ * @new_excpus: The new effective exclusize CPUs mask to validate
+ *
+ * Return: PRS error code (0 if valid, non-zero error code if invalid)
+ */
+static enum prs_errcode validate_partition(struct cpuset *cs, int new_prs,
+						 struct cpumask *new_excpus)
+{
+	struct cpuset *parent = parent_cs(cs);
+
+	if (new_prs == PRS_MEMBER)
+		return PERR_NONE;
+
+	if (cpumask_empty(new_excpus))
+		return PERR_INVCPUS;
+
+	if (prstate_housekeeping_conflict(new_prs, new_excpus))
+		return PERR_HKEEPING;
+
+	if (tasks_nocpu_error(parent, cs, new_excpus))
+		return PERR_NOCPUS;
+
+	return PERR_NONE;
+}
+
 /**
  * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
  * @cs:      The cpuset that requests change in partition root state
@@ -1805,19 +1833,9 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 			WARN_ON_ONCE(!cpumask_empty(cs->exclusive_cpus));
 		new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
 
-		/*
-		 * Enabling partition root is not allowed if its
-		 * effective_xcpus is empty.
-		 */
-		if (cpumask_empty(xcpus))
-			return PERR_INVCPUS;
-
-		if (prstate_housekeeping_conflict(new_prs, xcpus))
-			return PERR_HKEEPING;
-
-		if (tasks_nocpu_error(parent, cs, xcpus))
-			return PERR_NOCPUS;
-
+		part_error = validate_partition(cs, new_prs, xcpus);
+		if (part_error)
+			return part_error;
 		/*
 		 * This function will only be called when all the preliminary
 		 * checks have passed. At this point, the following condition
@@ -2367,36 +2385,6 @@ static int parse_cpuset_cpulist(const char *buf, struct cpumask *out_mask)
 	return 0;
 }
 
-/**
- * validate_partition - Validate a cpuset partition configuration
- * @cs: The cpuset to validate
- * @trialcs: The trial cpuset containing proposed configuration changes
- *
- * If any validation check fails, the appropriate error code is set in the
- * cpuset's prs_err field.
- *
- * Return: PRS error code (0 if valid, non-zero error code if invalid)
- */
-static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *trialcs)
-{
-	struct cpuset *parent = parent_cs(cs);
-
-	if (cs_is_member(trialcs))
-		return PERR_NONE;
-
-	if (cpumask_empty(trialcs->effective_xcpus))
-		return PERR_INVCPUS;
-
-	if (prstate_housekeeping_conflict(trialcs->partition_root_state,
-					  trialcs->effective_xcpus))
-		return PERR_HKEEPING;
-
-	if (tasks_nocpu_error(parent, cs, trialcs->effective_xcpus))
-		return PERR_NOCPUS;
-
-	return PERR_NONE;
-}
-
 static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset *trialcs,
 					struct tmpmasks *tmp)
 {
@@ -2451,7 +2439,8 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
 	if (cs_is_member(cs))
 		return;
 
-	prs_err = validate_partition(cs, trialcs);
+	prs_err = validate_partition(cs, trialcs->partition_root_state,
+				     trialcs->effective_xcpus);
 	if (prs_err)
 		trialcs->prs_err = cs->prs_err = prs_err;
 
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
  2025-09-28  7:12 ` [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier Chen Ridong
  2025-09-28  7:12 ` [PATCH -next RFC 02/16] cpuset: generalize validate_partition() interface Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-10-20  2:39   ` Waiman Long
  2025-09-28  7:12 ` [PATCH -next RFC 04/16] cpuset: factor out partition_disable() function Chen Ridong
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Extract the core partition enablement logic into a dedicated
partition_enable() function. This refactoring centralizes updates to key
cpuset data structures including remote_sibling, effective_xcpus,
partition_root_state, and prs_err.

The function handles the complete partition enablement workflow:
- Adding exclusive CPUs via partition_xcpus_add()
- Managing remote sibling relationships
- Synchronizing effective exclusive CPUs mask
- Updating partition state and error status
- Triggering required scheduler domain rebuilds

This creates a coherent interface for partition operations and establishes
a foundation for future local partition support while maintaining existing
remote partition behavior.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 55 +++++++++++++++++++++++++++++++++---------
 1 file changed, 44 insertions(+), 11 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0787904321a9..43ce62f4959c 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1515,6 +1515,49 @@ static inline bool is_local_partition(struct cpuset *cs)
 	return is_partition_valid(cs) && !is_remote_partition(cs);
 }
 
+static void partition_state_update(struct cpuset *cs, int new_prs,
+					  enum prs_errcode prs_err)
+{
+	lockdep_assert_held(&callback_lock);
+
+	cs->partition_root_state = new_prs;
+	WRITE_ONCE(cs->prs_err, prs_err);
+	if (!is_partition_valid(cs))
+		reset_partition_data(cs);
+}
+
+/**
+ * partition_enable - Transitions a cpuset to a partition root
+ * @cs: The cpuset to enable partition for
+ * @parent: Parent cpuset of @cs, NULL for remote parent
+ * @new_prs: New partition root state to set
+ * @new_excpus: New exclusive CPUs mask for the partition
+ *
+ * Transitions a cpuset to a partition root, only for v2.
+ */
+static void partition_enable(struct cpuset *cs, struct cpuset *parent,
+				 int new_prs, struct cpumask *new_excpus)
+{
+	bool isolcpus_updated;
+
+	lockdep_assert_held(&cpuset_mutex);
+	WARN_ON_ONCE(new_prs <= 0);
+	WARN_ON_ONCE(!cpuset_v2());
+
+	if (cs->partition_root_state == new_prs)
+		return;
+
+	spin_lock_irq(&callback_lock);
+	/* enable partition should only add exclusive cpus */
+	isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
+	list_add(&cs->remote_sibling, &remote_children);
+	cpumask_copy(cs->effective_xcpus, new_excpus);
+	partition_state_update(cs, new_prs, PERR_NONE);
+	spin_unlock_irq(&callback_lock);
+	update_unbound_workqueue_cpumask(isolcpus_updated);
+	cpuset_force_rebuild();
+}
+
 /*
  * remote_partition_enable - Enable current cpuset as a remote partition root
  * @cs: the cpuset to update
@@ -1528,8 +1571,6 @@ static inline bool is_local_partition(struct cpuset *cs)
 static int remote_partition_enable(struct cpuset *cs, int new_prs,
 				   struct tmpmasks *tmp)
 {
-	bool isolcpus_updated;
-
 	/*
 	 * The user must have sysadmin privilege.
 	 */
@@ -1552,15 +1593,7 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
 	    cpumask_subset(top_cpuset.effective_cpus, tmp->new_cpus))
 		return PERR_INVCPUS;
 
-	spin_lock_irq(&callback_lock);
-	isolcpus_updated = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
-	list_add(&cs->remote_sibling, &remote_children);
-	cpumask_copy(cs->effective_xcpus, tmp->new_cpus);
-	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
-	cpuset_force_rebuild();
-	cs->prs_err = 0;
-
+	partition_enable(cs, NULL, new_prs, tmp->new_cpus);
 	/*
 	 * Propagate changes in top_cpuset's effective_cpus down the hierarchy.
 	 */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 04/16] cpuset: factor out partition_disable() function
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (2 preceding siblings ...)
  2025-09-28  7:12 ` [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-09-28  7:12 ` [PATCH -next RFC 05/16] cpuset: factor out partition_update() function Chen Ridong
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Extract the core partition disablement logic into a dedicated
partition_disable() function. This refactoring centralizes updates to key
cpuset data structures including remote_sibling, effective_xcpus,
partition_root_state, and prs_err.

The function handles the complete partition disablement workflow:
- Removing exclusive CPUs via partition_xcpus_del()
- Cleaning up remote sibling relationships
- Recomputing effective exclusive CPUs mask
- Updating partition state and error status
- Triggering required scheduler domain rebuilds

This creates a symmetric interface with partition_enable() and establishes
a foundation for future local partition support while maintaining existing
remote partition behavior.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 49 +++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 17 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 43ce62f4959c..1944410ae872 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1558,6 +1558,35 @@ static void partition_enable(struct cpuset *cs, struct cpuset *parent,
 	cpuset_force_rebuild();
 }
 
+/**
+ * partition_disable - Disable partition root state for a cpuset
+ * @cs: The cpuset to disable partition for
+ * @parent: Parent cpuset of @cs, NULL for remote parent
+ * @new_prs: New partition root state (should be non-positive)
+ * @prs_err: Error code to set if disabling due to validation failure
+ */
+static void partition_disable(struct cpuset *cs, struct cpuset *parent,
+				    int new_prs, enum prs_errcode prs_err)
+{
+	bool isolcpus_updated;
+
+	lockdep_assert_held(&cpuset_mutex);
+	WARN_ON_ONCE(new_prs > 0);
+	WARN_ON_ONCE(!cpuset_v2());
+
+	spin_lock_irq(&callback_lock);
+	list_del_init(&cs->remote_sibling);
+	/* disable a partition should only delete exclusive cpus */
+	isolcpus_updated = partition_xcpus_del(cs->partition_root_state,
+						parent, cs->effective_xcpus);
+	/* effective_xcpus may need to be changed */
+	compute_excpus(cs, cs->effective_xcpus);
+	partition_state_update(cs, new_prs, prs_err);
+	spin_unlock_irq(&callback_lock);
+	update_unbound_workqueue_cpumask(isolcpus_updated);
+	cpuset_force_rebuild();
+}
+
 /*
  * remote_partition_enable - Enable current cpuset as a remote partition root
  * @cs: the cpuset to update
@@ -1613,27 +1642,13 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
  */
 static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 {
-	bool isolcpus_updated;
+	int new_prs;
 
 	WARN_ON_ONCE(!is_remote_partition(cs));
 	WARN_ON_ONCE(!cpumask_subset(cs->effective_xcpus, subpartitions_cpus));
 
-	spin_lock_irq(&callback_lock);
-	list_del_init(&cs->remote_sibling);
-	isolcpus_updated = partition_xcpus_del(cs->partition_root_state,
-					       NULL, cs->effective_xcpus);
-	if (cs->prs_err)
-		cs->partition_root_state = -cs->partition_root_state;
-	else
-		cs->partition_root_state = PRS_MEMBER;
-
-	/* effective_xcpus may need to be changed */
-	compute_excpus(cs, cs->effective_xcpus);
-	reset_partition_data(cs);
-	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
-	cpuset_force_rebuild();
-
+	new_prs = cs->prs_err ? -cs->partition_root_state : PRS_MEMBER;
+	partition_disable(cs, NULL, new_prs, cs->prs_err);
 	/*
 	 * Propagate changes in top_cpuset's effective_cpus down the hierarchy.
 	 */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 05/16] cpuset: factor out partition_update() function
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (3 preceding siblings ...)
  2025-09-28  7:12 ` [PATCH -next RFC 04/16] cpuset: factor out partition_disable() function Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-10-20  2:43   ` Waiman Long
  2025-09-28  7:12 ` [PATCH -next RFC 06/16] cpuset: introduce local_partition_enable() Chen Ridong
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Extract the core partition update logic into a dedicated partition_update()
function. This refactoring centralizes updates to key cpuset data
structures including remote_sibling, effective_xcpus, partition_root_state,
and prs_err.

The function handles the complete partition update workflow:
- Adding and removing exclusive CPUs via partition_xcpus_add()/del()
- Managing remote sibling relationships
- Synchronizing effective exclusive CPUs mask
- Updating partition state and error status
- Triggering required system updates and workqueue synchronization

This creates a coherent interface for partition operations and establishes
a foundation for enhanced partition management while maintaining existing
remote partition behavior.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 71 ++++++++++++++++++++++++++++--------------
 1 file changed, 47 insertions(+), 24 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 1944410ae872..0e2f95daf459 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1587,6 +1587,49 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
 	cpuset_force_rebuild();
 }
 
+/**
+ * partition_update - Update an existing partition configuration
+ * @cs: The cpuset to update
+ * @prs: Partition root state (must be positive)
+ * @xcpus: New exclusive CPUs mask for the partition (NULL to keep current)
+ * @excpus: New effective exclusive CPUs mask
+ * @tmp: Temporary masks
+ *
+ * Updates partition-related fields. The tmp->addmask is the CPU mask that
+ * will be added to the subpartitions_cpus and removed from parent's
+ * effective_cpus, and the tmp->delmask vice versa.
+ */
+static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
+				  struct cpumask *excpus, struct tmpmasks *tmp)
+{
+	bool isolcpus_updated;
+	bool excl_updated;
+	struct cpuset *parent;
+
+	lockdep_assert_held(&cpuset_mutex);
+	WARN_ON_ONCE(!cpuset_v2());
+	WARN_ON_ONCE(prs <= 0);
+
+	parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
+	excl_updated = !cpumask_empty(tmp->addmask) ||
+			!cpumask_empty(tmp->delmask);
+
+	spin_lock_irq(&callback_lock);
+	isolcpus_updated = partition_xcpus_add(prs, parent, tmp->addmask);
+	isolcpus_updated |= partition_xcpus_del(prs, parent, tmp->delmask);
+	/*
+	 * Need to update effective_xcpus and exclusive_cpus now as
+	 * update_sibling_cpumasks() below may iterate back to the same cs.
+	 */
+	cpumask_copy(cs->effective_xcpus, excpus);
+	if (xcpus)
+		cpumask_copy(cs->exclusive_cpus, xcpus);
+	spin_unlock_irq(&callback_lock);
+	update_unbound_workqueue_cpumask(isolcpus_updated);
+	if (excl_updated)
+		cpuset_force_rebuild();
+}
+
 /*
  * remote_partition_enable - Enable current cpuset as a remote partition root
  * @cs: the cpuset to update
@@ -1669,10 +1712,6 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 			       struct cpumask *excpus, struct tmpmasks *tmp)
 {
-	bool adding, deleting;
-	int prs = cs->partition_root_state;
-	int isolcpus_updated = 0;
-
 	if (WARN_ON_ONCE(!is_remote_partition(cs)))
 		return;
 
@@ -1683,15 +1722,15 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 		goto invalidate;
 	}
 
-	adding   = cpumask_andnot(tmp->addmask, excpus, cs->effective_xcpus);
-	deleting = cpumask_andnot(tmp->delmask, cs->effective_xcpus, excpus);
+	cpumask_andnot(tmp->addmask, excpus, cs->effective_xcpus);
+	cpumask_andnot(tmp->delmask, cs->effective_xcpus, excpus);
 
 	/*
 	 * Additions of remote CPUs is only allowed if those CPUs are
 	 * not allocated to other partitions and there are effective_cpus
 	 * left in the top cpuset.
 	 */
-	if (adding) {
+	if (!cpumask_empty(tmp->addmask)) {
 		WARN_ON_ONCE(cpumask_intersects(tmp->addmask, subpartitions_cpus));
 		if (!capable(CAP_SYS_ADMIN))
 			cs->prs_err = PERR_ACCESS;
@@ -1702,23 +1741,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 			goto invalidate;
 	}
 
-	spin_lock_irq(&callback_lock);
-	if (adding)
-		isolcpus_updated += partition_xcpus_add(prs, NULL, tmp->addmask);
-	if (deleting)
-		isolcpus_updated += partition_xcpus_del(prs, NULL, tmp->delmask);
-	/*
-	 * Need to update effective_xcpus and exclusive_cpus now as
-	 * update_sibling_cpumasks() below may iterate back to the same cs.
-	 */
-	cpumask_copy(cs->effective_xcpus, excpus);
-	if (xcpus)
-		cpumask_copy(cs->exclusive_cpus, xcpus);
-	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
-	if (adding || deleting)
-		cpuset_force_rebuild();
-
+	partition_update(cs, cs->partition_root_state, xcpus, excpus, tmp);
 	/*
 	 * Propagate changes in top_cpuset's effective_cpus down the hierarchy.
 	 */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 06/16] cpuset: introduce local_partition_enable()
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (4 preceding siblings ...)
  2025-09-28  7:12 ` [PATCH -next RFC 05/16] cpuset: factor out partition_update() function Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-10-20  2:44   ` Waiman Long
  2025-09-28  7:12 ` [PATCH -next RFC 07/16] cpuset: introduce local_partition_disable() Chen Ridong
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

The partition_enable() function introduced in the previous patch can be
reused to enable local partitions.

First, partition_enable() was enhanced to support local partition enabling
by properly handling parent's nr_subparts counter and adding notification
operations.

Then, the local_partition_enable() function is introduced, which factors
out the local partition enablement logic from
update_parent_effective_cpumask(). After passing local partition validation
checks, it delegates to partition_enable() to complete the partition setup.

This refactoring creates a clear separation between local and remote
partition operations while maintaining code reuse through the shared
partition_enable() infrastructure.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 111 +++++++++++++++++++++++++++--------------
 1 file changed, 74 insertions(+), 37 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0e2f95daf459..154992cdfe9a 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1539,6 +1539,7 @@ static void partition_enable(struct cpuset *cs, struct cpuset *parent,
 				 int new_prs, struct cpumask *new_excpus)
 {
 	bool isolcpus_updated;
+	int old_prs;
 
 	lockdep_assert_held(&cpuset_mutex);
 	WARN_ON_ONCE(new_prs <= 0);
@@ -1547,15 +1548,21 @@ static void partition_enable(struct cpuset *cs, struct cpuset *parent,
 	if (cs->partition_root_state == new_prs)
 		return;
 
+	old_prs = cs->partition_root_state;
 	spin_lock_irq(&callback_lock);
 	/* enable partition should only add exclusive cpus */
 	isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
-	list_add(&cs->remote_sibling, &remote_children);
+	/* enable remote partition */
+	if (!parent)
+		list_add(&cs->remote_sibling, &remote_children);
+	else if (!is_partition_valid(cs))
+		parent->nr_subparts += 1;
 	cpumask_copy(cs->effective_xcpus, new_excpus);
 	partition_state_update(cs, new_prs, PERR_NONE);
 	spin_unlock_irq(&callback_lock);
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 	cpuset_force_rebuild();
+	notify_partition_change(cs, old_prs);
 }
 
 /**
@@ -1801,6 +1808,68 @@ static enum prs_errcode validate_partition(struct cpuset *cs, int new_prs,
 	return PERR_NONE;
 }
 
+/**
+ * local_partition_enable - Enable local partition for a cpuset
+ * @cs: Target cpuset to become a local partition root
+ * @new_prs: New partition root state to apply
+ * @tmp: Temporary masks for CPU calculations
+ *
+ * This function enables local partition root capability for a cpuset by
+ * validating prerequisites, computing exclusive CPUs, and updating the
+ * partition hierarchy.
+ *
+ * Return: 0 on success, error code on failure
+ */
+static int local_partition_enable(struct cpuset *cs,
+				int new_prs, struct tmpmasks *tmp)
+{
+	struct cpuset *parent = parent_cs(cs);
+	enum prs_errcode part_error;
+
+	lockdep_assert_held(&cpuset_mutex);
+	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
+
+	/*
+	 * The parent must be a partition root.
+	 * The new cpumask, if present, or the current cpus_allowed must
+	 * not be empty.
+	 */
+	if (!is_partition_valid(parent)) {
+		return is_partition_invalid(parent)
+			? PERR_INVPARENT : PERR_NOTPART;
+	}
+
+	/*
+	 * Need to call compute_excpus() in case
+	 * exclusive_cpus not set. Sibling conflict should only happen
+	 * if exclusive_cpus isn't set.
+	 */
+	if (compute_excpus(cs, tmp->new_cpus))
+		WARN_ON_ONCE(!cpumask_empty(cs->exclusive_cpus));
+
+	part_error = validate_partition(cs, new_prs, tmp->new_cpus);
+	if (part_error)
+		return part_error;
+
+	/*
+	 * This function will only be called when all the preliminary
+	 * checks have passed. At this point, the following condition
+	 * should hold.
+	 *
+	 * (cs->effective_xcpus & cpu_active_mask) ⊆ parent->effective_cpus
+	 *
+	 * Warn if it is not the case.
+	 * addmask is used as temporary variable.
+	 */
+	cpumask_and(tmp->addmask, tmp->new_cpus, cpu_active_mask);
+	WARN_ON_ONCE(!cpumask_subset(tmp->addmask, parent->effective_cpus));
+	partition_enable(cs, parent, new_prs, tmp->new_cpus);
+
+	cpuset_update_tasks_cpumask(parent, tmp->addmask);
+	update_sibling_cpumasks(parent, cs, tmp);
+	return 0;
+}
+
 /**
  * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
  * @cs:      The cpuset that requests change in partition root state
@@ -1893,35 +1962,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 
 	nocpu = tasks_nocpu_error(parent, cs, xcpus);
 
-	if ((cmd == partcmd_enable) || (cmd == partcmd_enablei)) {
-		/*
-		 * Need to call compute_excpus() in case
-		 * exclusive_cpus not set. Sibling conflict should only happen
-		 * if exclusive_cpus isn't set.
-		 */
-		xcpus = tmp->delmask;
-		if (compute_excpus(cs, xcpus))
-			WARN_ON_ONCE(!cpumask_empty(cs->exclusive_cpus));
-		new_prs = (cmd == partcmd_enable) ? PRS_ROOT : PRS_ISOLATED;
-
-		part_error = validate_partition(cs, new_prs, xcpus);
-		if (part_error)
-			return part_error;
-		/*
-		 * This function will only be called when all the preliminary
-		 * checks have passed. At this point, the following condition
-		 * should hold.
-		 *
-		 * (cs->effective_xcpus & cpu_active_mask) ⊆ parent->effective_cpus
-		 *
-		 * Warn if it is not the case.
-		 */
-		cpumask_and(tmp->new_cpus, xcpus, cpu_active_mask);
-		WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
-
-		deleting = true;
-		subparts_delta++;
-	} else if (cmd == partcmd_disable) {
+	if (cmd == partcmd_disable) {
 		/*
 		 * May need to add cpus back to parent's effective_cpus
 		 * (and maybe removed from subpartitions_cpus/isolated_cpus)
@@ -3045,14 +3086,10 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		 * If parent is valid partition, enable local partiion.
 		 * Otherwise, enable a remote partition.
 		 */
-		if (is_partition_valid(parent)) {
-			enum partition_cmd cmd = (new_prs == PRS_ROOT)
-					       ? partcmd_enable : partcmd_enablei;
-
-			err = update_parent_effective_cpumask(cs, cmd, NULL, &tmpmask);
-		} else {
+		if (is_partition_valid(parent))
+			err = local_partition_enable(cs, new_prs, &tmpmask);
+		else
 			err = remote_partition_enable(cs, new_prs, &tmpmask);
-		}
 	} else if (old_prs && new_prs) {
 		/*
 		 * A change in load balance state only, no change in cpumasks.
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 07/16] cpuset: introduce local_partition_disable()
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (5 preceding siblings ...)
  2025-09-28  7:12 ` [PATCH -next RFC 06/16] cpuset: introduce local_partition_enable() Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-10-20  2:46   ` Waiman Long
  2025-09-28  7:12 ` [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate() Chen Ridong
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

The partition_disable() function introduced earlier can be extended to
handle local partition disablement.

First, partition_disable() was enhanced to support local partitions by
properly managing the parent's nr_subparts counter and integrating
notification operations.

Then, local_partition_disable() is introduced, which extracts the local
partition disable logic from update_parent_effective_cpumask(). It calls
partition_disable() to complete the disablement process.

This refactoring establishes a clear separation between local and remote
partition operations while promoting code reuse through the shared
partition_disable() infrastructure.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 60 ++++++++++++++++++++++++++++++------------
 1 file changed, 43 insertions(+), 17 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 154992cdfe9a..87ba43e93540 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1576,13 +1576,20 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
 				    int new_prs, enum prs_errcode prs_err)
 {
 	bool isolcpus_updated;
+	int old_prs;
 
 	lockdep_assert_held(&cpuset_mutex);
 	WARN_ON_ONCE(new_prs > 0);
 	WARN_ON_ONCE(!cpuset_v2());
 
+	old_prs = cs->partition_root_state;
 	spin_lock_irq(&callback_lock);
 	list_del_init(&cs->remote_sibling);
+	if (parent && is_partition_valid(parent) &&
+	    is_partition_valid(cs)) {
+		parent->nr_subparts -= 1;
+		WARN_ON_ONCE(parent->nr_subparts < 0);
+	}
 	/* disable a partition should only delete exclusive cpus */
 	isolcpus_updated = partition_xcpus_del(cs->partition_root_state,
 						parent, cs->effective_xcpus);
@@ -1592,6 +1599,9 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
 	spin_unlock_irq(&callback_lock);
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 	cpuset_force_rebuild();
+	/* Clear exclusive flag; no errors are expected */
+	update_partition_exclusive_flag(cs, new_prs);
+	notify_partition_change(cs, old_prs);
 }
 
 /**
@@ -1870,6 +1880,37 @@ static int local_partition_enable(struct cpuset *cs,
 	return 0;
 }
 
+/**
+ * local_partition_disable - Disable a local partition
+ * @cs: Target cpuset (local partition root) to disable
+ * @tmp: Temporary masks for CPU calculations
+ */
+static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
+{
+	struct cpuset *parent = parent_cs(cs);
+	bool cpumask_updated = false;
+
+	lockdep_assert_held(&cpuset_mutex);
+	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
+
+	if (!is_partition_valid(cs))
+		return;
+
+	/*
+	 * May need to add cpus back to parent's effective_cpus
+	 * (and maybe removed from subpartitions_cpus/isolated_cpus)
+	 * for valid partition root. xcpus may contain CPUs that
+	 * shouldn't be removed from the two global cpumasks.
+	 */
+	cpumask_updated = !cpumask_empty(cs->effective_xcpus);
+	partition_disable(cs, parent, PRS_MEMBER, PERR_NONE);
+
+	if (cpumask_updated) {
+		cpuset_update_tasks_cpumask(parent, tmp->addmask);
+		update_sibling_cpumasks(parent, cs, tmp);
+	}
+}
+
 /**
  * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
  * @cs:      The cpuset that requests change in partition root state
@@ -1962,20 +2003,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 
 	nocpu = tasks_nocpu_error(parent, cs, xcpus);
 
-	if (cmd == partcmd_disable) {
-		/*
-		 * May need to add cpus back to parent's effective_cpus
-		 * (and maybe removed from subpartitions_cpus/isolated_cpus)
-		 * for valid partition root. xcpus may contain CPUs that
-		 * shouldn't be removed from the two global cpumasks.
-		 */
-		if (is_partition_valid(cs)) {
-			cpumask_copy(tmp->addmask, cs->effective_xcpus);
-			adding = true;
-			subparts_delta--;
-		}
-		new_prs = PRS_MEMBER;
-	} else if (newmask) {
+	if (newmask) {
 		/*
 		 * Empty cpumask is not allowed
 		 */
@@ -3104,9 +3132,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		if (is_remote_partition(cs))
 			remote_partition_disable(cs, &tmpmask);
 		else
-			update_parent_effective_cpumask(cs, partcmd_disable,
-							NULL, &tmpmask);
-
+			local_partition_disable(cs, &tmpmask);
 		/*
 		 * Invalidation of child partitions will be done in
 		 * update_cpumasks_hier().
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate()
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (6 preceding siblings ...)
  2025-09-28  7:12 ` [PATCH -next RFC 07/16] cpuset: introduce local_partition_disable() Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-10-20  2:48   ` Waiman Long
  2025-09-28  7:12 ` [PATCH -next RFC 09/16] cpuset: introduce local_partition_update() Chen Ridong
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Build on the partition_disable() infrastructure introduced in the previous
patch to handle local partition invalidation.

The local_partition_invalidate() function factors out the local partition
invalidation logic from update_parent_effective_cpumask(), which delegates
to partition_disable() to complete the invalidation process.

Additionally, correct the transition logic in cpuset_hotplug_update_tasks()
when determining whether to transition an invalid partition root, the check
should be based on non-empty user_cpus rather than non-empty
effective_xcpus. This correction addresses the scenario where
exclusive_cpus is not set but cpus_allowed is configured - in this case,
effective_xcpus may be empty even though the partition should be considered
for re-enablement. The user_cpus-based check ensures proper partition state
transitions under these conditions.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 66 +++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 87ba43e93540..e460d03286ba 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1911,6 +1911,39 @@ static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 	}
 }
 
+/**
+ * local_partition_invalidate - Invalidate a local partition
+ * @cs: Target cpuset (local partition root) to invalidate
+ * @tmp: Temporary masks
+ */
+static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
+{
+	struct cpumask *xcpus = user_xcpus(cs);
+	struct cpuset *parent = parent_cs(cs);
+	int new_prs = cs->partition_root_state;
+	bool cpumask_updated = false;
+
+	lockdep_assert_held(&cpuset_mutex);
+	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
+
+	if (is_partition_invalid(cs))
+		return;
+	/*
+	 * Make the current partition invalid.
+	 */
+	if (is_partition_valid(parent))
+		cpumask_updated = cpumask_and(tmp->addmask,
+					      xcpus, parent->effective_xcpus);
+	if (cs->partition_root_state > 0)
+		new_prs = -cs->partition_root_state;
+
+	partition_disable(cs, parent, new_prs, cs->prs_err);
+	if (cpumask_updated) {
+		cpuset_update_tasks_cpumask(parent, tmp->addmask);
+		update_sibling_cpumasks(parent, cs, tmp);
+	}
+}
+
 /**
  * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
  * @cs:      The cpuset that requests change in partition root state
@@ -1972,23 +2005,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	adding = deleting = false;
 	old_prs = new_prs = cs->partition_root_state;
 
-	if (cmd == partcmd_invalidate) {
-		if (is_partition_invalid(cs))
-			return 0;
-
-		/*
-		 * Make the current partition invalid.
-		 */
-		if (is_partition_valid(parent))
-			adding = cpumask_and(tmp->addmask,
-					     xcpus, parent->effective_xcpus);
-		if (old_prs > 0) {
-			new_prs = -old_prs;
-			subparts_delta--;
-		}
-		goto write_error;
-	}
-
 	/*
 	 * The parent must be a partition root.
 	 * The new cpumask, if present, or the current cpus_allowed must
@@ -2552,7 +2568,7 @@ static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset *trialc
 			if (is_partition_valid(cp) &&
 			    cpumask_intersects(xcpus, cp->effective_xcpus)) {
 				rcu_read_unlock();
-				update_parent_effective_cpumask(cp, partcmd_invalidate, NULL, tmp);
+				local_partition_invalidate(cp, tmp);
 				rcu_read_lock();
 			}
 		}
@@ -2592,8 +2608,7 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
 					   trialcs->effective_xcpus, tmp);
 	} else {
 		if (trialcs->prs_err)
-			update_parent_effective_cpumask(cs, partcmd_invalidate,
-							NULL, tmp);
+			local_partition_invalidate(cs, tmp);
 		else
 			update_parent_effective_cpumask(cs, partcmd_update,
 							trialcs->effective_xcpus, tmp);
@@ -4037,18 +4052,21 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
 	 *    partitions.
 	 */
 	if (is_local_partition(cs) && (!is_partition_valid(parent) ||
-				tasks_nocpu_error(parent, cs, &new_cpus)))
+				tasks_nocpu_error(parent, cs, &new_cpus))) {
 		partcmd = partcmd_invalidate;
+		local_partition_invalidate(cs, tmp);
+	}
 	/*
 	 * On the other hand, an invalid partition root may be transitioned
-	 * back to a regular one with a non-empty effective xcpus.
+	 * back to a regular one with a non-empty user xcpus.
 	 */
 	else if (is_partition_valid(parent) && is_partition_invalid(cs) &&
-		 !cpumask_empty(cs->effective_xcpus))
+		 !cpumask_empty(user_xcpus(cs))) {
 		partcmd = partcmd_update;
+		update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
+	}
 
 	if (partcmd >= 0) {
-		update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
 		if ((partcmd == partcmd_invalidate) || is_partition_valid(cs)) {
 			compute_partition_effective_cpumask(cs, &new_cpus);
 			cpuset_force_rebuild();
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 09/16] cpuset: introduce local_partition_update()
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (7 preceding siblings ...)
  2025-09-28  7:12 ` [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate() Chen Ridong
@ 2025-09-28  7:12 ` Chen Ridong
  2025-10-20  2:57   ` Waiman Long
  2025-09-28  7:13 ` [PATCH -next RFC 10/16] cpuset: remove redundant partition field updates Chen Ridong
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:12 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Extend the partition_update() infrastructure to handle local partition
updates.

The local_partition_update() function replaces the command partcmd_update
previously handled within update_parent_effective_cpumask(). The update
logic follows a state-based approach:

1. Validation check: First verify if the local partition is currently valid
2. Invalidation handling: If the partition is invalid, trigger invalidation
3. State transition: If an invalid partition has no errors, transition to
   valid
4. cpus updates: For local partition that only cpu maks changes, use
   partition_update() to handle partition change.

With the introduction of this function, update_parent_effective_cpumask()
function is removed, simplifying the partition update code path and
creating a cleaner separation between local and remote partition
operations.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 376 +++++++++++++----------------------------
 1 file changed, 122 insertions(+), 254 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index e460d03286ba..d0217db04b69 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1622,12 +1622,14 @@ static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
 	bool isolcpus_updated;
 	bool excl_updated;
 	struct cpuset *parent;
+	int old_prs;
 
 	lockdep_assert_held(&cpuset_mutex);
 	WARN_ON_ONCE(!cpuset_v2());
 	WARN_ON_ONCE(prs <= 0);
 
 	parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
+	old_prs = cs->partition_root_state;
 	excl_updated = !cpumask_empty(tmp->addmask) ||
 			!cpumask_empty(tmp->delmask);
 
@@ -1645,6 +1647,8 @@ static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 	if (excl_updated)
 		cpuset_force_rebuild();
+	update_partition_exclusive_flag(cs, prs);
+	notify_partition_change(cs, old_prs);
 }
 
 /*
@@ -1790,6 +1794,27 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
 	return false;
 }
 
+static bool cpuset_user_cpus_exclusive(struct cpuset *cs)
+{
+	struct cpuset *parent = parent_cs(cs);
+
+	struct cgroup_subsys_state *css;
+	struct cpuset *child;
+	bool exclusive = true;
+
+	rcu_read_lock();
+	cpuset_for_each_child(child, css, parent) {
+		if (child == cs)
+			continue;
+		if (!cpusets_are_exclusive(cs, child)) {
+			exclusive = false;
+			break;
+		}
+	}
+	rcu_read_unlock();
+	return exclusive;
+}
+
 /**
  * validate_partition - Validate a cpuset partition configuration
  * @cs: The cpuset to validate
@@ -1818,6 +1843,39 @@ static enum prs_errcode validate_partition(struct cpuset *cs, int new_prs,
 	return PERR_NONE;
 }
 
+/**
+ * local_partition_check - Validate for local partition
+ * @cs: Target cpuset to validate
+ * @new_prs: New partition root state to validate
+ * @excpus: New exclusive effectuve CPUs mask to validate
+ * @excl_check: Flag to enable exclusive CPUs ownership validation
+ *
+ * Return: PERR_NONE if validation passes, appropriate error code otherwise
+ *
+ * Important: The caller must ensure that @cs's cpu mask is updated before
+ * invoking this function when exclusive CPU validation is required.
+ */
+static enum prs_errcode local_partition_check(struct cpuset *cs, int new_prs,
+							 struct cpumask *excpus, bool excl_check)
+{
+	struct cpuset *parent = parent_cs(cs);
+
+	/*
+	 * The parent must be a partition root.
+	 * The new cpumask, if present, or the current cpus_allowed must
+	 * not be empty.
+	 */
+	if (!is_partition_valid(parent)) {
+		return is_partition_invalid(parent)
+			? PERR_INVPARENT : PERR_NOTPART;
+	}
+
+	if (excl_check && !cpuset_user_cpus_exclusive(cs))
+		return PERR_NOTEXCL;
+
+	return validate_partition(cs, new_prs, excpus);
+}
+
 /**
  * local_partition_enable - Enable local partition for a cpuset
  * @cs: Target cpuset to become a local partition root
@@ -1945,280 +2003,85 @@ static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
 }
 
 /**
- * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
- * @cs:      The cpuset that requests change in partition root state
- * @cmd:     Partition root state change command
- * @newmask: Optional new cpumask for partcmd_update
- * @tmp:     Temporary addmask and delmask
- * Return:   0 or a partition root state error code
- *
- * For partcmd_enable*, the cpuset is being transformed from a non-partition
- * root to a partition root. The effective_xcpus (cpus_allowed if
- * effective_xcpus not set) mask of the given cpuset will be taken away from
- * parent's effective_cpus. The function will return 0 if all the CPUs listed
- * in effective_xcpus can be granted or an error code will be returned.
- *
- * For partcmd_disable, the cpuset is being transformed from a partition
- * root back to a non-partition root. Any CPUs in effective_xcpus will be
- * given back to parent's effective_cpus. 0 will always be returned.
+ * __local_partition_update - Update local CPU partition configuration
+ * @cs: Target cpuset to update
+ * @xcpus: New exclusive CPU mask
+ * @excpus: New effective exclusive CPU mask
+ * @tmp: Temporary mask storage for intermediate calculations
+ * @excl_check: Flag to enable exclusivity validation
  *
- * For partcmd_update, if the optional newmask is specified, the cpu list is
- * to be changed from effective_xcpus to newmask. Otherwise, effective_xcpus is
- * assumed to remain the same. The cpuset should either be a valid or invalid
- * partition root. The partition root state may change from valid to invalid
- * or vice versa. An error code will be returned if transitioning from
- * invalid to valid violates the exclusivity rule.
+ * Handles updates to local CPU partition configurations by validating
+ * changes, managing state transitions, and propagating updates through
+ * the cpuset hierarchy.
  *
- * For partcmd_invalidate, the current partition will be made invalid.
+ * Note on exclusivity checking: Exclusivity validation is required when
+ * transitioning from an invalid to valid partition state. However, when
+ * updating cpus_allowed or exclusive_cpus, exclusivity should have already
+ * been verified by validate_change(). In such cases, excl_check must be
+ * false since the cs cpumasks are not yet updated.
  *
- * The partcmd_enable* and partcmd_disable commands are used by
- * update_prstate(). An error code may be returned and the caller will check
- * for error.
- *
- * The partcmd_update command is used by update_cpumasks_hier() with newmask
- * NULL and update_cpumask() with newmask set. The partcmd_invalidate is used
- * by update_cpumask() with NULL newmask. In both cases, the callers won't
- * check for error and so partition_root_state and prs_err will be updated
- * directly.
+ * Return: Partition error code (PERR_NONE indicates success)
  */
-static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
-					   struct cpumask *newmask,
-					   struct tmpmasks *tmp)
+static int __local_partition_update(struct cpuset *cs, struct cpumask *xcpus,
+				  struct cpumask *excpus, struct tmpmasks *tmp,
+				  bool excl_check)
 {
 	struct cpuset *parent = parent_cs(cs);
-	int adding;	/* Adding cpus to parent's effective_cpus	*/
-	int deleting;	/* Deleting cpus from parent's effective_cpus	*/
-	int old_prs, new_prs;
 	int part_error = PERR_NONE;	/* Partition error? */
-	int subparts_delta = 0;
-	int isolcpus_updated = 0;
-	struct cpumask *xcpus = user_xcpus(cs);
-	bool nocpu;
+	int old_prs, new_prs;
+	bool cpumask_updated = false;
 
 	lockdep_assert_held(&cpuset_mutex);
-	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
+	/* For local partition only */
+	if (WARN_ON_ONCE(is_remote_partition(cs) || cs_is_member(cs)))
+		return PERR_NONE;
 
+	old_prs = cs->partition_root_state;
 	/*
-	 * new_prs will only be changed for the partcmd_update and
-	 * partcmd_invalidate commands.
+	 * If new_prs < 0, it might transition to valid partition state.
+	 * Use absolute value for validation checks.
 	 */
-	adding = deleting = false;
-	old_prs = new_prs = cs->partition_root_state;
-
-	/*
-	 * The parent must be a partition root.
-	 * The new cpumask, if present, or the current cpus_allowed must
-	 * not be empty.
-	 */
-	if (!is_partition_valid(parent)) {
-		return is_partition_invalid(parent)
-		       ? PERR_INVPARENT : PERR_NOTPART;
-	}
-	if (!newmask && xcpus_empty(cs))
-		return PERR_CPUSEMPTY;
-
-	nocpu = tasks_nocpu_error(parent, cs, xcpus);
-
-	if (newmask) {
-		/*
-		 * Empty cpumask is not allowed
-		 */
-		if (cpumask_empty(newmask)) {
-			part_error = PERR_CPUSEMPTY;
-			goto write_error;
-		}
-
-		/* Check newmask again, whether cpus are available for parent/cs */
-		nocpu |= tasks_nocpu_error(parent, cs, newmask);
-
-		/*
-		 * partcmd_update with newmask:
-		 *
-		 * Compute add/delete mask to/from effective_cpus
-		 *
-		 * For valid partition:
-		 *   addmask = exclusive_cpus & ~newmask
-		 *			      & parent->effective_xcpus
-		 *   delmask = newmask & ~exclusive_cpus
-		 *		       & parent->effective_xcpus
-		 *
-		 * For invalid partition:
-		 *   delmask = newmask & parent->effective_xcpus
-		 */
-		if (is_partition_invalid(cs)) {
-			adding = false;
-			deleting = cpumask_and(tmp->delmask,
-					newmask, parent->effective_xcpus);
-		} else {
-			cpumask_andnot(tmp->addmask, xcpus, newmask);
-			adding = cpumask_and(tmp->addmask, tmp->addmask,
-					     parent->effective_xcpus);
-
-			cpumask_andnot(tmp->delmask, newmask, xcpus);
-			deleting = cpumask_and(tmp->delmask, tmp->delmask,
-					       parent->effective_xcpus);
-		}
-		/*
-		 * The new CPUs to be removed from parent's effective CPUs
-		 * must be present.
-		 */
-		if (deleting) {
-			cpumask_and(tmp->new_cpus, tmp->delmask, cpu_active_mask);
-			WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
-		}
-
-		/*
-		 * Make partition invalid if parent's effective_cpus could
-		 * become empty and there are tasks in the parent.
-		 */
-		if (nocpu && (!adding ||
-		    !cpumask_intersects(tmp->addmask, cpu_active_mask))) {
-			part_error = PERR_NOCPUS;
-			deleting = false;
-			adding = cpumask_and(tmp->addmask,
-					     xcpus, parent->effective_xcpus);
-		}
-	} else {
-		/*
-		 * partcmd_update w/o newmask
-		 *
-		 * delmask = effective_xcpus & parent->effective_cpus
-		 *
-		 * This can be called from:
-		 * 1) update_cpumasks_hier()
-		 * 2) cpuset_hotplug_update_tasks()
-		 *
-		 * Check to see if it can be transitioned from valid to
-		 * invalid partition or vice versa.
-		 *
-		 * A partition error happens when parent has tasks and all
-		 * its effective CPUs will have to be distributed out.
-		 */
-		if (nocpu) {
-			part_error = PERR_NOCPUS;
-			if (is_partition_valid(cs))
-				adding = cpumask_and(tmp->addmask,
-						xcpus, parent->effective_xcpus);
-		} else if (is_partition_invalid(cs) && !cpumask_empty(xcpus) &&
-			   cpumask_subset(xcpus, parent->effective_xcpus)) {
-			struct cgroup_subsys_state *css;
-			struct cpuset *child;
-			bool exclusive = true;
-
-			/*
-			 * Convert invalid partition to valid has to
-			 * pass the cpu exclusivity test.
-			 */
-			rcu_read_lock();
-			cpuset_for_each_child(child, css, parent) {
-				if (child == cs)
-					continue;
-				if (!cpusets_are_exclusive(cs, child)) {
-					exclusive = false;
-					break;
-				}
-			}
-			rcu_read_unlock();
-			if (exclusive)
-				deleting = cpumask_and(tmp->delmask,
-						xcpus, parent->effective_cpus);
-			else
-				part_error = PERR_NOTEXCL;
-		}
-	}
-
-write_error:
-	if (part_error)
-		WRITE_ONCE(cs->prs_err, part_error);
-
-	if (cmd == partcmd_update) {
-		/*
-		 * Check for possible transition between valid and invalid
-		 * partition root.
-		 */
-		switch (cs->partition_root_state) {
-		case PRS_ROOT:
-		case PRS_ISOLATED:
-			if (part_error) {
-				new_prs = -old_prs;
-				subparts_delta--;
-			}
-			break;
-		case PRS_INVALID_ROOT:
-		case PRS_INVALID_ISOLATED:
-			if (!part_error) {
-				new_prs = -old_prs;
-				subparts_delta++;
-			}
-			break;
-		}
+	new_prs = old_prs < 0 ? -old_prs : old_prs;
+	part_error = local_partition_check(cs, new_prs, excpus, excl_check);
+	if (part_error) {
+		local_partition_invalidate(cs, tmp);
+		return part_error;
 	}
 
-	if (!adding && !deleting && (new_prs == old_prs))
-		return 0;
+	/* Nothing changes, return PERR_NONE */
+	if (new_prs == old_prs && cpumask_equal(excpus, cs->effective_xcpus))
+		return PERR_NONE;
 
 	/*
-	 * Transitioning between invalid to valid or vice versa may require
-	 * changing CS_CPU_EXCLUSIVE. In the case of partcmd_update,
-	 * validate_change() has already been successfully called and
-	 * CPU lists in cs haven't been updated yet. So defer it to later.
+	 * If partition was previously invalid but now passes checks,
+	 * enable it and update related flags
 	 */
-	if ((old_prs != new_prs) && (cmd != partcmd_update))  {
-		int err = update_partition_exclusive_flag(cs, new_prs);
-
-		if (err)
-			return err;
+	if (is_partition_invalid(cs) && !part_error) {
+		partition_enable(cs, parent, new_prs, excpus);
+		update_partition_exclusive_flag(cs, new_prs);
+		update_partition_sd_lb(cs, old_prs);
+		return part_error;
 	}
 
+	cpumask_updated = cpumask_andnot(tmp->addmask, excpus, cs->effective_xcpus);
+	cpumask_updated |= cpumask_andnot(tmp->delmask, cs->effective_xcpus, excpus);
+	partition_update(cs, new_prs, xcpus, excpus, tmp);
 	/*
-	 * Change the parent's effective_cpus & effective_xcpus (top cpuset
-	 * only).
-	 *
-	 * Newly added CPUs will be removed from effective_cpus and
-	 * newly deleted ones will be added back to effective_cpus.
-	 */
-	spin_lock_irq(&callback_lock);
-	if (old_prs != new_prs) {
-		cs->partition_root_state = new_prs;
-		if (new_prs <= 0)
-			cs->nr_subparts = 0;
-	}
-	/*
-	 * Adding to parent's effective_cpus means deletion CPUs from cs
-	 * and vice versa.
+	 * Propagate changes in parent's effective_cpus down the hierarchy.
 	 */
-	if (adding)
-		isolcpus_updated += partition_xcpus_del(old_prs, parent,
-							tmp->addmask);
-	if (deleting)
-		isolcpus_updated += partition_xcpus_add(new_prs, parent,
-							tmp->delmask);
-
-	if (is_partition_valid(parent)) {
-		parent->nr_subparts += subparts_delta;
-		WARN_ON_ONCE(parent->nr_subparts < 0);
-	}
-	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
-
-	if ((old_prs != new_prs) && (cmd == partcmd_update))
-		update_partition_exclusive_flag(cs, new_prs);
-
-	if (adding || deleting) {
+	if (cpumask_updated) {
 		cpuset_update_tasks_cpumask(parent, tmp->addmask);
 		update_sibling_cpumasks(parent, cs, tmp);
 	}
+	return part_error;
+}
 
-	/*
-	 * For partcmd_update without newmask, it is being called from
-	 * cpuset_handle_hotplug(). Update the load balance flag and
-	 * scheduling domain accordingly.
-	 */
-	if ((cmd == partcmd_update) && !newmask)
-		update_partition_sd_lb(cs, old_prs);
+static int local_partition_update(struct cpuset *cs, struct tmpmasks *tmp)
+{
+	struct cpuset *parent = parent_cs(cs);
 
-	notify_partition_change(cs, old_prs);
-	return 0;
+	cpumask_and(tmp->new_cpus, user_xcpus(cs), parent->effective_xcpus);
+	return __local_partition_update(cs, NULL, tmp->new_cpus, tmp, true);
 }
 
 /**
@@ -2419,9 +2282,16 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
 		if (!css_tryget_online(&cp->css))
 			continue;
 		rcu_read_unlock();
+		/*
+		 * The tmp->new_cpus may by modified.
+		 * Update effective_cpus before passing tmp to other functions.
+		 */
+		spin_lock_irq(&callback_lock);
+		cpumask_copy(cp->effective_cpus, tmp->new_cpus);
+		spin_unlock_irq(&callback_lock);
 
 		if (update_parent) {
-			update_parent_effective_cpumask(cp, partcmd_update, NULL, tmp);
+			local_partition_update(cp, tmp);
 			/*
 			 * The cpuset partition_root_state may become
 			 * invalid. Capture it.
@@ -2430,7 +2300,6 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
 		}
 
 		spin_lock_irq(&callback_lock);
-		cpumask_copy(cp->effective_cpus, tmp->new_cpus);
 		cp->partition_root_state = new_prs;
 		if (!cpumask_empty(cp->exclusive_cpus) && (cp != cs))
 			compute_excpus(cp, cp->effective_xcpus);
@@ -2610,8 +2479,8 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
 		if (trialcs->prs_err)
 			local_partition_invalidate(cs, tmp);
 		else
-			update_parent_effective_cpumask(cs, partcmd_update,
-							trialcs->effective_xcpus, tmp);
+			__local_partition_update(cs, trialcs->exclusive_cpus,
+						trialcs->effective_xcpus, tmp, false);
 	}
 }
 
@@ -4063,9 +3932,8 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
 	else if (is_partition_valid(parent) && is_partition_invalid(cs) &&
 		 !cpumask_empty(user_xcpus(cs))) {
 		partcmd = partcmd_update;
-		update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
+		local_partition_update(cs, tmp);
 	}
-
 	if (partcmd >= 0) {
 		if ((partcmd == partcmd_invalidate) || is_partition_valid(cs)) {
 			compute_partition_effective_cpumask(cs, &new_cpus);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 10/16] cpuset: remove redundant partition field updates
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (8 preceding siblings ...)
  2025-09-28  7:12 ` [PATCH -next RFC 09/16] cpuset: introduce local_partition_update() Chen Ridong
@ 2025-09-28  7:13 ` Chen Ridong
  2025-09-28  7:13 ` [PATCH -next RFC 11/16] cpuset: simplify partition update logic for hotplug tasks Chen Ridong
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:13 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

With the previous patch series, partition-related fields are now properly
managed during partition enable, disable, and update operations. There
should be no need to set these fields outside of these dedicated partition
operations.

This patch removes the redundant partition field updates from the cpumask
setting code path. However, one exception remains: when setting
cpuset.cpus.exclusive on a non-partition cpuset, update_exclusive_cpumask()
must still set effective_xcpus directly. This is necessary because no
partition operation is invoked in this scenario, yet effective_xcpus needs
to be properly initialized.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index d0217db04b69..9e98df542715 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2526,9 +2526,6 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 
 	spin_lock_irq(&callback_lock);
 	cpumask_copy(cs->cpus_allowed, trialcs->cpus_allowed);
-	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
-	if ((old_prs > 0) && !is_partition_valid(cs))
-		reset_partition_data(cs);
 	spin_unlock_irq(&callback_lock);
 
 	/* effective_cpus/effective_xcpus will be updated here */
@@ -2592,8 +2589,6 @@ static int update_exclusive_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 	spin_lock_irq(&callback_lock);
 	cpumask_copy(cs->exclusive_cpus, trialcs->exclusive_cpus);
 	cpumask_copy(cs->effective_xcpus, trialcs->effective_xcpus);
-	if ((old_prs > 0) && !is_partition_valid(cs))
-		reset_partition_data(cs);
 	spin_unlock_irq(&callback_lock);
 
 	/*
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 11/16] cpuset: simplify partition update logic for hotplug tasks
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (9 preceding siblings ...)
  2025-09-28  7:13 ` [PATCH -next RFC 10/16] cpuset: remove redundant partition field updates Chen Ridong
@ 2025-09-28  7:13 ` Chen Ridong
  2025-10-20  3:00   ` Waiman Long
  2025-09-28  7:13 ` [PATCH -next RFC 12/16] cpuset: unify local partition disable and invalidate Chen Ridong
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:13 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Simplify the partition update logic in cpuset_hotplug_update_tasks() by
calling the unified local_partition_update() interface.

For local partitions, the previous patch introduced local_partition_update
which handles both validation state transitions:
- Invalidates local partitions that fail validation checks
- Transitions invalid partitions to valid state when no errors are detected

This eliminates the need for separate transition logic
in cpuset_hotplug_update_tasks(), which can now simply call
local_partition_update() to handle all local partition changes.

This patch simplifies the logic by always proceeding to update_tasks for
remote partitions, regardless of whether they were disabled or not. Since
the original code didn't perform any meaningful operations for non-disabled
remote partitions, this change should not affect functionality.

The partition_cmd mechanism can now be safely removed as it is no longer
referenced by any code paths after the partition update logic
simplification.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 67 ++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 41 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 9e98df542715..a1896a199c8b 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1211,17 +1211,6 @@ static void compute_effective_cpumask(struct cpumask *new_cpus,
 	cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
 }
 
-/*
- * Commands for update_parent_effective_cpumask
- */
-enum partition_cmd {
-	partcmd_enable,		/* Enable partition root	  */
-	partcmd_enablei,	/* Enable isolated partition root */
-	partcmd_disable,	/* Disable partition root	  */
-	partcmd_update,		/* Update parent's effective_cpus */
-	partcmd_invalidate,	/* Make partition invalid	  */
-};
-
 static void update_sibling_cpumasks(struct cpuset *parent, struct cpuset *cs,
 				    struct tmpmasks *tmp);
 
@@ -2062,6 +2051,9 @@ static int __local_partition_update(struct cpuset *cs, struct cpumask *xcpus,
 		update_partition_sd_lb(cs, old_prs);
 		return part_error;
 	}
+	/* Nothing changes, return PERR_NONE */
+	if (new_prs == old_prs && cpumask_equal(excpus, cs->effective_xcpus))
+		return PERR_NONE;
 
 	cpumask_updated = cpumask_andnot(tmp->addmask, excpus, cs->effective_xcpus);
 	cpumask_updated |= cpumask_andnot(tmp->delmask, cs->effective_xcpus, excpus);
@@ -3868,8 +3860,6 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
 	static nodemask_t new_mems;
 	bool cpus_updated;
 	bool mems_updated;
-	bool remote;
-	int partcmd = -1;
 	struct cpuset *parent;
 retry:
 	wait_event(cpuset_attach_wq, cs->attach_in_progress == 0);
@@ -3896,16 +3886,15 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
 	 * Compute effective_cpus for valid partition root, may invalidate
 	 * child partition roots if necessary.
 	 */
-	remote = is_remote_partition(cs);
-	if (remote || (is_partition_valid(cs) && is_partition_valid(parent)))
+	if (is_remote_partition(cs)) {
 		compute_partition_effective_cpumask(cs, &new_cpus);
-
-	if (remote && cpumask_empty(&new_cpus) &&
-	    partition_is_populated(cs, NULL)) {
-		cs->prs_err = PERR_HOTPLUG;
-		remote_partition_disable(cs, tmp);
-		compute_effective_cpumask(&new_cpus, cs, parent);
-		remote = false;
+		if (cpumask_empty(&new_cpus) &&
+		    partition_is_populated(cs, NULL)) {
+			cs->prs_err = PERR_HOTPLUG;
+			remote_partition_disable(cs, tmp);
+			compute_effective_cpumask(&new_cpus, cs, parent);
+		}
+		goto update_tasks;
 	}
 
 	/*
@@ -3913,28 +3902,24 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
 	 * the following conditions hold:
 	 * 1) empty effective cpus but not valid empty partition.
 	 * 2) parent is invalid or doesn't grant any cpus to child
-	 *    partitions.
-	 */
-	if (is_local_partition(cs) && (!is_partition_valid(parent) ||
-				tasks_nocpu_error(parent, cs, &new_cpus))) {
-		partcmd = partcmd_invalidate;
-		local_partition_invalidate(cs, tmp);
-	}
-	/*
+	 *  partitions.
+	 *
 	 * On the other hand, an invalid partition root may be transitioned
 	 * back to a regular one with a non-empty user xcpus.
+	 *
+	 * local_partition_update can handle these cases.
 	 */
-	else if (is_partition_valid(parent) && is_partition_invalid(cs) &&
-		 !cpumask_empty(user_xcpus(cs))) {
-		partcmd = partcmd_update;
-		local_partition_update(cs, tmp);
-	}
-	if (partcmd >= 0) {
-		if ((partcmd == partcmd_invalidate) || is_partition_valid(cs)) {
-			compute_partition_effective_cpumask(cs, &new_cpus);
-			cpuset_force_rebuild();
-		}
-	}
+	local_partition_update(cs, tmp);
+
+	/*
+	 * Recompute effective CPU mask after partition state update:
+	 * - For valid partitions: calculate partition-specific effective CPUs
+	 * - For invalid partitions: compute member effective CPU mask
+	 */
+	if (is_partition_valid(cs))
+		compute_partition_effective_cpumask(cs, &new_cpus);
+	else
+		compute_effective_cpumask(&new_cpus, cs, parent);
 
 update_tasks:
 	cpus_updated = !cpumask_equal(&new_cpus, cs->effective_cpus);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 12/16] cpuset: unify local partition disable and invalidate
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (10 preceding siblings ...)
  2025-09-28  7:13 ` [PATCH -next RFC 11/16] cpuset: simplify partition update logic for hotplug tasks Chen Ridong
@ 2025-09-28  7:13 ` Chen Ridong
  2025-09-28  7:13 ` [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask Chen Ridong
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:13 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

The local_partition_invalidate() and local_partition_disable() functions
contain similar logic that can be unified into a single implementation.
This patch consolidates both functions into local_partition_disable(),
creating symmetry with the existing remote_partition_disable() function.

To support this unification, the partition error code is added as an
input parameter to local_partition_disable(), allowing proper recording
of the disablement reason for both invalid and regular disable operations.

This refactoring reduces code duplication and establishes a consistent
interface for partition disable operations across both local and remote
partition types.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 53 +++++++++++-------------------------------
 1 file changed, 14 insertions(+), 39 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index a1896a199c8b..6625b803ba02 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1932,10 +1932,12 @@ static int local_partition_enable(struct cpuset *cs,
  * @cs: Target cpuset (local partition root) to disable
  * @tmp: Temporary masks for CPU calculations
  */
-static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
+static void local_partition_disable(struct cpuset *cs, enum prs_errcode part_error,
+				    struct tmpmasks *tmp)
 {
 	struct cpuset *parent = parent_cs(cs);
 	bool cpumask_updated = false;
+	int new_prs;
 
 	lockdep_assert_held(&cpuset_mutex);
 	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
@@ -1943,48 +1945,21 @@ static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 	if (!is_partition_valid(cs))
 		return;
 
+	if (part_error)
+		new_prs = -cs->partition_root_state;
+	else
+		new_prs = 0;
 	/*
 	 * May need to add cpus back to parent's effective_cpus
 	 * (and maybe removed from subpartitions_cpus/isolated_cpus)
 	 * for valid partition root. xcpus may contain CPUs that
 	 * shouldn't be removed from the two global cpumasks.
 	 */
-	cpumask_updated = !cpumask_empty(cs->effective_xcpus);
-	partition_disable(cs, parent, PRS_MEMBER, PERR_NONE);
-
-	if (cpumask_updated) {
-		cpuset_update_tasks_cpumask(parent, tmp->addmask);
-		update_sibling_cpumasks(parent, cs, tmp);
-	}
-}
-
-/**
- * local_partition_invalidate - Invalidate a local partition
- * @cs: Target cpuset (local partition root) to invalidate
- * @tmp: Temporary masks
- */
-static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
-{
-	struct cpumask *xcpus = user_xcpus(cs);
-	struct cpuset *parent = parent_cs(cs);
-	int new_prs = cs->partition_root_state;
-	bool cpumask_updated = false;
-
-	lockdep_assert_held(&cpuset_mutex);
-	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
-
-	if (is_partition_invalid(cs))
-		return;
-	/*
-	 * Make the current partition invalid.
-	 */
 	if (is_partition_valid(parent))
-		cpumask_updated = cpumask_and(tmp->addmask,
-					      xcpus, parent->effective_xcpus);
-	if (cs->partition_root_state > 0)
-		new_prs = -cs->partition_root_state;
+		cpumask_updated = !cpumask_empty(cs->effective_xcpus);
+
+	partition_disable(cs, parent, new_prs, part_error);
 
-	partition_disable(cs, parent, new_prs, cs->prs_err);
 	if (cpumask_updated) {
 		cpuset_update_tasks_cpumask(parent, tmp->addmask);
 		update_sibling_cpumasks(parent, cs, tmp);
@@ -2033,7 +2008,7 @@ static int __local_partition_update(struct cpuset *cs, struct cpumask *xcpus,
 	new_prs = old_prs < 0 ? -old_prs : old_prs;
 	part_error = local_partition_check(cs, new_prs, excpus, excl_check);
 	if (part_error) {
-		local_partition_invalidate(cs, tmp);
+		local_partition_disable(cs, part_error, tmp);
 		return part_error;
 	}
 
@@ -2429,7 +2404,7 @@ static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset *trialc
 			if (is_partition_valid(cp) &&
 			    cpumask_intersects(xcpus, cp->effective_xcpus)) {
 				rcu_read_unlock();
-				local_partition_invalidate(cp, tmp);
+				local_partition_disable(cp, PERR_NOTEXCL, tmp);
 				rcu_read_lock();
 			}
 		}
@@ -2469,7 +2444,7 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
 					   trialcs->effective_xcpus, tmp);
 	} else {
 		if (trialcs->prs_err)
-			local_partition_invalidate(cs, tmp);
+			local_partition_disable(cs, trialcs->prs_err, tmp);
 		else
 			__local_partition_update(cs, trialcs->exclusive_cpus,
 						trialcs->effective_xcpus, tmp, false);
@@ -3003,7 +2978,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		if (is_remote_partition(cs))
 			remote_partition_disable(cs, &tmpmask);
 		else
-			local_partition_disable(cs, &tmpmask);
+			local_partition_disable(cs, PERR_NONE, &tmpmask);
 		/*
 		 * Invalidation of child partitions will be done in
 		 * update_cpumasks_hier().
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (11 preceding siblings ...)
  2025-09-28  7:13 ` [PATCH -next RFC 12/16] cpuset: unify local partition disable and invalidate Chen Ridong
@ 2025-09-28  7:13 ` Chen Ridong
  2025-10-20  3:02   ` Waiman Long
  2025-09-28  7:13 ` [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root Chen Ridong
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:13 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

Replace the partition invalidation logic in the
compute_partition_effective_cpumask() with a call to partition_disable().

This centralizes partition state management and ensures consistent
handling of partition disable operations throughout the cpuset subsystem.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 6625b803ba02..20288dbd6ccf 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -170,15 +170,6 @@ static inline bool cs_is_member(const struct cpuset *cs)
 	return cs->partition_root_state == PRS_MEMBER;
 }
 
-/*
- * Callers should hold callback_lock to modify partition_root_state.
- */
-static inline void make_partition_invalid(struct cpuset *cs)
-{
-	if (cs->partition_root_state > 0)
-		cs->partition_root_state = -cs->partition_root_state;
-}
-
 /*
  * Send notification event of whenever partition_root_state changes.
  */
@@ -2073,6 +2064,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 	struct cgroup_subsys_state *css;
 	struct cpuset *child;
 	bool populated = partition_is_populated(cs, NULL);
+	enum prs_errcode prs_err;
 
 	/*
 	 * Check child partition roots to see if they should be
@@ -2095,26 +2087,20 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 		 * partition root.
 		 */
 		WARN_ON_ONCE(is_remote_partition(child));
-		child->prs_err = 0;
+		prs_err = 0;
 		if (!cpumask_subset(child->effective_xcpus,
 				    cs->effective_xcpus))
-			child->prs_err = PERR_INVCPUS;
+			prs_err = PERR_INVCPUS;
 		else if (populated &&
 			 cpumask_subset(new_ecpus, child->effective_xcpus))
-			child->prs_err = PERR_NOCPUS;
-
-		if (child->prs_err) {
-			int old_prs = child->partition_root_state;
+			prs_err = PERR_NOCPUS;
 
+		if (prs_err) {
 			/*
 			 * Invalidate child partition
 			 */
-			spin_lock_irq(&callback_lock);
-			make_partition_invalid(child);
-			cs->nr_subparts--;
-			child->nr_subparts = 0;
-			spin_unlock_irq(&callback_lock);
-			notify_partition_change(child, old_prs);
+			partition_disable(child, parent_cs(child),
+					  -child->partition_root_state, prs_err);
 			continue;
 		}
 		cpumask_andnot(new_ecpus, new_ecpus,
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (12 preceding siblings ...)
  2025-09-28  7:13 ` [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask Chen Ridong
@ 2025-09-28  7:13 ` Chen Ridong
  2025-10-20  3:06   ` Waiman Long
  2025-09-28  7:13 ` [PATCH -next RFC 15/16] cpuset: use partition_disable for update_prstate Chen Ridong
                   ` (3 subsequent siblings)
  17 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:13 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

A bug was detected with the following steps:

  # cd /sys/fs/cgroup/
  # mkdir test
  # echo 9 > test/cpuset.cpus
  # echo isolated > test/cpuset.cpus.partition
  # cat test/cpuset.cpus.partition
  isolated
  # cat test/cpuset.cpus
  9
  # echo root > test/cpuset.cpus.partition
  # cat test/cpuset.cpus
  9
  # cat test/cpuset.cpus.partition
  root

CPU 9 was initially placed in an isolated partition. When the partition
type is changed from isolated to root, CPU 9 remains in what becomes a
valid root partition. This violates the rule that isolcpus can only be
assigned to isolated partitions.

Fix the issue by re-enabling partition validation, which performs
comprehensive partition error checking. In the scenario described above,
this change causes the operation to fail with housekeeping conflicts,
preventing the invalid configuration.

Additionally, when enable a local partition, the warning for tmp->addmask
not being a subset of parent's effective CPUs was removed. This warning was
triggered during local partition re-enablement because the CPUs were
already added to exclusive_cpus during the previous enable operation. The
subset check is not applicable in this re-enablement scenario.

Fixes: f28e22441f35 ("cgroup/cpuset: Add a new isolated cpus.partition type")
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 20288dbd6ccf..2aaa688c596f 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1873,6 +1873,7 @@ static int local_partition_enable(struct cpuset *cs,
 {
 	struct cpuset *parent = parent_cs(cs);
 	enum prs_errcode part_error;
+	bool cpumask_updated = false;
 
 	lockdep_assert_held(&cpuset_mutex);
 	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
@@ -1899,22 +1900,14 @@ static int local_partition_enable(struct cpuset *cs,
 	if (part_error)
 		return part_error;
 
-	/*
-	 * This function will only be called when all the preliminary
-	 * checks have passed. At this point, the following condition
-	 * should hold.
-	 *
-	 * (cs->effective_xcpus & cpu_active_mask) ⊆ parent->effective_cpus
-	 *
-	 * Warn if it is not the case.
-	 * addmask is used as temporary variable.
-	 */
-	cpumask_and(tmp->addmask, tmp->new_cpus, cpu_active_mask);
-	WARN_ON_ONCE(!cpumask_subset(tmp->addmask, parent->effective_cpus));
+	cpumask_updated = cpumask_andnot(tmp->addmask, tmp->new_cpus,
+					 parent->effective_cpus);
 	partition_enable(cs, parent, new_prs, tmp->new_cpus);
 
-	cpuset_update_tasks_cpumask(parent, tmp->addmask);
-	update_sibling_cpumasks(parent, cs, tmp);
+	if (cpumask_updated) {
+		cpuset_update_tasks_cpumask(parent, tmp->addmask);
+		update_sibling_cpumasks(parent, cs, tmp);
+	}
 	return 0;
 }
 
@@ -2902,7 +2895,6 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	int err = PERR_NONE, old_prs = cs->partition_root_state;
 	struct cpuset *parent = parent_cs(cs);
 	struct tmpmasks tmpmask;
-	bool isolcpus_updated = false;
 
 	if (old_prs == new_prs)
 		return 0;
@@ -2920,7 +2912,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	if (err)
 		goto out;
 
-	if (!old_prs) {
+	if (new_prs > 0) {
 		/*
 		 * cpus_allowed and exclusive_cpus cannot be both empty.
 		 */
@@ -2950,12 +2942,6 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 			err = local_partition_enable(cs, new_prs, &tmpmask);
 		else
 			err = remote_partition_enable(cs, new_prs, &tmpmask);
-	} else if (old_prs && new_prs) {
-		/*
-		 * A change in load balance state only, no change in cpumasks.
-		 * Need to update isolated_cpus.
-		 */
-		isolcpus_updated = true;
 	} else {
 		/*
 		 * Switching back to member is always allowed even if it
@@ -2985,16 +2971,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	WRITE_ONCE(cs->prs_err, err);
 	if (!is_partition_valid(cs))
 		reset_partition_data(cs);
-	else if (isolcpus_updated)
-		isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
 
 	/* Force update if switching back to member & update effective_xcpus */
 	update_cpumasks_hier(cs, &tmpmask, !new_prs);
 
 	/* A newly created partition must have effective_xcpus set */
-	WARN_ON_ONCE(!old_prs && (new_prs > 0)
+	WARN_ON_ONCE(!old_prs && (cs->partition_root_state > 0)
 			      && cpumask_empty(cs->effective_xcpus));
 
 	/* Update sched domains and load balance flag */
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 15/16] cpuset: use partition_disable for update_prstate
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (13 preceding siblings ...)
  2025-09-28  7:13 ` [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root Chen Ridong
@ 2025-09-28  7:13 ` Chen Ridong
  2025-09-28  7:13 ` [PATCH -next RFC 16/16] cpuset: remove prs_err clear when notify_partition_change Chen Ridong
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:13 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

The update_prstate() function currently handles partition state transitions
by directly manipulating partition-related fields. However, when a
partition is enabled or disabled through the proper interfaces, these
fields are already set correctly, making the direct manipulation redundant.

This patch refactors the logic to use partition_disable() when partition
errors are detected, ensuring consistent state management. For successful
state transitions, the partition-related fields are set appropriately.

This change eliminates redundant field manipulation and centralizes
partition state management through the established partition_disable()
interface, improving code maintainability and consistency.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 2aaa688c596f..a22cf97e6af5 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -2961,17 +2961,8 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	 * Make partition invalid & disable CS_CPU_EXCLUSIVE if an error
 	 * happens.
 	 */
-	if (err) {
-		new_prs = -new_prs;
-		update_partition_exclusive_flag(cs, new_prs);
-	}
-
-	spin_lock_irq(&callback_lock);
-	cs->partition_root_state = new_prs;
-	WRITE_ONCE(cs->prs_err, err);
-	if (!is_partition_valid(cs))
-		reset_partition_data(cs);
-	spin_unlock_irq(&callback_lock);
+	if (err)
+		partition_disable(cs, parent, -new_prs, err);
 
 	/* Force update if switching back to member & update effective_xcpus */
 	update_cpumasks_hier(cs, &tmpmask, !new_prs);
@@ -2983,7 +2974,6 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	/* Update sched domains and load balance flag */
 	update_partition_sd_lb(cs, old_prs);
 
-	notify_partition_change(cs, old_prs);
 	if (force_sd_rebuild)
 		rebuild_sched_domains_locked();
 	free_tmpmasks(&tmpmask);
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* [PATCH -next RFC 16/16] cpuset: remove prs_err clear when notify_partition_change
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (14 preceding siblings ...)
  2025-09-28  7:13 ` [PATCH -next RFC 15/16] cpuset: use partition_disable for update_prstate Chen Ridong
@ 2025-09-28  7:13 ` Chen Ridong
  2025-09-28  9:57 ` [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
  2025-09-28 16:00 ` Waiman Long
  17 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  7:13 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny; +Cc: cgups, linux-kernel, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

The prs_err should be properly set when partition state is set, it
does't have to reset in the notify_partition_change, just remove it.

Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/cpuset.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index a22cf97e6af5..262572af68bb 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -178,10 +178,6 @@ static inline void notify_partition_change(struct cpuset *cs, int old_prs)
 	if (old_prs == cs->partition_root_state)
 		return;
 	cgroup_file_notify(&cs->partition_file);
-
-	/* Reset prs_err if not invalid */
-	if (is_partition_valid(cs))
-		WRITE_ONCE(cs->prs_err, PERR_NONE);
 }
 
 /*
-- 
2.34.1


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 00/16] cpuset: rework local partition logic
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (15 preceding siblings ...)
  2025-09-28  7:13 ` [PATCH -next RFC 16/16] cpuset: remove prs_err clear when notify_partition_change Chen Ridong
@ 2025-09-28  9:57 ` Chen Ridong
  2025-09-28 16:00 ` Waiman Long
  17 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-28  9:57 UTC (permalink / raw)
  To: longman, tj, hannes, mkoutny
  Cc: linux-kernel, lujialin4, chenridong, open list:CONTROL GROUP (CGROUP)


On 2025/9/28 15:12, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
> 
> The current local partition implementation consolidates all operations
> (enable, disable, invalidate, and update) within the large
> update_parent_effective_cpumask() function, which exceeds 300 lines.
> This monolithic approach has become increasingly difficult to understand
> and maintain. Additionally, partition-related fields are updated in
> multiple locations, leading to redundant code and potential corner case
> oversights.
> 
> This patch series refactors the local partition logic by separating
> operations into dedicated functions: local_partition_enable(),
> local_partition_disable(), and local_partition_update(), creating
> symmetry with the existing remote partition infrastructure.
> 
> The series is organized as follows:
> 
> 1. Infrastructure Preparation (Patches 1-2):
>    - Code cleanup and preparation for the refactoring work
> 
> 2. Core Partition Operations (Patches 3-5):
>    - Factor out partition_enable(), partition_disable(), and
>      partition_update() functions from remote partition operations
> 
> 3. Local Partition Implementation (Patches 6-9):
>    - Separate update_parent_effective_cpumask() into dedicated functions:
>      * local_partition_enable()
>      * local_partition_disable()
>      * local_partition_invalidate()
>      * local_partition_update()
> 
> 4. Optimization and Cleanup (Patches 10-16):
>    - Remove redundant partition-related operations
>    - Additional optimizations based on the new architecture
> 
> Key improvements:
> - Centralized management of partition-related fields (partition_root_state,
>   prs_err, nr_subparts, remote_sibling, effective_xcpus) within the
>   partition_enable/disable/update functions
> - Consistent operation patterns for both local and remote partitions
>   with type-specific validation checks
> - Fixed bug where isolcpus remained in root partition after isolated
>   partition transitioned to root
> 
> Chen Ridong (16):
>   cpuset: use update_partition_sd_lb in update_cpumasks_hier
>   cpuset: generalize validate_partition() interface
>   cpuset: factor out partition_enable() function
>   cpuset: factor out partition_disable() function
>   cpuset: factor out partition_update() function
>   cpuset: introduce local_partition_enable()
>   cpuset: introduce local_partition_disable()
>   cpuset: introduce local_partition_invalidate()
>   cpuset: introduce local_partition_update()
>   cpuset: remove redundant partition field updates
>   cpuset: simplify partition update logic for hotplug tasks
>   cpuset: unify local partition disable and invalidate
>   cpuset: use partition_disable for compute_partition_effective_cpumask
>   cpuset: fix isolcpus stay in root when isolated partition changes to
>     root
>   cpuset: use partition_disable for update_prstate
>   cpuset: remove prs_err clear when notify_partition_change
> 
>  kernel/cgroup/cpuset.c | 907 ++++++++++++++++++-----------------------
>  1 file changed, 408 insertions(+), 499 deletions(-)
> 

+cc cgroups@vger.kernel.org

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 00/16] cpuset: rework local partition logic
  2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
                   ` (16 preceding siblings ...)
  2025-09-28  9:57 ` [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
@ 2025-09-28 16:00 ` Waiman Long
  2025-09-29  1:17   ` Chen Ridong
  2025-10-17  1:05   ` Chen Ridong
  17 siblings, 2 replies; 45+ messages in thread
From: Waiman Long @ 2025-09-28 16:00 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgroups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> The current local partition implementation consolidates all operations
> (enable, disable, invalidate, and update) within the large
> update_parent_effective_cpumask() function, which exceeds 300 lines.
> This monolithic approach has become increasingly difficult to understand
> and maintain. Additionally, partition-related fields are updated in
> multiple locations, leading to redundant code and potential corner case
> oversights.
>
> This patch series refactors the local partition logic by separating
> operations into dedicated functions: local_partition_enable(),
> local_partition_disable(), and local_partition_update(), creating
> symmetry with the existing remote partition infrastructure.
>
> The series is organized as follows:
>
> 1. Infrastructure Preparation (Patches 1-2):
>     - Code cleanup and preparation for the refactoring work
>
> 2. Core Partition Operations (Patches 3-5):
>     - Factor out partition_enable(), partition_disable(), and
>       partition_update() functions from remote partition operations
>
> 3. Local Partition Implementation (Patches 6-9):
>     - Separate update_parent_effective_cpumask() into dedicated functions:
>       * local_partition_enable()
>       * local_partition_disable()
>       * local_partition_invalidate()
>       * local_partition_update()
>
> 4. Optimization and Cleanup (Patches 10-16):
>     - Remove redundant partition-related operations
>     - Additional optimizations based on the new architecture
>
> Key improvements:
> - Centralized management of partition-related fields (partition_root_state,
>    prs_err, nr_subparts, remote_sibling, effective_xcpus) within the
>    partition_enable/disable/update functions
> - Consistent operation patterns for both local and remote partitions
>    with type-specific validation checks
> - Fixed bug where isolcpus remained in root partition after isolated
>    partition transitioned to root

You are really active in restructuring the cpuset code. However, the 
next merge window for v6.18 is going to open later today or tomorrow. I 
will start reviewing this patch series once the merge window closes 2 
weeks later.

Cheers,
Longman

> Chen Ridong (16):
>    cpuset: use update_partition_sd_lb in update_cpumasks_hier
>    cpuset: generalize validate_partition() interface
>    cpuset: factor out partition_enable() function
>    cpuset: factor out partition_disable() function
>    cpuset: factor out partition_update() function
>    cpuset: introduce local_partition_enable()
>    cpuset: introduce local_partition_disable()
>    cpuset: introduce local_partition_invalidate()
>    cpuset: introduce local_partition_update()
>    cpuset: remove redundant partition field updates
>    cpuset: simplify partition update logic for hotplug tasks
>    cpuset: unify local partition disable and invalidate
>    cpuset: use partition_disable for compute_partition_effective_cpumask
>    cpuset: fix isolcpus stay in root when isolated partition changes to
>      root
>    cpuset: use partition_disable for update_prstate
>    cpuset: remove prs_err clear when notify_partition_change
>
>   kernel/cgroup/cpuset.c | 907 ++++++++++++++++++-----------------------
>   1 file changed, 408 insertions(+), 499 deletions(-)
>


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 00/16] cpuset: rework local partition logic
  2025-09-28 16:00 ` Waiman Long
@ 2025-09-29  1:17   ` Chen Ridong
  2025-10-17  1:05   ` Chen Ridong
  1 sibling, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-09-29  1:17 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgroups, linux-kernel, lujialin4, chenridong



On 2025/9/29 0:00, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> The current local partition implementation consolidates all operations
>> (enable, disable, invalidate, and update) within the large
>> update_parent_effective_cpumask() function, which exceeds 300 lines.
>> This monolithic approach has become increasingly difficult to understand
>> and maintain. Additionally, partition-related fields are updated in
>> multiple locations, leading to redundant code and potential corner case
>> oversights.
>>
>> This patch series refactors the local partition logic by separating
>> operations into dedicated functions: local_partition_enable(),
>> local_partition_disable(), and local_partition_update(), creating
>> symmetry with the existing remote partition infrastructure.
>>
>> The series is organized as follows:
>>
>> 1. Infrastructure Preparation (Patches 1-2):
>>     - Code cleanup and preparation for the refactoring work
>>
>> 2. Core Partition Operations (Patches 3-5):
>>     - Factor out partition_enable(), partition_disable(), and
>>       partition_update() functions from remote partition operations
>>
>> 3. Local Partition Implementation (Patches 6-9):
>>     - Separate update_parent_effective_cpumask() into dedicated functions:
>>       * local_partition_enable()
>>       * local_partition_disable()
>>       * local_partition_invalidate()
>>       * local_partition_update()
>>
>> 4. Optimization and Cleanup (Patches 10-16):
>>     - Remove redundant partition-related operations
>>     - Additional optimizations based on the new architecture
>>
>> Key improvements:
>> - Centralized management of partition-related fields (partition_root_state,
>>    prs_err, nr_subparts, remote_sibling, effective_xcpus) within the
>>    partition_enable/disable/update functions
>> - Consistent operation patterns for both local and remote partitions
>>    with type-specific validation checks
>> - Fixed bug where isolcpus remained in root partition after isolated
>>    partition transitioned to root
> 
> You are really active in restructuring the cpuset code. However, the next merge window for v6.18 is
> going to open later today or tomorrow. I will start reviewing this patch series once the merge
> window closes 2 weeks later.
> 
> Cheers,
> Longman
> 

Thank you for letting me know about your schedule.

I've been quite active in the cgroup, especially with cpuset, I believe. :)

I've been thinking about reworking this series for some time, and I finally got it done.
Looking forward to your review.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 00/16] cpuset: rework local partition logic
  2025-09-28 16:00 ` Waiman Long
  2025-09-29  1:17   ` Chen Ridong
@ 2025-10-17  1:05   ` Chen Ridong
  1 sibling, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-17  1:05 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgroups, linux-kernel, lujialin4, chenridong



On 2025/9/29 0:00, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> The current local partition implementation consolidates all operations
>> (enable, disable, invalidate, and update) within the large
>> update_parent_effective_cpumask() function, which exceeds 300 lines.
>> This monolithic approach has become increasingly difficult to understand
>> and maintain. Additionally, partition-related fields are updated in
>> multiple locations, leading to redundant code and potential corner case
>> oversights.
>>
>> This patch series refactors the local partition logic by separating
>> operations into dedicated functions: local_partition_enable(),
>> local_partition_disable(), and local_partition_update(), creating
>> symmetry with the existing remote partition infrastructure.
>>
>> The series is organized as follows:
>>
>> 1. Infrastructure Preparation (Patches 1-2):
>>     - Code cleanup and preparation for the refactoring work
>>
>> 2. Core Partition Operations (Patches 3-5):
>>     - Factor out partition_enable(), partition_disable(), and
>>       partition_update() functions from remote partition operations
>>
>> 3. Local Partition Implementation (Patches 6-9):
>>     - Separate update_parent_effective_cpumask() into dedicated functions:
>>       * local_partition_enable()
>>       * local_partition_disable()
>>       * local_partition_invalidate()
>>       * local_partition_update()
>>
>> 4. Optimization and Cleanup (Patches 10-16):
>>     - Remove redundant partition-related operations
>>     - Additional optimizations based on the new architecture
>>
>> Key improvements:
>> - Centralized management of partition-related fields (partition_root_state,
>>    prs_err, nr_subparts, remote_sibling, effective_xcpus) within the
>>    partition_enable/disable/update functions
>> - Consistent operation patterns for both local and remote partitions
>>    with type-specific validation checks
>> - Fixed bug where isolcpus remained in root partition after isolated
>>    partition transitioned to root
> 
> You are really active in restructuring the cpuset code. However, the next merge window for v6.18 is
> going to open later today or tomorrow. I will start reviewing this patch series once the merge
> window closes 2 weeks later.
> 

Hi Longman,

I noticed that v6.18-rc1 was tagged on October 12.
I’d really appreciate it if you could find some time to review this patch series when you’re available.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier
  2025-09-28  7:12 ` [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier Chen Ridong
@ 2025-10-20  2:37   ` Waiman Long
  2025-10-20  7:30     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  2:37 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> For cgroup v2, when a cpuset is not a valid partition root, it inherits
> the CS_SCHED_LOAD_BALANCE flag from its parent. The existing logic in
> update_cpumasks_hier() manually handled this inheritance condition.
>
> This patch replaces the inline implementation with a call to the dedicated
> update_partition_sd_lb() helper function, which already encapsulates the
> same logic. The helper function comprehensively handles both the load
> balance flag update and the necessary scheduling domain rebuild decision.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 16 +++-------------
>   1 file changed, 3 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 52468d2c178a..052f9e0c7a65 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1257,6 +1257,8 @@ static void update_partition_sd_lb(struct cpuset *cs, int old_prs)
>   	bool rebuild_domains = (new_prs > 0) || (old_prs > 0);
>   	bool new_lb;
>   
> +	if (!cpuset_v2())
> +		return;
>   	/*
>   	 * If cs is not a valid partition root, the load balance state
>   	 * will follow its parent.
> @@ -2276,19 +2278,7 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
>   			!cpumask_equal(cp->cpus_allowed, cp->effective_cpus));
>   
>   		cpuset_update_tasks_cpumask(cp, cp->effective_cpus);
> -
> -		/*
> -		 * On default hierarchy, inherit the CS_SCHED_LOAD_BALANCE
> -		 * from parent if current cpuset isn't a valid partition root
> -		 * and their load balance states differ.
> -		 */
> -		if (cpuset_v2() && !is_partition_valid(cp) &&
> -		    (is_sched_load_balance(parent) != is_sched_load_balance(cp))) {
> -			if (is_sched_load_balance(parent))
> -				set_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
> -			else
> -				clear_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
> -		}
> +		update_partition_sd_lb(cp, old_prs);
>   
>   		/*
>   		 * On legacy hierarchy, if the effective cpumask of any non-
Calling update_partition_sd_lb() directly from update_cpumasks_hier() 
may incorrectly force rebuidling sched domain when it is not really 
necessary.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function
  2025-09-28  7:12 ` [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function Chen Ridong
@ 2025-10-20  2:39   ` Waiman Long
  2025-10-20  7:48     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  2:39 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Extract the core partition enablement logic into a dedicated
> partition_enable() function. This refactoring centralizes updates to key
> cpuset data structures including remote_sibling, effective_xcpus,
> partition_root_state, and prs_err.
>
> The function handles the complete partition enablement workflow:
> - Adding exclusive CPUs via partition_xcpus_add()
> - Managing remote sibling relationships
> - Synchronizing effective exclusive CPUs mask
> - Updating partition state and error status
> - Triggering required scheduler domain rebuilds
>
> This creates a coherent interface for partition operations and establishes
> a foundation for future local partition support while maintaining existing
> remote partition behavior.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 55 +++++++++++++++++++++++++++++++++---------
>   1 file changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 0787904321a9..43ce62f4959c 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1515,6 +1515,49 @@ static inline bool is_local_partition(struct cpuset *cs)
>   	return is_partition_valid(cs) && !is_remote_partition(cs);
>   }
>   
> +static void partition_state_update(struct cpuset *cs, int new_prs,
> +					  enum prs_errcode prs_err)
> +{
> +	lockdep_assert_held(&callback_lock);
> +
> +	cs->partition_root_state = new_prs;
> +	WRITE_ONCE(cs->prs_err, prs_err);
> +	if (!is_partition_valid(cs))
> +		reset_partition_data(cs);
> +}
> +
> +/**
> + * partition_enable - Transitions a cpuset to a partition root
> + * @cs: The cpuset to enable partition for
> + * @parent: Parent cpuset of @cs, NULL for remote parent
> + * @new_prs: New partition root state to set
> + * @new_excpus: New exclusive CPUs mask for the partition
> + *
> + * Transitions a cpuset to a partition root, only for v2.
> + */
> +static void partition_enable(struct cpuset *cs, struct cpuset *parent,
> +				 int new_prs, struct cpumask *new_excpus)
> +{
> +	bool isolcpus_updated;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	WARN_ON_ONCE(new_prs <= 0);
> +	WARN_ON_ONCE(!cpuset_v2());
> +
> +	if (cs->partition_root_state == new_prs)
> +		return;
> +
> +	spin_lock_irq(&callback_lock);
> +	/* enable partition should only add exclusive cpus */
> +	isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
> +	list_add(&cs->remote_sibling, &remote_children);
> +	cpumask_copy(cs->effective_xcpus, new_excpus);
> +	partition_state_update(cs, new_prs, PERR_NONE);
> +	spin_unlock_irq(&callback_lock);
> +	update_unbound_workqueue_cpumask(isolcpus_updated);
> +	cpuset_force_rebuild();
> +}
> +
partition_enable() is supposed to be a common helper used for the 
creation of both local and remote partitions. The one in this patch does 
work for remote partition but not for local partition. I would prefer to 
make it good for both cases when you introduce it instead adding code in 
patch 6 to make it work for local partition later in the series. It will 
make it easier to review instead of jumping back and forth to make sure 
that it will do the right thing.

Cheers,
Longman



^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 05/16] cpuset: factor out partition_update() function
  2025-09-28  7:12 ` [PATCH -next RFC 05/16] cpuset: factor out partition_update() function Chen Ridong
@ 2025-10-20  2:43   ` Waiman Long
  2025-10-20  8:05     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  2:43 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Extract the core partition update logic into a dedicated partition_update()
> function. This refactoring centralizes updates to key cpuset data
> structures including remote_sibling, effective_xcpus, partition_root_state,
> and prs_err.
>
> The function handles the complete partition update workflow:
> - Adding and removing exclusive CPUs via partition_xcpus_add()/del()
> - Managing remote sibling relationships
> - Synchronizing effective exclusive CPUs mask
> - Updating partition state and error status
> - Triggering required system updates and workqueue synchronization
>
> This creates a coherent interface for partition operations and establishes
> a foundation for enhanced partition management while maintaining existing
> remote partition behavior.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 71 ++++++++++++++++++++++++++++--------------
>   1 file changed, 47 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 1944410ae872..0e2f95daf459 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1587,6 +1587,49 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
>   	cpuset_force_rebuild();
>   }
>   
> +/**
> + * partition_update - Update an existing partition configuration
> + * @cs: The cpuset to update
> + * @prs: Partition root state (must be positive)
> + * @xcpus: New exclusive CPUs mask for the partition (NULL to keep current)
> + * @excpus: New effective exclusive CPUs mask
> + * @tmp: Temporary masks
> + *
> + * Updates partition-related fields. The tmp->addmask is the CPU mask that
> + * will be added to the subpartitions_cpus and removed from parent's
> + * effective_cpus, and the tmp->delmask vice versa.
> + */
> +static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
> +				  struct cpumask *excpus, struct tmpmasks *tmp)
> +{
> +	bool isolcpus_updated;
> +	bool excl_updated;
> +	struct cpuset *parent;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	WARN_ON_ONCE(!cpuset_v2());
> +	WARN_ON_ONCE(prs <= 0);
> +
> +	parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
> +	excl_updated = !cpumask_empty(tmp->addmask) ||
> +			!cpumask_empty(tmp->delmask);
> +
> +	spin_lock_irq(&callback_lock);
> +	isolcpus_updated = partition_xcpus_add(prs, parent, tmp->addmask);
> +	isolcpus_updated |= partition_xcpus_del(prs, parent, tmp->delmask);

The current partition_xcpus_add/del() functions assume the given cpumas 
is non-empty. In the new partition_update() helper, you can pass an 
empty cpumask to them. This will cause useless work to be done. Also 
isolcpus_update may not be correct because of that causing unneeded work 
to be done in the workqueue code.

-Longman


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 06/16] cpuset: introduce local_partition_enable()
  2025-09-28  7:12 ` [PATCH -next RFC 06/16] cpuset: introduce local_partition_enable() Chen Ridong
@ 2025-10-20  2:44   ` Waiman Long
  2025-10-20  8:06     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  2:44 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> The partition_enable() function introduced in the previous patch can be
> reused to enable local partitions.
>
> First, partition_enable() was enhanced to support local partition enabling
> by properly handling parent's nr_subparts counter and adding notification
> operations.
>
> Then, the local_partition_enable() function is introduced, which factors
> out the local partition enablement logic from
> update_parent_effective_cpumask(). After passing local partition validation
> checks, it delegates to partition_enable() to complete the partition setup.
>
> This refactoring creates a clear separation between local and remote
> partition operations while maintaining code reuse through the shared
> partition_enable() infrastructure.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 111 +++++++++++++++++++++++++++--------------
>   1 file changed, 74 insertions(+), 37 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 0e2f95daf459..154992cdfe9a 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1539,6 +1539,7 @@ static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>   				 int new_prs, struct cpumask *new_excpus)
>   {
>   	bool isolcpus_updated;
> +	int old_prs;
>   
>   	lockdep_assert_held(&cpuset_mutex);
>   	WARN_ON_ONCE(new_prs <= 0);
> @@ -1547,15 +1548,21 @@ static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>   	if (cs->partition_root_state == new_prs)
>   		return;
>   
> +	old_prs = cs->partition_root_state;
>   	spin_lock_irq(&callback_lock);
>   	/* enable partition should only add exclusive cpus */
>   	isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
> -	list_add(&cs->remote_sibling, &remote_children);
> +	/* enable remote partition */
> +	if (!parent)
> +		list_add(&cs->remote_sibling, &remote_children);
> +	else if (!is_partition_valid(cs))
> +		parent->nr_subparts += 1;
>   	cpumask_copy(cs->effective_xcpus, new_excpus);
>   	partition_state_update(cs, new_prs, PERR_NONE);
>   	spin_unlock_irq(&callback_lock);
>   	update_unbound_workqueue_cpumask(isolcpus_updated);
>   	cpuset_force_rebuild();
> +	notify_partition_change(cs, old_prs);
>   }

As commented in an earlier patch, the partition_enable() chnage should 
be moved there.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 07/16] cpuset: introduce local_partition_disable()
  2025-09-28  7:12 ` [PATCH -next RFC 07/16] cpuset: introduce local_partition_disable() Chen Ridong
@ 2025-10-20  2:46   ` Waiman Long
  2025-10-20  8:06     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  2:46 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> The partition_disable() function introduced earlier can be extended to
> handle local partition disablement.
>
> First, partition_disable() was enhanced to support local partitions by
> properly managing the parent's nr_subparts counter and integrating
> notification operations.
>
> Then, local_partition_disable() is introduced, which extracts the local
> partition disable logic from update_parent_effective_cpumask(). It calls
> partition_disable() to complete the disablement process.
>
> This refactoring establishes a clear separation between local and remote
> partition operations while promoting code reuse through the shared
> partition_disable() infrastructure.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 60 ++++++++++++++++++++++++++++++------------
>   1 file changed, 43 insertions(+), 17 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 154992cdfe9a..87ba43e93540 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1576,13 +1576,20 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
>   				    int new_prs, enum prs_errcode prs_err)
>   {
>   	bool isolcpus_updated;
> +	int old_prs;
>   
>   	lockdep_assert_held(&cpuset_mutex);
>   	WARN_ON_ONCE(new_prs > 0);
>   	WARN_ON_ONCE(!cpuset_v2());
>   
> +	old_prs = cs->partition_root_state;
>   	spin_lock_irq(&callback_lock);
>   	list_del_init(&cs->remote_sibling);
> +	if (parent && is_partition_valid(parent) &&
> +	    is_partition_valid(cs)) {
> +		parent->nr_subparts -= 1;
> +		WARN_ON_ONCE(parent->nr_subparts < 0);
> +	}
>   	/* disable a partition should only delete exclusive cpus */
>   	isolcpus_updated = partition_xcpus_del(cs->partition_root_state,
>   						parent, cs->effective_xcpus);
> @@ -1592,6 +1599,9 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
>   	spin_unlock_irq(&callback_lock);
>   	update_unbound_workqueue_cpumask(isolcpus_updated);
>   	cpuset_force_rebuild();
> +	/* Clear exclusive flag; no errors are expected */
> +	update_partition_exclusive_flag(cs, new_prs);
> +	notify_partition_change(cs, old_prs);
>   }
>   

Similarly, change to partition_disable() should be done previously in 
patch 4 ("cpuset: factor out partition_disable() function") for 
completeness.

Cheers,
Longman



^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate()
  2025-09-28  7:12 ` [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate() Chen Ridong
@ 2025-10-20  2:48   ` Waiman Long
  2025-10-20  8:28     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  2:48 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Build on the partition_disable() infrastructure introduced in the previous
> patch to handle local partition invalidation.
>
> The local_partition_invalidate() function factors out the local partition
> invalidation logic from update_parent_effective_cpumask(), which delegates
> to partition_disable() to complete the invalidation process.
>
> Additionally, correct the transition logic in cpuset_hotplug_update_tasks()
> when determining whether to transition an invalid partition root, the check
> should be based on non-empty user_cpus rather than non-empty
> effective_xcpus. This correction addresses the scenario where
> exclusive_cpus is not set but cpus_allowed is configured - in this case,
> effective_xcpus may be empty even though the partition should be considered
> for re-enablement. The user_cpus-based check ensures proper partition state
> transitions under these conditions.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 66 +++++++++++++++++++++++++++---------------
>   1 file changed, 42 insertions(+), 24 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 87ba43e93540..e460d03286ba 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1911,6 +1911,39 @@ static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
>   	}
>   }
>   
> +/**
> + * local_partition_invalidate - Invalidate a local partition
> + * @cs: Target cpuset (local partition root) to invalidate
> + * @tmp: Temporary masks
> + */
> +static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
> +{
> +	struct cpumask *xcpus = user_xcpus(cs);
> +	struct cpuset *parent = parent_cs(cs);
> +	int new_prs = cs->partition_root_state;
> +	bool cpumask_updated = false;
> +
> +	lockdep_assert_held(&cpuset_mutex);
> +	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
> +
> +	if (is_partition_invalid(cs))
> +		return;
You should change the check to if (!is_partition_valid(cs)). You can 
avoid the case that partition_disable() is called with a member.

> +	/*
> +	 * Make the current partition invalid.
> +	 */
> +	if (is_partition_valid(parent))
> +		cpumask_updated = cpumask_and(tmp->addmask,
> +					      xcpus, parent->effective_xcpus);
> +	if (cs->partition_root_state > 0)
> +		new_prs = -cs->partition_root_state;
> +
> +	partition_disable(cs, parent, new_prs, cs->prs_err);
> +	if (cpumask_updated) {
> +		cpuset_update_tasks_cpumask(parent, tmp->addmask);
> +		update_sibling_cpumasks(parent, cs, tmp);
> +	}
> +}
> +
>   /**
>    * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
>    * @cs:      The cpuset that requests change in partition root state
> @@ -1972,23 +2005,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>   	adding = deleting = false;
>   	old_prs = new_prs = cs->partition_root_state;
>   
> -	if (cmd == partcmd_invalidate) {
> -		if (is_partition_invalid(cs))
> -			return 0;
> -
> -		/*
> -		 * Make the current partition invalid.
> -		 */
> -		if (is_partition_valid(parent))
> -			adding = cpumask_and(tmp->addmask,
> -					     xcpus, parent->effective_xcpus);
> -		if (old_prs > 0) {
> -			new_prs = -old_prs;
> -			subparts_delta--;
> -		}
> -		goto write_error;
> -	}
> -
>   	/*
>   	 * The parent must be a partition root.
>   	 * The new cpumask, if present, or the current cpus_allowed must
> @@ -2552,7 +2568,7 @@ static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset *trialc
>   			if (is_partition_valid(cp) &&
>   			    cpumask_intersects(xcpus, cp->effective_xcpus)) {
>   				rcu_read_unlock();
> -				update_parent_effective_cpumask(cp, partcmd_invalidate, NULL, tmp);
> +				local_partition_invalidate(cp, tmp);
>   				rcu_read_lock();
>   			}
>   		}
> @@ -2592,8 +2608,7 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>   					   trialcs->effective_xcpus, tmp);
>   	} else {
>   		if (trialcs->prs_err)
> -			update_parent_effective_cpumask(cs, partcmd_invalidate,
> -							NULL, tmp);
> +			local_partition_invalidate(cs, tmp);
>   		else
>   			update_parent_effective_cpumask(cs, partcmd_update,
>   							trialcs->effective_xcpus, tmp);
> @@ -4037,18 +4052,21 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks *tmp)
>   	 *    partitions.
>   	 */
>   	if (is_local_partition(cs) && (!is_partition_valid(parent) ||
> -				tasks_nocpu_error(parent, cs, &new_cpus)))
> +				tasks_nocpu_error(parent, cs, &new_cpus))) {
>   		partcmd = partcmd_invalidate;
> +		local_partition_invalidate(cs, tmp);
> +	}
>   	/*
>   	 * On the other hand, an invalid partition root may be transitioned
> -	 * back to a regular one with a non-empty effective xcpus.
> +	 * back to a regular one with a non-empty user xcpus.
>   	 */
>   	else if (is_partition_valid(parent) && is_partition_invalid(cs) &&
> -		 !cpumask_empty(cs->effective_xcpus))
> +		 !cpumask_empty(user_xcpus(cs))) {

I believe the effective_xcpus is not cleared currently when a partition 
is invalidated. Anyway, this change is also OK especially if 
effective_xcpus will be cleared in a later patch.

Cheers,
Longman

>   		partcmd = partcmd_update;
> +		update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
> +	}
>   
>   	if (partcmd >= 0) {
> -		update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
>   		if ((partcmd == partcmd_invalidate) || is_partition_valid(cs)) {
>   			compute_partition_effective_cpumask(cs, &new_cpus);
>   			cpuset_force_rebuild();


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 09/16] cpuset: introduce local_partition_update()
  2025-09-28  7:12 ` [PATCH -next RFC 09/16] cpuset: introduce local_partition_update() Chen Ridong
@ 2025-10-20  2:57   ` Waiman Long
  2025-10-20  9:24     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  2:57 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong


On 9/28/25 3:12 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Extend the partition_update() infrastructure to handle local partition
> updates.
>
> The local_partition_update() function replaces the command partcmd_update
> previously handled within update_parent_effective_cpumask(). The update
> logic follows a state-based approach:
>
> 1. Validation check: First verify if the local partition is currently valid
> 2. Invalidation handling: If the partition is invalid, trigger invalidation
> 3. State transition: If an invalid partition has no errors, transition to
>     valid
> 4. cpus updates: For local partition that only cpu maks changes, use
"cpu mask"?
>     partition_update() to handle partition change.
>
> With the introduction of this function, update_parent_effective_cpumask()
> function is removed, simplifying the partition update code path and
> creating a cleaner separation between local and remote partition
> operations.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 376 +++++++++++++----------------------------
>   1 file changed, 122 insertions(+), 254 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index e460d03286ba..d0217db04b69 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1622,12 +1622,14 @@ static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
>   	bool isolcpus_updated;
>   	bool excl_updated;
>   	struct cpuset *parent;
> +	int old_prs;
>   
>   	lockdep_assert_held(&cpuset_mutex);
>   	WARN_ON_ONCE(!cpuset_v2());
>   	WARN_ON_ONCE(prs <= 0);
>   
>   	parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
> +	old_prs = cs->partition_root_state;
>   	excl_updated = !cpumask_empty(tmp->addmask) ||
>   			!cpumask_empty(tmp->delmask);
>   
> @@ -1645,6 +1647,8 @@ static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
>   	update_unbound_workqueue_cpumask(isolcpus_updated);
>   	if (excl_updated)
>   		cpuset_force_rebuild();
> +	update_partition_exclusive_flag(cs, prs);
> +	notify_partition_change(cs, old_prs);
>   }
>   

Again, change to partition_update() should be done in the patch that 
introduces it.

>   /*
> @@ -1790,6 +1794,27 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask *new_cpus)
>   	return false;
>   }
>   
> +static bool cpuset_user_cpus_exclusive(struct cpuset *cs)

The cpuset prefix is only needed if it is an externally visible 
function. For this one, I think a better name should be 
"is_user_xcpus_exclusive".

> +{
> +	struct cpuset *parent = parent_cs(cs);
> +
> +	struct cgroup_subsys_state *css;
> +	struct cpuset *child;
> +	bool exclusive = true;
> +
> +	rcu_read_lock();
> +	cpuset_for_each_child(child, css, parent) {
> +		if (child == cs)
> +			continue;
> +		if (!cpusets_are_exclusive(cs, child)) {
> +			exclusive = false;
> +			break;
> +		}
> +	}
> +	rcu_read_unlock();
> +	return exclusive;
> +}
> +
>   /**
>    * validate_partition - Validate a cpuset partition configuration
>    * @cs: The cpuset to validate
> @@ -1818,6 +1843,39 @@ static enum prs_errcode validate_partition(struct cpuset *cs, int new_prs,
>   	return PERR_NONE;
>   }
>   
> +/**
> + * local_partition_check - Validate for local partition
> + * @cs: Target cpuset to validate
> + * @new_prs: New partition root state to validate
> + * @excpus: New exclusive effectuve CPUs mask to validate
> + * @excl_check: Flag to enable exclusive CPUs ownership validation
> + *
> + * Return: PERR_NONE if validation passes, appropriate error code otherwise
> + *
> + * Important: The caller must ensure that @cs's cpu mask is updated before
> + * invoking this function when exclusive CPU validation is required.
> + */
> +static enum prs_errcode local_partition_check(struct cpuset *cs, int new_prs,
> +							 struct cpumask *excpus, bool excl_check)

I would suggest naming it to "validate_local_partition()" as the local 
counterpart of validate_partition().

> +{
> +	struct cpuset *parent = parent_cs(cs);
> +
> +	/*
> +	 * The parent must be a partition root.
> +	 * The new cpumask, if present, or the current cpus_allowed must
> +	 * not be empty.
> +	 */
> +	if (!is_partition_valid(parent)) {
> +		return is_partition_invalid(parent)
> +			? PERR_INVPARENT : PERR_NOTPART;
> +	}
> +
> +	if (excl_check && !cpuset_user_cpus_exclusive(cs))
> +		return PERR_NOTEXCL;
> +
> +	return validate_partition(cs, new_prs, excpus);
> +}
> +
>   /**
>    * local_partition_enable - Enable local partition for a cpuset
>    * @cs: Target cpuset to become a local partition root
> @@ -1945,280 +2003,85 @@ static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
>   }
>   
>   /**
> - * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
> - * @cs:      The cpuset that requests change in partition root state
> - * @cmd:     Partition root state change command
> - * @newmask: Optional new cpumask for partcmd_update
> - * @tmp:     Temporary addmask and delmask
> - * Return:   0 or a partition root state error code
> - *
> - * For partcmd_enable*, the cpuset is being transformed from a non-partition
> - * root to a partition root. The effective_xcpus (cpus_allowed if
> - * effective_xcpus not set) mask of the given cpuset will be taken away from
> - * parent's effective_cpus. The function will return 0 if all the CPUs listed
> - * in effective_xcpus can be granted or an error code will be returned.
> - *
> - * For partcmd_disable, the cpuset is being transformed from a partition
> - * root back to a non-partition root. Any CPUs in effective_xcpus will be
> - * given back to parent's effective_cpus. 0 will always be returned.
> + * __local_partition_update - Update local CPU partition configuration
> + * @cs: Target cpuset to update
> + * @xcpus: New exclusive CPU mask
> + * @excpus: New effective exclusive CPU mask
> + * @tmp: Temporary mask storage for intermediate calculations
> + * @excl_check: Flag to enable exclusivity validation
>    *
> - * For partcmd_update, if the optional newmask is specified, the cpu list is
> - * to be changed from effective_xcpus to newmask. Otherwise, effective_xcpus is
> - * assumed to remain the same. The cpuset should either be a valid or invalid
> - * partition root. The partition root state may change from valid to invalid
> - * or vice versa. An error code will be returned if transitioning from
> - * invalid to valid violates the exclusivity rule.
> + * Handles updates to local CPU partition configurations by validating
> + * changes, managing state transitions, and propagating updates through
> + * the cpuset hierarchy.
>    *
> - * For partcmd_invalidate, the current partition will be made invalid.
> + * Note on exclusivity checking: Exclusivity validation is required when
> + * transitioning from an invalid to valid partition state. However, when
> + * updating cpus_allowed or exclusive_cpus, exclusivity should have already
> + * been verified by validate_change(). In such cases, excl_check must be
> + * false since the cs cpumasks are not yet updated.
>    *
> - * The partcmd_enable* and partcmd_disable commands are used by
> - * update_prstate(). An error code may be returned and the caller will check
> - * for error.
> - *
> - * The partcmd_update command is used by update_cpumasks_hier() with newmask
> - * NULL and update_cpumask() with newmask set. The partcmd_invalidate is used
> - * by update_cpumask() with NULL newmask. In both cases, the callers won't
> - * check for error and so partition_root_state and prs_err will be updated
> - * directly.
> + * Return: Partition error code (PERR_NONE indicates success)
>    */
> -static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
> -					   struct cpumask *newmask,
> -					   struct tmpmasks *tmp)
Separate out the removal of update_parent_effective_cpumask() into its 
own patch as intermixing the removal of this code and new code make it 
harder to review.

> +static int __local_partition_update(struct cpuset *cs, struct cpumask *xcpus,
> +				  struct cpumask *excpus, struct tmpmasks *tmp,
> +				  bool excl_check)
>   {
>   	struct cpuset *parent = parent_cs(cs);
> -	int adding;	/* Adding cpus to parent's effective_cpus	*/
> -	int deleting;	/* Deleting cpus from parent's effective_cpus	*/
> -	int old_prs, new_prs;
>   	int part_error = PERR_NONE;	/* Partition error? */
> -	int subparts_delta = 0;
> -	int isolcpus_updated = 0;
> -	struct cpumask *xcpus = user_xcpus(cs);
> -	bool nocpu;
> +	int old_prs, new_prs;
> +	bool cpumask_updated = false;
>   
>   	lockdep_assert_held(&cpuset_mutex);
> -	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
> +	/* For local partition only */
> +	if (WARN_ON_ONCE(is_remote_partition(cs) || cs_is_member(cs)))
> +		return PERR_NONE;
>   
> +	old_prs = cs->partition_root_state;
>   	/*
> -	 * new_prs will only be changed for the partcmd_update and
> -	 * partcmd_invalidate commands.
> +	 * If new_prs < 0, it might transition to valid partition state.
> +	 * Use absolute value for validation checks.
>   	 */
> -	adding = deleting = false;
> -	old_prs = new_prs = cs->partition_root_state;
> -
> -	/*
> -	 * The parent must be a partition root.
> -	 * The new cpumask, if present, or the current cpus_allowed must
> -	 * not be empty.
> -	 */
> -	if (!is_partition_valid(parent)) {
> -		return is_partition_invalid(parent)
> -		       ? PERR_INVPARENT : PERR_NOTPART;
> -	}
> -	if (!newmask && xcpus_empty(cs))
> -		return PERR_CPUSEMPTY;
> -
> -	nocpu = tasks_nocpu_error(parent, cs, xcpus);
> -
> -	if (newmask) {
> -		/*
> -		 * Empty cpumask is not allowed
> -		 */
> -		if (cpumask_empty(newmask)) {
> -			part_error = PERR_CPUSEMPTY;
> -			goto write_error;
> -		}
> -
> -		/* Check newmask again, whether cpus are available for parent/cs */
> -		nocpu |= tasks_nocpu_error(parent, cs, newmask);
> -
> -		/*
> -		 * partcmd_update with newmask:
> -		 *
> -		 * Compute add/delete mask to/from effective_cpus
> -		 *
> -		 * For valid partition:
> -		 *   addmask = exclusive_cpus & ~newmask
> -		 *			      & parent->effective_xcpus
> -		 *   delmask = newmask & ~exclusive_cpus
> -		 *		       & parent->effective_xcpus
> -		 *
> -		 * For invalid partition:
> -		 *   delmask = newmask & parent->effective_xcpus
> -		 */
> -		if (is_partition_invalid(cs)) {
> -			adding = false;
> -			deleting = cpumask_and(tmp->delmask,
> -					newmask, parent->effective_xcpus);
> -		} else {
> -			cpumask_andnot(tmp->addmask, xcpus, newmask);
> -			adding = cpumask_and(tmp->addmask, tmp->addmask,
> -					     parent->effective_xcpus);
> -
> -			cpumask_andnot(tmp->delmask, newmask, xcpus);
> -			deleting = cpumask_and(tmp->delmask, tmp->delmask,
> -					       parent->effective_xcpus);
> -		}
> -		/*
> -		 * The new CPUs to be removed from parent's effective CPUs
> -		 * must be present.
> -		 */
> -		if (deleting) {
> -			cpumask_and(tmp->new_cpus, tmp->delmask, cpu_active_mask);
> -			WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
> -		}
> -
> -		/*
> -		 * Make partition invalid if parent's effective_cpus could
> -		 * become empty and there are tasks in the parent.
> -		 */
> -		if (nocpu && (!adding ||
> -		    !cpumask_intersects(tmp->addmask, cpu_active_mask))) {
> -			part_error = PERR_NOCPUS;
> -			deleting = false;
> -			adding = cpumask_and(tmp->addmask,
> -					     xcpus, parent->effective_xcpus);
> -		}
> -	} else {
> -		/*
> -		 * partcmd_update w/o newmask
> -		 *
> -		 * delmask = effective_xcpus & parent->effective_cpus
> -		 *
> -		 * This can be called from:
> -		 * 1) update_cpumasks_hier()
> -		 * 2) cpuset_hotplug_update_tasks()
> -		 *
> -		 * Check to see if it can be transitioned from valid to
> -		 * invalid partition or vice versa.
> -		 *
> -		 * A partition error happens when parent has tasks and all
> -		 * its effective CPUs will have to be distributed out.
> -		 */
> -		if (nocpu) {
> -			part_error = PERR_NOCPUS;
> -			if (is_partition_valid(cs))
> -				adding = cpumask_and(tmp->addmask,
> -						xcpus, parent->effective_xcpus);
> -		} else if (is_partition_invalid(cs) && !cpumask_empty(xcpus) &&
> -			   cpumask_subset(xcpus, parent->effective_xcpus)) {
> -			struct cgroup_subsys_state *css;
> -			struct cpuset *child;
> -			bool exclusive = true;
> -
> -			/*
> -			 * Convert invalid partition to valid has to
> -			 * pass the cpu exclusivity test.
> -			 */
> -			rcu_read_lock();
> -			cpuset_for_each_child(child, css, parent) {
> -				if (child == cs)
> -					continue;
> -				if (!cpusets_are_exclusive(cs, child)) {
> -					exclusive = false;
> -					break;
> -				}
> -			}
> -			rcu_read_unlock();
> -			if (exclusive)
> -				deleting = cpumask_and(tmp->delmask,
> -						xcpus, parent->effective_cpus);
> -			else
> -				part_error = PERR_NOTEXCL;
> -		}
> -	}
> -
> -write_error:
> -	if (part_error)
> -		WRITE_ONCE(cs->prs_err, part_error);
> -
> -	if (cmd == partcmd_update) {
> -		/*
> -		 * Check for possible transition between valid and invalid
> -		 * partition root.
> -		 */
> -		switch (cs->partition_root_state) {
> -		case PRS_ROOT:
> -		case PRS_ISOLATED:
> -			if (part_error) {
> -				new_prs = -old_prs;
> -				subparts_delta--;
> -			}
> -			break;
> -		case PRS_INVALID_ROOT:
> -		case PRS_INVALID_ISOLATED:
> -			if (!part_error) {
> -				new_prs = -old_prs;
> -				subparts_delta++;
> -			}
> -			break;
> -		}
> +	new_prs = old_prs < 0 ? -old_prs : old_prs;
> +	part_error = local_partition_check(cs, new_prs, excpus, excl_check);
> +	if (part_error) {
> +		local_partition_invalidate(cs, tmp);

local_partition_invalidate() should only called if old_prs > 0.

> +		return part_error;
>   	}
>   
> -	if (!adding && !deleting && (new_prs == old_prs))
> -		return 0;
> +	/* Nothing changes, return PERR_NONE */
> +	if (new_prs == old_prs && cpumask_equal(excpus, cs->effective_xcpus))
> +		return PERR_NONE;
>   
>   	/*
> -	 * Transitioning between invalid to valid or vice versa may require
> -	 * changing CS_CPU_EXCLUSIVE. In the case of partcmd_update,
> -	 * validate_change() has already been successfully called and
> -	 * CPU lists in cs haven't been updated yet. So defer it to later.
> +	 * If partition was previously invalid but now passes checks,
> +	 * enable it and update related flags
>   	 */
> -	if ((old_prs != new_prs) && (cmd != partcmd_update))  {
> -		int err = update_partition_exclusive_flag(cs, new_prs);
> -
> -		if (err)
> -			return err;
> +	if (is_partition_invalid(cs) && !part_error) {
The !part_error check should be unnecessary as this path will not be 
reached if part_error is non-zero.

> +		partition_enable(cs, parent, new_prs, excpus);
> +		update_partition_exclusive_flag(cs, new_prs);
> +		update_partition_sd_lb(cs, old_prs);
> +		return part_error;
Just return PERR_NONE if it is not expected to be set.

>   	}
>   
> +	cpumask_updated = cpumask_andnot(tmp->addmask, excpus, cs->effective_xcpus);
> +	cpumask_updated |= cpumask_andnot(tmp->delmask, cs->effective_xcpus, excpus);
> +	partition_update(cs, new_prs, xcpus, excpus, tmp);
>   	/*
> -	 * Change the parent's effective_cpus & effective_xcpus (top cpuset
> -	 * only).
> -	 *
> -	 * Newly added CPUs will be removed from effective_cpus and
> -	 * newly deleted ones will be added back to effective_cpus.
> -	 */
> -	spin_lock_irq(&callback_lock);
> -	if (old_prs != new_prs) {
> -		cs->partition_root_state = new_prs;
> -		if (new_prs <= 0)
> -			cs->nr_subparts = 0;
> -	}
> -	/*
> -	 * Adding to parent's effective_cpus means deletion CPUs from cs
> -	 * and vice versa.
> +	 * Propagate changes in parent's effective_cpus down the hierarchy.
>   	 */
> -	if (adding)
> -		isolcpus_updated += partition_xcpus_del(old_prs, parent,
> -							tmp->addmask);
> -	if (deleting)
> -		isolcpus_updated += partition_xcpus_add(new_prs, parent,
> -							tmp->delmask);
> -
> -	if (is_partition_valid(parent)) {
> -		parent->nr_subparts += subparts_delta;
> -		WARN_ON_ONCE(parent->nr_subparts < 0);
> -	}
> -	spin_unlock_irq(&callback_lock);
> -	update_unbound_workqueue_cpumask(isolcpus_updated);
> -
> -	if ((old_prs != new_prs) && (cmd == partcmd_update))
> -		update_partition_exclusive_flag(cs, new_prs);
> -
> -	if (adding || deleting) {
> +	if (cpumask_updated) {
>   		cpuset_update_tasks_cpumask(parent, tmp->addmask);
>   		update_sibling_cpumasks(parent, cs, tmp);
>   	}
> +	return part_error;

Ditto.

Cheers,
Longman



^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 11/16] cpuset: simplify partition update logic for hotplug tasks
  2025-09-28  7:13 ` [PATCH -next RFC 11/16] cpuset: simplify partition update logic for hotplug tasks Chen Ridong
@ 2025-10-20  3:00   ` Waiman Long
  2025-10-20  8:44     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  3:00 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong


On 9/28/25 3:13 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Simplify the partition update logic in cpuset_hotplug_update_tasks() by
> calling the unified local_partition_update() interface.
>
> For local partitions, the previous patch introduced local_partition_update
> which handles both validation state transitions:
> - Invalidates local partitions that fail validation checks
> - Transitions invalid partitions to valid state when no errors are detected
>
> This eliminates the need for separate transition logic
> in cpuset_hotplug_update_tasks(), which can now simply call
> local_partition_update() to handle all local partition changes.
>
> This patch simplifies the logic by always proceeding to update_tasks for
> remote partitions, regardless of whether they were disabled or not. Since
> the original code didn't perform any meaningful operations for non-disabled
> remote partitions, this change should not affect functionality.
>
> The partition_cmd mechanism can now be safely removed as it is no longer

It is partition_cmd enum type.

> referenced by any code paths after the partition update logic
> simplification.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 67 ++++++++++++++++--------------------------
>   1 file changed, 26 insertions(+), 41 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 9e98df542715..a1896a199c8b 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1211,17 +1211,6 @@ static void compute_effective_cpumask(struct cpumask *new_cpus,
>   	cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
>   }
>   
> -/*
> - * Commands for update_parent_effective_cpumask
> - */
> -enum partition_cmd {
> -	partcmd_enable,		/* Enable partition root	  */
> -	partcmd_enablei,	/* Enable isolated partition root */
> -	partcmd_disable,	/* Disable partition root	  */
> -	partcmd_update,		/* Update parent's effective_cpus */
> -	partcmd_invalidate,	/* Make partition invalid	  */
> -};
> -
>   static void update_sibling_cpumasks(struct cpuset *parent, struct cpuset *cs,
>   				    struct tmpmasks *tmp);
>   
> @@ -2062,6 +2051,9 @@ static int __local_partition_update(struct cpuset *cs, struct cpumask *xcpus,
>   		update_partition_sd_lb(cs, old_prs);
>   		return part_error;
>   	}
> +	/* Nothing changes, return PERR_NONE */
> +	if (new_prs == old_prs && cpumask_equal(excpus, cs->effective_xcpus))
> +		return PERR_NONE;
I believe you already have this check added when you introduce 
__local_partition_update() in patch 9. It is a duplicate.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask
  2025-09-28  7:13 ` [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask Chen Ridong
@ 2025-10-20  3:02   ` Waiman Long
  2025-10-20  8:47     ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20  3:02 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong


On 9/28/25 3:13 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> Replace the partition invalidation logic in the
> compute_partition_effective_cpumask() with a call to partition_disable().
>
> This centralizes partition state management and ensures consistent
> handling of partition disable operations throughout the cpuset subsystem.
>
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 28 +++++++---------------------
>   1 file changed, 7 insertions(+), 21 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 6625b803ba02..20288dbd6ccf 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -170,15 +170,6 @@ static inline bool cs_is_member(const struct cpuset *cs)
>   	return cs->partition_root_state == PRS_MEMBER;
>   }
>   
> -/*
> - * Callers should hold callback_lock to modify partition_root_state.
> - */
> -static inline void make_partition_invalid(struct cpuset *cs)
> -{
> -	if (cs->partition_root_state > 0)
> -		cs->partition_root_state = -cs->partition_root_state;
> -}
> -
>   /*
>    * Send notification event of whenever partition_root_state changes.
>    */
> @@ -2073,6 +2064,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   	struct cgroup_subsys_state *css;
>   	struct cpuset *child;
>   	bool populated = partition_is_populated(cs, NULL);
> +	enum prs_errcode prs_err;
>   
>   	/*
>   	 * Check child partition roots to see if they should be
> @@ -2095,26 +2087,20 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>   		 * partition root.
>   		 */
>   		WARN_ON_ONCE(is_remote_partition(child));
> -		child->prs_err = 0;
> +		prs_err = 0;
>   		if (!cpumask_subset(child->effective_xcpus,
>   				    cs->effective_xcpus))
> -			child->prs_err = PERR_INVCPUS;
> +			prs_err = PERR_INVCPUS;
>   		else if (populated &&
>   			 cpumask_subset(new_ecpus, child->effective_xcpus))
> -			child->prs_err = PERR_NOCPUS;
> -
> -		if (child->prs_err) {
> -			int old_prs = child->partition_root_state;
> +			prs_err = PERR_NOCPUS;
>   
> +		if (prs_err) {
>   			/*
>   			 * Invalidate child partition
>   			 */
> -			spin_lock_irq(&callback_lock);
> -			make_partition_invalid(child);
> -			cs->nr_subparts--;
> -			child->nr_subparts = 0;
> -			spin_unlock_irq(&callback_lock);
> -			notify_partition_change(child, old_prs);
> +			partition_disable(child, parent_cs(child),

The parent of child should just be cs. You don't need to use parent_cs() 
to get it.

Cheers,
Longman


> +					  -child->partition_root_state, prs_err);
>   			continue;
>   		}
>   		cpumask_andnot(new_ecpus, new_ecpus,


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root
  2025-09-28  7:13 ` [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root Chen Ridong
@ 2025-10-20  3:06   ` Waiman Long
  2025-10-20  9:13     ` Chen Ridong
  2025-10-22 10:49     ` Chen Ridong
  0 siblings, 2 replies; 45+ messages in thread
From: Waiman Long @ 2025-10-20  3:06 UTC (permalink / raw)
  To: Chen Ridong, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 9/28/25 3:13 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
>
> A bug was detected with the following steps:
>
>    # cd /sys/fs/cgroup/
>    # mkdir test
>    # echo 9 > test/cpuset.cpus
>    # echo isolated > test/cpuset.cpus.partition
>    # cat test/cpuset.cpus.partition
>    isolated
>    # cat test/cpuset.cpus
>    9
>    # echo root > test/cpuset.cpus.partition
>    # cat test/cpuset.cpus
>    9
>    # cat test/cpuset.cpus.partition
>    root
>
> CPU 9 was initially placed in an isolated partition. When the partition
> type is changed from isolated to root, CPU 9 remains in what becomes a
> valid root partition. This violates the rule that isolcpus can only be
> assigned to isolated partitions.

I am a bit confused at the beginning about this as it does not clearly 
state that CPU 9 was listed in the "isolcpus" boot command line 
parameter, but I believe this is what you mean here. Yes, there is a 
restriction that a boot time isolcpus CPU cannot be put into a 
non-isolated partition, though that will likely to be relaxed in the 
near future.

Anyway, it is a real corner case. I also don't believe commit 
f28e22441f35 is the one that introduced this issue as the restriction 
was added later on via commit 4a74e418881f ("cgroup/cpuset: Check 
partition conflict with housekeeping setup").

As you have added a Fixes tag, it should be moved to the front of the 
series as it is likely to be backported to stable. Putting it near the 
end of a series with a lot of changes in between will make it harder to 
backport to the stable kernels.

> Fix the issue by re-enabling partition validation, which performs
> comprehensive partition error checking. In the scenario described above,
> this change causes the operation to fail with housekeeping conflicts,
> preventing the invalid configuration.
 From the code diff below, I don't know how you re-enable partition 
validation.

>
> Additionally, when enable a local partition, the warning for tmp->addmask
> not being a subset of parent's effective CPUs was removed. This warning was
> triggered during local partition re-enablement because the CPUs were
> already added to exclusive_cpus during the previous enable operation. The
> subset check is not applicable in this re-enablement scenario.

That should be in the new code that you introduce in this series. So it 
either be integrated into one of your earlier patches or be separated 
out as a separate patch without the Fixes tag as it is not applicable 
for the stable releases.

Cheers,
Longman

>
> Fixes: f28e22441f35 ("cgroup/cpuset: Add a new isolated cpus.partition type")
> Signed-off-by: Chen Ridong <chenridong@huawei.com>
> ---
>   kernel/cgroup/cpuset.c | 35 +++++++++--------------------------
>   1 file changed, 9 insertions(+), 26 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 20288dbd6ccf..2aaa688c596f 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1873,6 +1873,7 @@ static int local_partition_enable(struct cpuset *cs,
>   {
>   	struct cpuset *parent = parent_cs(cs);
>   	enum prs_errcode part_error;
> +	bool cpumask_updated = false;
>   
>   	lockdep_assert_held(&cpuset_mutex);
>   	WARN_ON_ONCE(is_remote_partition(cs));	/* For local partition only */
> @@ -1899,22 +1900,14 @@ static int local_partition_enable(struct cpuset *cs,
>   	if (part_error)
>   		return part_error;
>   
> -	/*
> -	 * This function will only be called when all the preliminary
> -	 * checks have passed. At this point, the following condition
> -	 * should hold.
> -	 *
> -	 * (cs->effective_xcpus & cpu_active_mask) ⊆ parent->effective_cpus
> -	 *
> -	 * Warn if it is not the case.
> -	 * addmask is used as temporary variable.
> -	 */
> -	cpumask_and(tmp->addmask, tmp->new_cpus, cpu_active_mask);
> -	WARN_ON_ONCE(!cpumask_subset(tmp->addmask, parent->effective_cpus));
> +	cpumask_updated = cpumask_andnot(tmp->addmask, tmp->new_cpus,
> +					 parent->effective_cpus);
>   	partition_enable(cs, parent, new_prs, tmp->new_cpus);
>   
> -	cpuset_update_tasks_cpumask(parent, tmp->addmask);
> -	update_sibling_cpumasks(parent, cs, tmp);
> +	if (cpumask_updated) {
> +		cpuset_update_tasks_cpumask(parent, tmp->addmask);
> +		update_sibling_cpumasks(parent, cs, tmp);
> +	}
>   	return 0;
>   }
>   
> @@ -2902,7 +2895,6 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   	int err = PERR_NONE, old_prs = cs->partition_root_state;
>   	struct cpuset *parent = parent_cs(cs);
>   	struct tmpmasks tmpmask;
> -	bool isolcpus_updated = false;
>   
>   	if (old_prs == new_prs)
>   		return 0;
> @@ -2920,7 +2912,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   	if (err)
>   		goto out;
>   
> -	if (!old_prs) {
> +	if (new_prs > 0) {
>   		/*
>   		 * cpus_allowed and exclusive_cpus cannot be both empty.
>   		 */
> @@ -2950,12 +2942,6 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   			err = local_partition_enable(cs, new_prs, &tmpmask);
>   		else
>   			err = remote_partition_enable(cs, new_prs, &tmpmask);
> -	} else if (old_prs && new_prs) {
> -		/*
> -		 * A change in load balance state only, no change in cpumasks.
> -		 * Need to update isolated_cpus.
> -		 */
> -		isolcpus_updated = true;
>   	} else {
>   		/*
>   		 * Switching back to member is always allowed even if it
> @@ -2985,16 +2971,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>   	WRITE_ONCE(cs->prs_err, err);
>   	if (!is_partition_valid(cs))
>   		reset_partition_data(cs);
> -	else if (isolcpus_updated)
> -		isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>   	spin_unlock_irq(&callback_lock);
> -	update_unbound_workqueue_cpumask(isolcpus_updated);
>   
>   	/* Force update if switching back to member & update effective_xcpus */
>   	update_cpumasks_hier(cs, &tmpmask, !new_prs);
>   
>   	/* A newly created partition must have effective_xcpus set */
> -	WARN_ON_ONCE(!old_prs && (new_prs > 0)
> +	WARN_ON_ONCE(!old_prs && (cs->partition_root_state > 0)
>   			      && cpumask_empty(cs->effective_xcpus));
>   
>   	/* Update sched domains and load balance flag */


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier
  2025-10-20  2:37   ` Waiman Long
@ 2025-10-20  7:30     ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  7:30 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 10:37, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> For cgroup v2, when a cpuset is not a valid partition root, it inherits
>> the CS_SCHED_LOAD_BALANCE flag from its parent. The existing logic in
>> update_cpumasks_hier() manually handled this inheritance condition.
>>
>> This patch replaces the inline implementation with a call to the dedicated
>> update_partition_sd_lb() helper function, which already encapsulates the
>> same logic. The helper function comprehensively handles both the load
>> balance flag update and the necessary scheduling domain rebuild decision.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 16 +++-------------
>>   1 file changed, 3 insertions(+), 13 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 52468d2c178a..052f9e0c7a65 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1257,6 +1257,8 @@ static void update_partition_sd_lb(struct cpuset *cs, int old_prs)
>>       bool rebuild_domains = (new_prs > 0) || (old_prs > 0);
>>       bool new_lb;
>>   +    if (!cpuset_v2())
>> +        return;
>>       /*
>>        * If cs is not a valid partition root, the load balance state
>>        * will follow its parent.
>> @@ -2276,19 +2278,7 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
>>               !cpumask_equal(cp->cpus_allowed, cp->effective_cpus));
>>             cpuset_update_tasks_cpumask(cp, cp->effective_cpus);
>> -
>> -        /*
>> -         * On default hierarchy, inherit the CS_SCHED_LOAD_BALANCE
>> -         * from parent if current cpuset isn't a valid partition root
>> -         * and their load balance states differ.
>> -         */
>> -        if (cpuset_v2() && !is_partition_valid(cp) &&
>> -            (is_sched_load_balance(parent) != is_sched_load_balance(cp))) {
>> -            if (is_sched_load_balance(parent))
>> -                set_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
>> -            else
>> -                clear_bit(CS_SCHED_LOAD_BALANCE, &cp->flags);
>> -        }
>> +        update_partition_sd_lb(cp, old_prs);
>>             /*
>>            * On legacy hierarchy, if the effective cpumask of any non-
> Calling update_partition_sd_lb() directly from update_cpumasks_hier() may incorrectly force
> rebuidling sched domain when it is not really necessary.
> 
> Cheers,
> Longman

Thank you Longman,

You are correct. Besides the change for the CS_SCHED_LOAD_BALANCE flag, which can lead to rebuilding
the sched domain, the condition of new_prs or old_prs being greater than 0 may also trigger a domain
rebuild, which is unnecessary. I will drop this patch in next version.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function
  2025-10-20  2:39   ` Waiman Long
@ 2025-10-20  7:48     ` Chen Ridong
  2025-10-20 19:42       ` Waiman Long
  0 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  7:48 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 10:39, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Extract the core partition enablement logic into a dedicated
>> partition_enable() function. This refactoring centralizes updates to key
>> cpuset data structures including remote_sibling, effective_xcpus,
>> partition_root_state, and prs_err.
>>
>> The function handles the complete partition enablement workflow:
>> - Adding exclusive CPUs via partition_xcpus_add()
>> - Managing remote sibling relationships
>> - Synchronizing effective exclusive CPUs mask
>> - Updating partition state and error status
>> - Triggering required scheduler domain rebuilds
>>
>> This creates a coherent interface for partition operations and establishes
>> a foundation for future local partition support while maintaining existing
>> remote partition behavior.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 55 +++++++++++++++++++++++++++++++++---------
>>   1 file changed, 44 insertions(+), 11 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 0787904321a9..43ce62f4959c 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1515,6 +1515,49 @@ static inline bool is_local_partition(struct cpuset *cs)
>>       return is_partition_valid(cs) && !is_remote_partition(cs);
>>   }
>>   +static void partition_state_update(struct cpuset *cs, int new_prs,
>> +                      enum prs_errcode prs_err)
>> +{
>> +    lockdep_assert_held(&callback_lock);
>> +
>> +    cs->partition_root_state = new_prs;
>> +    WRITE_ONCE(cs->prs_err, prs_err);
>> +    if (!is_partition_valid(cs))
>> +        reset_partition_data(cs);
>> +}
>> +
>> +/**
>> + * partition_enable - Transitions a cpuset to a partition root
>> + * @cs: The cpuset to enable partition for
>> + * @parent: Parent cpuset of @cs, NULL for remote parent
>> + * @new_prs: New partition root state to set
>> + * @new_excpus: New exclusive CPUs mask for the partition
>> + *
>> + * Transitions a cpuset to a partition root, only for v2.
>> + */
>> +static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>> +                 int new_prs, struct cpumask *new_excpus)
>> +{
>> +    bool isolcpus_updated;
>> +
>> +    lockdep_assert_held(&cpuset_mutex);
>> +    WARN_ON_ONCE(new_prs <= 0);
>> +    WARN_ON_ONCE(!cpuset_v2());
>> +
>> +    if (cs->partition_root_state == new_prs)
>> +        return;
>> +
>> +    spin_lock_irq(&callback_lock);
>> +    /* enable partition should only add exclusive cpus */
>> +    isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
>> +    list_add(&cs->remote_sibling, &remote_children);
>> +    cpumask_copy(cs->effective_xcpus, new_excpus);
>> +    partition_state_update(cs, new_prs, PERR_NONE);
>> +    spin_unlock_irq(&callback_lock);
>> +    update_unbound_workqueue_cpumask(isolcpus_updated);
>> +    cpuset_force_rebuild();
>> +}
>> +
> partition_enable() is supposed to be a common helper used for the creation of both local and remote
> partitions. The one in this patch does work for remote partition but not for local partition. I
> would prefer to make it good for both cases when you introduce it instead adding code in patch 6 to
> make it work for local partition later in the series. It will make it easier to review instead of
> jumping back and forth to make sure that it will do the right thing.
> 
> Cheers,
> Longman
> 

Thank you, Longman.

My original intention was to keep the changes easier to review. Patches 3–5 are meant to be pure
refactoring moves of code from the remote partition logic, without altering any behavior.

Would it be clearer to proceed in the following stages:

1. Introduce partition_enable(), partition_disable(), and partition_update() with their complete
logic first.
2. Replace the corresponding logic in remote partitions with these new helpers.
3. Then, replace the logic in local partitions with the same helpers.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 05/16] cpuset: factor out partition_update() function
  2025-10-20  2:43   ` Waiman Long
@ 2025-10-20  8:05     ` Chen Ridong
  2025-10-20 19:45       ` Waiman Long
  0 siblings, 1 reply; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  8:05 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 10:43, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Extract the core partition update logic into a dedicated partition_update()
>> function. This refactoring centralizes updates to key cpuset data
>> structures including remote_sibling, effective_xcpus, partition_root_state,
>> and prs_err.
>>
>> The function handles the complete partition update workflow:
>> - Adding and removing exclusive CPUs via partition_xcpus_add()/del()
>> - Managing remote sibling relationships
>> - Synchronizing effective exclusive CPUs mask
>> - Updating partition state and error status
>> - Triggering required system updates and workqueue synchronization
>>
>> This creates a coherent interface for partition operations and establishes
>> a foundation for enhanced partition management while maintaining existing
>> remote partition behavior.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 71 ++++++++++++++++++++++++++++--------------
>>   1 file changed, 47 insertions(+), 24 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 1944410ae872..0e2f95daf459 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1587,6 +1587,49 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
>>       cpuset_force_rebuild();
>>   }
>>   +/**
>> + * partition_update - Update an existing partition configuration
>> + * @cs: The cpuset to update
>> + * @prs: Partition root state (must be positive)
>> + * @xcpus: New exclusive CPUs mask for the partition (NULL to keep current)
>> + * @excpus: New effective exclusive CPUs mask
>> + * @tmp: Temporary masks
>> + *
>> + * Updates partition-related fields. The tmp->addmask is the CPU mask that
>> + * will be added to the subpartitions_cpus and removed from parent's
>> + * effective_cpus, and the tmp->delmask vice versa.
>> + */
>> +static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
>> +                  struct cpumask *excpus, struct tmpmasks *tmp)
>> +{
>> +    bool isolcpus_updated;
>> +    bool excl_updated;
>> +    struct cpuset *parent;
>> +
>> +    lockdep_assert_held(&cpuset_mutex);
>> +    WARN_ON_ONCE(!cpuset_v2());
>> +    WARN_ON_ONCE(prs <= 0);
>> +
>> +    parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>> +    excl_updated = !cpumask_empty(tmp->addmask) ||
>> +            !cpumask_empty(tmp->delmask);
>> +
>> +    spin_lock_irq(&callback_lock);
>> +    isolcpus_updated = partition_xcpus_add(prs, parent, tmp->addmask);
>> +    isolcpus_updated |= partition_xcpus_del(prs, parent, tmp->delmask);
> 
> The current partition_xcpus_add/del() functions assume the given cpumas is non-empty. In the new
> partition_update() helper, you can pass an empty cpumask to them. This will cause useless work to be
> done. Also isolcpus_update may not be correct because of that causing unneeded work to be done in
> the workqueue code.
> 
> -Longman

Thank you, Longman.

I think we can add a check for empty cpumask inputs in partition_xcpus_add() and
partition_xcpus_del() to avoid unnecessary operations.

To clarify, do you mean that passing an empty cpumask to these functions might lead to incorrect
isolcpus_updated value? or are you referring to other potential logic issues?

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 06/16] cpuset: introduce local_partition_enable()
  2025-10-20  2:44   ` Waiman Long
@ 2025-10-20  8:06     ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  8:06 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 10:44, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> The partition_enable() function introduced in the previous patch can be
>> reused to enable local partitions.
>>
>> First, partition_enable() was enhanced to support local partition enabling
>> by properly handling parent's nr_subparts counter and adding notification
>> operations.
>>
>> Then, the local_partition_enable() function is introduced, which factors
>> out the local partition enablement logic from
>> update_parent_effective_cpumask(). After passing local partition validation
>> checks, it delegates to partition_enable() to complete the partition setup.
>>
>> This refactoring creates a clear separation between local and remote
>> partition operations while maintaining code reuse through the shared
>> partition_enable() infrastructure.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 111 +++++++++++++++++++++++++++--------------
>>   1 file changed, 74 insertions(+), 37 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 0e2f95daf459..154992cdfe9a 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1539,6 +1539,7 @@ static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>>                    int new_prs, struct cpumask *new_excpus)
>>   {
>>       bool isolcpus_updated;
>> +    int old_prs;
>>         lockdep_assert_held(&cpuset_mutex);
>>       WARN_ON_ONCE(new_prs <= 0);
>> @@ -1547,15 +1548,21 @@ static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>>       if (cs->partition_root_state == new_prs)
>>           return;
>>   +    old_prs = cs->partition_root_state;
>>       spin_lock_irq(&callback_lock);
>>       /* enable partition should only add exclusive cpus */
>>       isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
>> -    list_add(&cs->remote_sibling, &remote_children);
>> +    /* enable remote partition */
>> +    if (!parent)
>> +        list_add(&cs->remote_sibling, &remote_children);
>> +    else if (!is_partition_valid(cs))
>> +        parent->nr_subparts += 1;
>>       cpumask_copy(cs->effective_xcpus, new_excpus);
>>       partition_state_update(cs, new_prs, PERR_NONE);
>>       spin_unlock_irq(&callback_lock);
>>       update_unbound_workqueue_cpumask(isolcpus_updated);
>>       cpuset_force_rebuild();
>> +    notify_partition_change(cs, old_prs);
>>   }
> 
> As commented in an earlier patch, the partition_enable() chnage should be moved there.
> 
> Cheers,
> Longman

Thank you, will update.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 07/16] cpuset: introduce local_partition_disable()
  2025-10-20  2:46   ` Waiman Long
@ 2025-10-20  8:06     ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  8:06 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 10:46, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> The partition_disable() function introduced earlier can be extended to
>> handle local partition disablement.
>>
>> First, partition_disable() was enhanced to support local partitions by
>> properly managing the parent's nr_subparts counter and integrating
>> notification operations.
>>
>> Then, local_partition_disable() is introduced, which extracts the local
>> partition disable logic from update_parent_effective_cpumask(). It calls
>> partition_disable() to complete the disablement process.
>>
>> This refactoring establishes a clear separation between local and remote
>> partition operations while promoting code reuse through the shared
>> partition_disable() infrastructure.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 60 ++++++++++++++++++++++++++++++------------
>>   1 file changed, 43 insertions(+), 17 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 154992cdfe9a..87ba43e93540 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1576,13 +1576,20 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
>>                       int new_prs, enum prs_errcode prs_err)
>>   {
>>       bool isolcpus_updated;
>> +    int old_prs;
>>         lockdep_assert_held(&cpuset_mutex);
>>       WARN_ON_ONCE(new_prs > 0);
>>       WARN_ON_ONCE(!cpuset_v2());
>>   +    old_prs = cs->partition_root_state;
>>       spin_lock_irq(&callback_lock);
>>       list_del_init(&cs->remote_sibling);
>> +    if (parent && is_partition_valid(parent) &&
>> +        is_partition_valid(cs)) {
>> +        parent->nr_subparts -= 1;
>> +        WARN_ON_ONCE(parent->nr_subparts < 0);
>> +    }
>>       /* disable a partition should only delete exclusive cpus */
>>       isolcpus_updated = partition_xcpus_del(cs->partition_root_state,
>>                           parent, cs->effective_xcpus);
>> @@ -1592,6 +1599,9 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
>>       spin_unlock_irq(&callback_lock);
>>       update_unbound_workqueue_cpumask(isolcpus_updated);
>>       cpuset_force_rebuild();
>> +    /* Clear exclusive flag; no errors are expected */
>> +    update_partition_exclusive_flag(cs, new_prs);
>> +    notify_partition_change(cs, old_prs);
>>   }
>>   
> 
> Similarly, change to partition_disable() should be done previously in patch 4 ("cpuset: factor out
> partition_disable() function") for completeness.
> 
> Cheers,
> Longman
> 

Thank you, will update.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate()
  2025-10-20  2:48   ` Waiman Long
@ 2025-10-20  8:28     ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  8:28 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 10:48, Waiman Long wrote:
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Build on the partition_disable() infrastructure introduced in the previous
>> patch to handle local partition invalidation.
>>
>> The local_partition_invalidate() function factors out the local partition
>> invalidation logic from update_parent_effective_cpumask(), which delegates
>> to partition_disable() to complete the invalidation process.
>>
>> Additionally, correct the transition logic in cpuset_hotplug_update_tasks()
>> when determining whether to transition an invalid partition root, the check
>> should be based on non-empty user_cpus rather than non-empty
>> effective_xcpus. This correction addresses the scenario where
>> exclusive_cpus is not set but cpus_allowed is configured - in this case,
>> effective_xcpus may be empty even though the partition should be considered
>> for re-enablement. The user_cpus-based check ensures proper partition state
>> transitions under these conditions.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 66 +++++++++++++++++++++++++++---------------
>>   1 file changed, 42 insertions(+), 24 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 87ba43e93540..e460d03286ba 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1911,6 +1911,39 @@ static void local_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
>>       }
>>   }
>>   +/**
>> + * local_partition_invalidate - Invalidate a local partition
>> + * @cs: Target cpuset (local partition root) to invalidate
>> + * @tmp: Temporary masks
>> + */
>> +static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks *tmp)
>> +{
>> +    struct cpumask *xcpus = user_xcpus(cs);
>> +    struct cpuset *parent = parent_cs(cs);
>> +    int new_prs = cs->partition_root_state;
>> +    bool cpumask_updated = false;
>> +
>> +    lockdep_assert_held(&cpuset_mutex);
>> +    WARN_ON_ONCE(is_remote_partition(cs));    /* For local partition only */
>> +
>> +    if (is_partition_invalid(cs))
>> +        return;
> You should change the check to if (!is_partition_valid(cs)). You can avoid the case that
> partition_disable() is called with a member.
> 

Thank you for the suggestion.

I kept the current check to align with the logic from update_parent_effective_cpumask() and to keep
the changes easier to review.

In patch 12, I've unified local_partition_invalidate and the original local_partition_disable into a
single local_partition_disable function, which now uses the if (!is_partition_valid(cs)) check. This
also brings it in line with the existing remote_partition_disable().

Since the local_partition_invalidate is removed in the subsequent patches, I believe it's a minor issue.

>> +    /*
>> +     * Make the current partition invalid.
>> +     */
>> +    if (is_partition_valid(parent))
>> +        cpumask_updated = cpumask_and(tmp->addmask,
>> +                          xcpus, parent->effective_xcpus);
>> +    if (cs->partition_root_state > 0)
>> +        new_prs = -cs->partition_root_state;
>> +
>> +    partition_disable(cs, parent, new_prs, cs->prs_err);
>> +    if (cpumask_updated) {
>> +        cpuset_update_tasks_cpumask(parent, tmp->addmask);
>> +        update_sibling_cpumasks(parent, cs, tmp);
>> +    }
>> +}
>> +
>>   /**
>>    * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
>>    * @cs:      The cpuset that requests change in partition root state
>> @@ -1972,23 +2005,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>       adding = deleting = false;
>>       old_prs = new_prs = cs->partition_root_state;
>>   -    if (cmd == partcmd_invalidate) {
>> -        if (is_partition_invalid(cs))
>> -            return 0;
>> -
>> -        /*
>> -         * Make the current partition invalid.
>> -         */
>> -        if (is_partition_valid(parent))
>> -            adding = cpumask_and(tmp->addmask,
>> -                         xcpus, parent->effective_xcpus);
>> -        if (old_prs > 0) {
>> -            new_prs = -old_prs;
>> -            subparts_delta--;
>> -        }
>> -        goto write_error;
>> -    }
>> -
>>       /*
>>        * The parent must be a partition root.
>>        * The new cpumask, if present, or the current cpus_allowed must
>> @@ -2552,7 +2568,7 @@ static int cpus_allowed_validate_change(struct cpuset *cs, struct cpuset
>> *trialc
>>               if (is_partition_valid(cp) &&
>>                   cpumask_intersects(xcpus, cp->effective_xcpus)) {
>>                   rcu_read_unlock();
>> -                update_parent_effective_cpumask(cp, partcmd_invalidate, NULL, tmp);
>> +                local_partition_invalidate(cp, tmp);
>>                   rcu_read_lock();
>>               }
>>           }
>> @@ -2592,8 +2608,7 @@ static void partition_cpus_change(struct cpuset *cs, struct cpuset *trialcs,
>>                          trialcs->effective_xcpus, tmp);
>>       } else {
>>           if (trialcs->prs_err)
>> -            update_parent_effective_cpumask(cs, partcmd_invalidate,
>> -                            NULL, tmp);
>> +            local_partition_invalidate(cs, tmp);
>>           else
>>               update_parent_effective_cpumask(cs, partcmd_update,
>>                               trialcs->effective_xcpus, tmp);
>> @@ -4037,18 +4052,21 @@ static void cpuset_hotplug_update_tasks(struct cpuset *cs, struct tmpmasks
>> *tmp)
>>        *    partitions.
>>        */
>>       if (is_local_partition(cs) && (!is_partition_valid(parent) ||
>> -                tasks_nocpu_error(parent, cs, &new_cpus)))
>> +                tasks_nocpu_error(parent, cs, &new_cpus))) {
>>           partcmd = partcmd_invalidate;
>> +        local_partition_invalidate(cs, tmp);
>> +    }
>>       /*
>>        * On the other hand, an invalid partition root may be transitioned
>> -     * back to a regular one with a non-empty effective xcpus.
>> +     * back to a regular one with a non-empty user xcpus.
>>        */
>>       else if (is_partition_valid(parent) && is_partition_invalid(cs) &&
>> -         !cpumask_empty(cs->effective_xcpus))
>> +         !cpumask_empty(user_xcpus(cs))) {
> 
> I believe the effective_xcpus is not cleared currently when a partition is invalidated. Anyway, this
> change is also OK especially if effective_xcpus will be cleared in a later patch.
> 
> Cheers,
> Longman
> 
>>           partcmd = partcmd_update;
>> +        update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
>> +    }
>>         if (partcmd >= 0) {
>> -        update_parent_effective_cpumask(cs, partcmd, NULL, tmp);
>>           if ((partcmd == partcmd_invalidate) || is_partition_valid(cs)) {
>>               compute_partition_effective_cpumask(cs, &new_cpus);
>>               cpuset_force_rebuild();

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 11/16] cpuset: simplify partition update logic for hotplug tasks
  2025-10-20  3:00   ` Waiman Long
@ 2025-10-20  8:44     ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  8:44 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 11:00, Waiman Long wrote:
> 
> On 9/28/25 3:13 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Simplify the partition update logic in cpuset_hotplug_update_tasks() by
>> calling the unified local_partition_update() interface.
>>
>> For local partitions, the previous patch introduced local_partition_update
>> which handles both validation state transitions:
>> - Invalidates local partitions that fail validation checks
>> - Transitions invalid partitions to valid state when no errors are detected
>>
>> This eliminates the need for separate transition logic
>> in cpuset_hotplug_update_tasks(), which can now simply call
>> local_partition_update() to handle all local partition changes.
>>
>> This patch simplifies the logic by always proceeding to update_tasks for
>> remote partitions, regardless of whether they were disabled or not. Since
>> the original code didn't perform any meaningful operations for non-disabled
>> remote partitions, this change should not affect functionality.
>>
>> The partition_cmd mechanism can now be safely removed as it is no longer
> 
> It is partition_cmd enum type.
> 

Thank you,

Will update.

>> referenced by any code paths after the partition update logic
>> simplification.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 67 ++++++++++++++++--------------------------
>>   1 file changed, 26 insertions(+), 41 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 9e98df542715..a1896a199c8b 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1211,17 +1211,6 @@ static void compute_effective_cpumask(struct cpumask *new_cpus,
>>       cpumask_and(new_cpus, cs->cpus_allowed, parent->effective_cpus);
>>   }
>>   -/*
>> - * Commands for update_parent_effective_cpumask
>> - */
>> -enum partition_cmd {
>> -    partcmd_enable,        /* Enable partition root      */
>> -    partcmd_enablei,    /* Enable isolated partition root */
>> -    partcmd_disable,    /* Disable partition root      */
>> -    partcmd_update,        /* Update parent's effective_cpus */
>> -    partcmd_invalidate,    /* Make partition invalid      */
>> -};
>> -
>>   static void update_sibling_cpumasks(struct cpuset *parent, struct cpuset *cs,
>>                       struct tmpmasks *tmp);
>>   @@ -2062,6 +2051,9 @@ static int __local_partition_update(struct cpuset *cs, struct cpumask *xcpus,
>>           update_partition_sd_lb(cs, old_prs);
>>           return part_error;
>>       }
>> +    /* Nothing changes, return PERR_NONE */
>> +    if (new_prs == old_prs && cpumask_equal(excpus, cs->effective_xcpus))
>> +        return PERR_NONE;
> I believe you already have this check added when you introduce __local_partition_update() in patch
> 9. It is a duplicate.
> 
> Cheers,
> Longman

Thank you,

Will update.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask
  2025-10-20  3:02   ` Waiman Long
@ 2025-10-20  8:47     ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  8:47 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 11:02, Waiman Long wrote:
> 
> On 9/28/25 3:13 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Replace the partition invalidation logic in the
>> compute_partition_effective_cpumask() with a call to partition_disable().
>>
>> This centralizes partition state management and ensures consistent
>> handling of partition disable operations throughout the cpuset subsystem.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 28 +++++++---------------------
>>   1 file changed, 7 insertions(+), 21 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 6625b803ba02..20288dbd6ccf 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -170,15 +170,6 @@ static inline bool cs_is_member(const struct cpuset *cs)
>>       return cs->partition_root_state == PRS_MEMBER;
>>   }
>>   -/*
>> - * Callers should hold callback_lock to modify partition_root_state.
>> - */
>> -static inline void make_partition_invalid(struct cpuset *cs)
>> -{
>> -    if (cs->partition_root_state > 0)
>> -        cs->partition_root_state = -cs->partition_root_state;
>> -}
>> -
>>   /*
>>    * Send notification event of whenever partition_root_state changes.
>>    */
>> @@ -2073,6 +2064,7 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>       struct cgroup_subsys_state *css;
>>       struct cpuset *child;
>>       bool populated = partition_is_populated(cs, NULL);
>> +    enum prs_errcode prs_err;
>>         /*
>>        * Check child partition roots to see if they should be
>> @@ -2095,26 +2087,20 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>>            * partition root.
>>            */
>>           WARN_ON_ONCE(is_remote_partition(child));
>> -        child->prs_err = 0;
>> +        prs_err = 0;
>>           if (!cpumask_subset(child->effective_xcpus,
>>                       cs->effective_xcpus))
>> -            child->prs_err = PERR_INVCPUS;
>> +            prs_err = PERR_INVCPUS;
>>           else if (populated &&
>>                cpumask_subset(new_ecpus, child->effective_xcpus))
>> -            child->prs_err = PERR_NOCPUS;
>> -
>> -        if (child->prs_err) {
>> -            int old_prs = child->partition_root_state;
>> +            prs_err = PERR_NOCPUS;
>>   +        if (prs_err) {
>>               /*
>>                * Invalidate child partition
>>                */
>> -            spin_lock_irq(&callback_lock);
>> -            make_partition_invalid(child);
>> -            cs->nr_subparts--;
>> -            child->nr_subparts = 0;
>> -            spin_unlock_irq(&callback_lock);
>> -            notify_partition_change(child, old_prs);
>> +            partition_disable(child, parent_cs(child),
> 
> The parent of child should just be cs. You don't need to use parent_cs() to get it.
> 
> Cheers,
> Longman
> 

Thank you, Longman,

Will update.

> 
>> +                      -child->partition_root_state, prs_err);
>>               continue;
>>           }
>>           cpumask_andnot(new_ecpus, new_ecpus,

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root
  2025-10-20  3:06   ` Waiman Long
@ 2025-10-20  9:13     ` Chen Ridong
  2025-10-22 10:49     ` Chen Ridong
  1 sibling, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  9:13 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 11:06, Waiman Long wrote:
> On 9/28/25 3:13 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> A bug was detected with the following steps:
>>
>>    # cd /sys/fs/cgroup/
>>    # mkdir test
>>    # echo 9 > test/cpuset.cpus
>>    # echo isolated > test/cpuset.cpus.partition
>>    # cat test/cpuset.cpus.partition
>>    isolated
>>    # cat test/cpuset.cpus
>>    9
>>    # echo root > test/cpuset.cpus.partition
>>    # cat test/cpuset.cpus
>>    9
>>    # cat test/cpuset.cpus.partition
>>    root
>>
>> CPU 9 was initially placed in an isolated partition. When the partition
>> type is changed from isolated to root, CPU 9 remains in what becomes a
>> valid root partition. This violates the rule that isolcpus can only be
>> assigned to isolated partitions.
> 
> I am a bit confused at the beginning about this as it does not clearly state that CPU 9 was listed
> in the "isolcpus" boot command line parameter, but I believe this is what you mean here. Yes, there
> is a restriction that a boot time isolcpus CPU cannot be put into a non-isolated partition, though
> that will likely to be relaxed in the near future.
> 

Yep, the CPU 9 was listed in the "isolcpus" boot command line parameter.

> Anyway, it is a real corner case. I also don't believe commit f28e22441f35 is the one that
> introduced this issue as the restriction was added later on via commit 4a74e418881f ("cgroup/cpuset:
> Check partition conflict with housekeeping setup").
> 
> As you have added a Fixes tag, it should be moved to the front of the series as it is likely to be
> backported to stable. Putting it near the end of a series with a lot of changes in between will make
> it harder to backport to the stable kernels.
> 

Maybe I should find some way to fix this issue first.

>> Fix the issue by re-enabling partition validation, which performs
>> comprehensive partition error checking. In the scenario described above,
>> this change causes the operation to fail with housekeeping conflicts,
>> preventing the invalid configuration.
> From the code diff below, I don't know how you re-enable partition validation.
> 

When a valid local partition has its type changed (e.g., from "isolated" to "root"), the
remote_partition_enable function is not invoked again. Consequently, the critical check for whether
the partition can be enabled is skipped during this process.

  echo isolated > test/cpuset.cpus.partition
  echo root > test/cpuset.cpus.partition

>>
>> Additionally, when enable a local partition, the warning for tmp->addmask
>> not being a subset of parent's effective CPUs was removed. This warning was
>> triggered during local partition re-enablement because the CPUs were
>> already added to exclusive_cpus during the previous enable operation. The
>> subset check is not applicable in this re-enablement scenario.
> 
> That should be in the new code that you introduce in this series. So it either be integrated into
> one of your earlier patches or be separated out as a separate patch without the Fixes tag as it is
> not applicable for the stable releases.
> 
> Cheers,
> Longman
> 

Okay, I will try to integrate it into earlier patches.

>>
>> Fixes: f28e22441f35 ("cgroup/cpuset: Add a new isolated cpus.partition type")
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 35 +++++++++--------------------------
>>   1 file changed, 9 insertions(+), 26 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index 20288dbd6ccf..2aaa688c596f 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1873,6 +1873,7 @@ static int local_partition_enable(struct cpuset *cs,
>>   {
>>       struct cpuset *parent = parent_cs(cs);
>>       enum prs_errcode part_error;
>> +    bool cpumask_updated = false;
>>         lockdep_assert_held(&cpuset_mutex);
>>       WARN_ON_ONCE(is_remote_partition(cs));    /* For local partition only */
>> @@ -1899,22 +1900,14 @@ static int local_partition_enable(struct cpuset *cs,
>>       if (part_error)
>>           return part_error;
>>   -    /*
>> -     * This function will only be called when all the preliminary
>> -     * checks have passed. At this point, the following condition
>> -     * should hold.
>> -     *
>> -     * (cs->effective_xcpus & cpu_active_mask) ⊆ parent->effective_cpus
>> -     *
>> -     * Warn if it is not the case.
>> -     * addmask is used as temporary variable.
>> -     */
>> -    cpumask_and(tmp->addmask, tmp->new_cpus, cpu_active_mask);
>> -    WARN_ON_ONCE(!cpumask_subset(tmp->addmask, parent->effective_cpus));
>> +    cpumask_updated = cpumask_andnot(tmp->addmask, tmp->new_cpus,
>> +                     parent->effective_cpus);
>>       partition_enable(cs, parent, new_prs, tmp->new_cpus);
>>   -    cpuset_update_tasks_cpumask(parent, tmp->addmask);
>> -    update_sibling_cpumasks(parent, cs, tmp);
>> +    if (cpumask_updated) {
>> +        cpuset_update_tasks_cpumask(parent, tmp->addmask);
>> +        update_sibling_cpumasks(parent, cs, tmp);
>> +    }
>>       return 0;
>>   }
>>   @@ -2902,7 +2895,6 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>       int err = PERR_NONE, old_prs = cs->partition_root_state;
>>       struct cpuset *parent = parent_cs(cs);
>>       struct tmpmasks tmpmask;
>> -    bool isolcpus_updated = false;
>>         if (old_prs == new_prs)
>>           return 0;
>> @@ -2920,7 +2912,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>       if (err)
>>           goto out;
>>   -    if (!old_prs) {
>> +    if (new_prs > 0) {
>>           /*
>>            * cpus_allowed and exclusive_cpus cannot be both empty.
>>            */
>> @@ -2950,12 +2942,6 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>               err = local_partition_enable(cs, new_prs, &tmpmask);
>>           else
>>               err = remote_partition_enable(cs, new_prs, &tmpmask);
>> -    } else if (old_prs && new_prs) {
>> -        /*
>> -         * A change in load balance state only, no change in cpumasks.
>> -         * Need to update isolated_cpus.
>> -         */
>> -        isolcpus_updated = true;
>>       } else {
>>           /*
>>            * Switching back to member is always allowed even if it
>> @@ -2985,16 +2971,13 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>       WRITE_ONCE(cs->prs_err, err);
>>       if (!is_partition_valid(cs))
>>           reset_partition_data(cs);
>> -    else if (isolcpus_updated)
>> -        isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>>       spin_unlock_irq(&callback_lock);
>> -    update_unbound_workqueue_cpumask(isolcpus_updated);
>>         /* Force update if switching back to member & update effective_xcpus */
>>       update_cpumasks_hier(cs, &tmpmask, !new_prs);
>>         /* A newly created partition must have effective_xcpus set */
>> -    WARN_ON_ONCE(!old_prs && (new_prs > 0)
>> +    WARN_ON_ONCE(!old_prs && (cs->partition_root_state > 0)
>>                     && cpumask_empty(cs->effective_xcpus));
>>         /* Update sched domains and load balance flag */

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 09/16] cpuset: introduce local_partition_update()
  2025-10-20  2:57   ` Waiman Long
@ 2025-10-20  9:24     ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-20  9:24 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 10:57, Waiman Long wrote:
> 
> On 9/28/25 3:12 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> Extend the partition_update() infrastructure to handle local partition
>> updates.
>>
>> The local_partition_update() function replaces the command partcmd_update
>> previously handled within update_parent_effective_cpumask(). The update
>> logic follows a state-based approach:
>>
>> 1. Validation check: First verify if the local partition is currently valid
>> 2. Invalidation handling: If the partition is invalid, trigger invalidation
>> 3. State transition: If an invalid partition has no errors, transition to
>>     valid
>> 4. cpus updates: For local partition that only cpu maks changes, use
> "cpu mask"?
>>     partition_update() to handle partition change.
>>
>> With the introduction of this function, update_parent_effective_cpumask()
>> function is removed, simplifying the partition update code path and
>> creating a cleaner separation between local and remote partition
>> operations.
>>
>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>> ---
>>   kernel/cgroup/cpuset.c | 376 +++++++++++++----------------------------
>>   1 file changed, 122 insertions(+), 254 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index e460d03286ba..d0217db04b69 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1622,12 +1622,14 @@ static void partition_update(struct cpuset *cs, int prs, struct cpumask
>> *xcpus,
>>       bool isolcpus_updated;
>>       bool excl_updated;
>>       struct cpuset *parent;
>> +    int old_prs;
>>         lockdep_assert_held(&cpuset_mutex);
>>       WARN_ON_ONCE(!cpuset_v2());
>>       WARN_ON_ONCE(prs <= 0);
>>         parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>> +    old_prs = cs->partition_root_state;
>>       excl_updated = !cpumask_empty(tmp->addmask) ||
>>               !cpumask_empty(tmp->delmask);
>>   @@ -1645,6 +1647,8 @@ static void partition_update(struct cpuset *cs, int prs, struct cpumask
>> *xcpus,
>>       update_unbound_workqueue_cpumask(isolcpus_updated);
>>       if (excl_updated)
>>           cpuset_force_rebuild();
>> +    update_partition_exclusive_flag(cs, prs);
>> +    notify_partition_change(cs, old_prs);
>>   }
>>   
> 
> Again, change to partition_update() should be done in the patch that introduces it.
> 

Thank you,

Will update.

>>   /*
>> @@ -1790,6 +1794,27 @@ static bool prstate_housekeeping_conflict(int prstate, struct cpumask
>> *new_cpus)
>>       return false;
>>   }
>>   +static bool cpuset_user_cpus_exclusive(struct cpuset *cs)
> 
> The cpuset prefix is only needed if it is an externally visible function. For this one, I think a
> better name should be "is_user_xcpus_exclusive".
> 

Will update.

>> +{
>> +    struct cpuset *parent = parent_cs(cs);
>> +
>> +    struct cgroup_subsys_state *css;
>> +    struct cpuset *child;
>> +    bool exclusive = true;
>> +
>> +    rcu_read_lock();
>> +    cpuset_for_each_child(child, css, parent) {
>> +        if (child == cs)
>> +            continue;
>> +        if (!cpusets_are_exclusive(cs, child)) {
>> +            exclusive = false;
>> +            break;
>> +        }
>> +    }
>> +    rcu_read_unlock();
>> +    return exclusive;
>> +}
>> +
>>   /**
>>    * validate_partition - Validate a cpuset partition configuration
>>    * @cs: The cpuset to validate
>> @@ -1818,6 +1843,39 @@ static enum prs_errcode validate_partition(struct cpuset *cs, int new_prs,
>>       return PERR_NONE;
>>   }
>>   +/**
>> + * local_partition_check - Validate for local partition
>> + * @cs: Target cpuset to validate
>> + * @new_prs: New partition root state to validate
>> + * @excpus: New exclusive effectuve CPUs mask to validate
>> + * @excl_check: Flag to enable exclusive CPUs ownership validation
>> + *
>> + * Return: PERR_NONE if validation passes, appropriate error code otherwise
>> + *
>> + * Important: The caller must ensure that @cs's cpu mask is updated before
>> + * invoking this function when exclusive CPU validation is required.
>> + */
>> +static enum prs_errcode local_partition_check(struct cpuset *cs, int new_prs,
>> +                             struct cpumask *excpus, bool excl_check)
> 
> I would suggest naming it to "validate_local_partition()" as the local counterpart of
> validate_partition().
> 

Will upate

>> +{
>> +    struct cpuset *parent = parent_cs(cs);
>> +
>> +    /*
>> +     * The parent must be a partition root.
>> +     * The new cpumask, if present, or the current cpus_allowed must
>> +     * not be empty.
>> +     */
>> +    if (!is_partition_valid(parent)) {
>> +        return is_partition_invalid(parent)
>> +            ? PERR_INVPARENT : PERR_NOTPART;
>> +    }
>> +
>> +    if (excl_check && !cpuset_user_cpus_exclusive(cs))
>> +        return PERR_NOTEXCL;
>> +
>> +    return validate_partition(cs, new_prs, excpus);
>> +}
>> +
>>   /**
>>    * local_partition_enable - Enable local partition for a cpuset
>>    * @cs: Target cpuset to become a local partition root
>> @@ -1945,280 +2003,85 @@ static void local_partition_invalidate(struct cpuset *cs, struct tmpmasks
>> *tmp)
>>   }
>>     /**
>> - * update_parent_effective_cpumask - update effective_cpus mask of parent cpuset
>> - * @cs:      The cpuset that requests change in partition root state
>> - * @cmd:     Partition root state change command
>> - * @newmask: Optional new cpumask for partcmd_update
>> - * @tmp:     Temporary addmask and delmask
>> - * Return:   0 or a partition root state error code
>> - *
>> - * For partcmd_enable*, the cpuset is being transformed from a non-partition
>> - * root to a partition root. The effective_xcpus (cpus_allowed if
>> - * effective_xcpus not set) mask of the given cpuset will be taken away from
>> - * parent's effective_cpus. The function will return 0 if all the CPUs listed
>> - * in effective_xcpus can be granted or an error code will be returned.
>> - *
>> - * For partcmd_disable, the cpuset is being transformed from a partition
>> - * root back to a non-partition root. Any CPUs in effective_xcpus will be
>> - * given back to parent's effective_cpus. 0 will always be returned.
>> + * __local_partition_update - Update local CPU partition configuration
>> + * @cs: Target cpuset to update
>> + * @xcpus: New exclusive CPU mask
>> + * @excpus: New effective exclusive CPU mask
>> + * @tmp: Temporary mask storage for intermediate calculations
>> + * @excl_check: Flag to enable exclusivity validation
>>    *
>> - * For partcmd_update, if the optional newmask is specified, the cpu list is
>> - * to be changed from effective_xcpus to newmask. Otherwise, effective_xcpus is
>> - * assumed to remain the same. The cpuset should either be a valid or invalid
>> - * partition root. The partition root state may change from valid to invalid
>> - * or vice versa. An error code will be returned if transitioning from
>> - * invalid to valid violates the exclusivity rule.
>> + * Handles updates to local CPU partition configurations by validating
>> + * changes, managing state transitions, and propagating updates through
>> + * the cpuset hierarchy.
>>    *
>> - * For partcmd_invalidate, the current partition will be made invalid.
>> + * Note on exclusivity checking: Exclusivity validation is required when
>> + * transitioning from an invalid to valid partition state. However, when
>> + * updating cpus_allowed or exclusive_cpus, exclusivity should have already
>> + * been verified by validate_change(). In such cases, excl_check must be
>> + * false since the cs cpumasks are not yet updated.
>>    *
>> - * The partcmd_enable* and partcmd_disable commands are used by
>> - * update_prstate(). An error code may be returned and the caller will check
>> - * for error.
>> - *
>> - * The partcmd_update command is used by update_cpumasks_hier() with newmask
>> - * NULL and update_cpumask() with newmask set. The partcmd_invalidate is used
>> - * by update_cpumask() with NULL newmask. In both cases, the callers won't
>> - * check for error and so partition_root_state and prs_err will be updated
>> - * directly.
>> + * Return: Partition error code (PERR_NONE indicates success)
>>    */
>> -static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>> -                       struct cpumask *newmask,
>> -                       struct tmpmasks *tmp)
> Separate out the removal of update_parent_effective_cpumask() into its own patch as intermixing the
> removal of this code and new code make it harder to review.
> 

Will do.

>> +static int __local_partition_update(struct cpuset *cs, struct cpumask *xcpus,
>> +                  struct cpumask *excpus, struct tmpmasks *tmp,
>> +                  bool excl_check)
>>   {
>>       struct cpuset *parent = parent_cs(cs);
>> -    int adding;    /* Adding cpus to parent's effective_cpus    */
>> -    int deleting;    /* Deleting cpus from parent's effective_cpus    */
>> -    int old_prs, new_prs;
>>       int part_error = PERR_NONE;    /* Partition error? */
>> -    int subparts_delta = 0;
>> -    int isolcpus_updated = 0;
>> -    struct cpumask *xcpus = user_xcpus(cs);
>> -    bool nocpu;
>> +    int old_prs, new_prs;
>> +    bool cpumask_updated = false;
>>         lockdep_assert_held(&cpuset_mutex);
>> -    WARN_ON_ONCE(is_remote_partition(cs));    /* For local partition only */
>> +    /* For local partition only */
>> +    if (WARN_ON_ONCE(is_remote_partition(cs) || cs_is_member(cs)))
>> +        return PERR_NONE;
>>   +    old_prs = cs->partition_root_state;
>>       /*
>> -     * new_prs will only be changed for the partcmd_update and
>> -     * partcmd_invalidate commands.
>> +     * If new_prs < 0, it might transition to valid partition state.
>> +     * Use absolute value for validation checks.
>>        */
>> -    adding = deleting = false;
>> -    old_prs = new_prs = cs->partition_root_state;
>> -
>> -    /*
>> -     * The parent must be a partition root.
>> -     * The new cpumask, if present, or the current cpus_allowed must
>> -     * not be empty.
>> -     */
>> -    if (!is_partition_valid(parent)) {
>> -        return is_partition_invalid(parent)
>> -               ? PERR_INVPARENT : PERR_NOTPART;
>> -    }
>> -    if (!newmask && xcpus_empty(cs))
>> -        return PERR_CPUSEMPTY;
>> -
>> -    nocpu = tasks_nocpu_error(parent, cs, xcpus);
>> -
>> -    if (newmask) {
>> -        /*
>> -         * Empty cpumask is not allowed
>> -         */
>> -        if (cpumask_empty(newmask)) {
>> -            part_error = PERR_CPUSEMPTY;
>> -            goto write_error;
>> -        }
>> -
>> -        /* Check newmask again, whether cpus are available for parent/cs */
>> -        nocpu |= tasks_nocpu_error(parent, cs, newmask);
>> -
>> -        /*
>> -         * partcmd_update with newmask:
>> -         *
>> -         * Compute add/delete mask to/from effective_cpus
>> -         *
>> -         * For valid partition:
>> -         *   addmask = exclusive_cpus & ~newmask
>> -         *                  & parent->effective_xcpus
>> -         *   delmask = newmask & ~exclusive_cpus
>> -         *               & parent->effective_xcpus
>> -         *
>> -         * For invalid partition:
>> -         *   delmask = newmask & parent->effective_xcpus
>> -         */
>> -        if (is_partition_invalid(cs)) {
>> -            adding = false;
>> -            deleting = cpumask_and(tmp->delmask,
>> -                    newmask, parent->effective_xcpus);
>> -        } else {
>> -            cpumask_andnot(tmp->addmask, xcpus, newmask);
>> -            adding = cpumask_and(tmp->addmask, tmp->addmask,
>> -                         parent->effective_xcpus);
>> -
>> -            cpumask_andnot(tmp->delmask, newmask, xcpus);
>> -            deleting = cpumask_and(tmp->delmask, tmp->delmask,
>> -                           parent->effective_xcpus);
>> -        }
>> -        /*
>> -         * The new CPUs to be removed from parent's effective CPUs
>> -         * must be present.
>> -         */
>> -        if (deleting) {
>> -            cpumask_and(tmp->new_cpus, tmp->delmask, cpu_active_mask);
>> -            WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
>> -        }
>> -
>> -        /*
>> -         * Make partition invalid if parent's effective_cpus could
>> -         * become empty and there are tasks in the parent.
>> -         */
>> -        if (nocpu && (!adding ||
>> -            !cpumask_intersects(tmp->addmask, cpu_active_mask))) {
>> -            part_error = PERR_NOCPUS;
>> -            deleting = false;
>> -            adding = cpumask_and(tmp->addmask,
>> -                         xcpus, parent->effective_xcpus);
>> -        }
>> -    } else {
>> -        /*
>> -         * partcmd_update w/o newmask
>> -         *
>> -         * delmask = effective_xcpus & parent->effective_cpus
>> -         *
>> -         * This can be called from:
>> -         * 1) update_cpumasks_hier()
>> -         * 2) cpuset_hotplug_update_tasks()
>> -         *
>> -         * Check to see if it can be transitioned from valid to
>> -         * invalid partition or vice versa.
>> -         *
>> -         * A partition error happens when parent has tasks and all
>> -         * its effective CPUs will have to be distributed out.
>> -         */
>> -        if (nocpu) {
>> -            part_error = PERR_NOCPUS;
>> -            if (is_partition_valid(cs))
>> -                adding = cpumask_and(tmp->addmask,
>> -                        xcpus, parent->effective_xcpus);
>> -        } else if (is_partition_invalid(cs) && !cpumask_empty(xcpus) &&
>> -               cpumask_subset(xcpus, parent->effective_xcpus)) {
>> -            struct cgroup_subsys_state *css;
>> -            struct cpuset *child;
>> -            bool exclusive = true;
>> -
>> -            /*
>> -             * Convert invalid partition to valid has to
>> -             * pass the cpu exclusivity test.
>> -             */
>> -            rcu_read_lock();
>> -            cpuset_for_each_child(child, css, parent) {
>> -                if (child == cs)
>> -                    continue;
>> -                if (!cpusets_are_exclusive(cs, child)) {
>> -                    exclusive = false;
>> -                    break;
>> -                }
>> -            }
>> -            rcu_read_unlock();
>> -            if (exclusive)
>> -                deleting = cpumask_and(tmp->delmask,
>> -                        xcpus, parent->effective_cpus);
>> -            else
>> -                part_error = PERR_NOTEXCL;
>> -        }
>> -    }
>> -
>> -write_error:
>> -    if (part_error)
>> -        WRITE_ONCE(cs->prs_err, part_error);
>> -
>> -    if (cmd == partcmd_update) {
>> -        /*
>> -         * Check for possible transition between valid and invalid
>> -         * partition root.
>> -         */
>> -        switch (cs->partition_root_state) {
>> -        case PRS_ROOT:
>> -        case PRS_ISOLATED:
>> -            if (part_error) {
>> -                new_prs = -old_prs;
>> -                subparts_delta--;
>> -            }
>> -            break;
>> -        case PRS_INVALID_ROOT:
>> -        case PRS_INVALID_ISOLATED:
>> -            if (!part_error) {
>> -                new_prs = -old_prs;
>> -                subparts_delta++;
>> -            }
>> -            break;
>> -        }
>> +    new_prs = old_prs < 0 ? -old_prs : old_prs;
>> +    part_error = local_partition_check(cs, new_prs, excpus, excl_check);
>> +    if (part_error) {
>> +        local_partition_invalidate(cs, tmp);
> 
> local_partition_invalidate() should only called if old_prs > 0.
> 
>> +        return part_error;
>>       }
>>   -    if (!adding && !deleting && (new_prs == old_prs))
>> -        return 0;
>> +    /* Nothing changes, return PERR_NONE */
>> +    if (new_prs == old_prs && cpumask_equal(excpus, cs->effective_xcpus))
>> +        return PERR_NONE;
>>         /*
>> -     * Transitioning between invalid to valid or vice versa may require
>> -     * changing CS_CPU_EXCLUSIVE. In the case of partcmd_update,
>> -     * validate_change() has already been successfully called and
>> -     * CPU lists in cs haven't been updated yet. So defer it to later.
>> +     * If partition was previously invalid but now passes checks,
>> +     * enable it and update related flags
>>        */
>> -    if ((old_prs != new_prs) && (cmd != partcmd_update))  {
>> -        int err = update_partition_exclusive_flag(cs, new_prs);
>> -
>> -        if (err)
>> -            return err;
>> +    if (is_partition_invalid(cs) && !part_error) {
> The !part_error check should be unnecessary as this path will not be reached if part_error is non-zero.
> 

Thank you, Longman, you are right.

Checking !part_error here is redundant because part_error check is already performed at the
beginning of the function.

>> +        partition_enable(cs, parent, new_prs, excpus);
>> +        update_partition_exclusive_flag(cs, new_prs);
>> +        update_partition_sd_lb(cs, old_prs);
>> +        return part_error;
> Just return PERR_NONE if it is not expected to be set.
> 

Will update.

>>       }
>>   +    cpumask_updated = cpumask_andnot(tmp->addmask, excpus, cs->effective_xcpus);
>> +    cpumask_updated |= cpumask_andnot(tmp->delmask, cs->effective_xcpus, excpus);
>> +    partition_update(cs, new_prs, xcpus, excpus, tmp);
>>       /*
>> -     * Change the parent's effective_cpus & effective_xcpus (top cpuset
>> -     * only).
>> -     *
>> -     * Newly added CPUs will be removed from effective_cpus and
>> -     * newly deleted ones will be added back to effective_cpus.
>> -     */
>> -    spin_lock_irq(&callback_lock);
>> -    if (old_prs != new_prs) {
>> -        cs->partition_root_state = new_prs;
>> -        if (new_prs <= 0)
>> -            cs->nr_subparts = 0;
>> -    }
>> -    /*
>> -     * Adding to parent's effective_cpus means deletion CPUs from cs
>> -     * and vice versa.
>> +     * Propagate changes in parent's effective_cpus down the hierarchy.
>>        */
>> -    if (adding)
>> -        isolcpus_updated += partition_xcpus_del(old_prs, parent,
>> -                            tmp->addmask);
>> -    if (deleting)
>> -        isolcpus_updated += partition_xcpus_add(new_prs, parent,
>> -                            tmp->delmask);
>> -
>> -    if (is_partition_valid(parent)) {
>> -        parent->nr_subparts += subparts_delta;
>> -        WARN_ON_ONCE(parent->nr_subparts < 0);
>> -    }
>> -    spin_unlock_irq(&callback_lock);
>> -    update_unbound_workqueue_cpumask(isolcpus_updated);
>> -
>> -    if ((old_prs != new_prs) && (cmd == partcmd_update))
>> -        update_partition_exclusive_flag(cs, new_prs);
>> -
>> -    if (adding || deleting) {
>> +    if (cpumask_updated) {
>>           cpuset_update_tasks_cpumask(parent, tmp->addmask);
>>           update_sibling_cpumasks(parent, cs, tmp);
>>       }
>> +    return part_error;
> 
> Ditto.
> 
> Cheers,
> Longman
> 

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function
  2025-10-20  7:48     ` Chen Ridong
@ 2025-10-20 19:42       ` Waiman Long
  2025-10-21  0:52         ` Chen Ridong
  0 siblings, 1 reply; 45+ messages in thread
From: Waiman Long @ 2025-10-20 19:42 UTC (permalink / raw)
  To: Chen Ridong, Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 10/20/25 3:48 AM, Chen Ridong wrote:
>
> On 2025/10/20 10:39, Waiman Long wrote:
>> On 9/28/25 3:12 AM, Chen Ridong wrote:
>>> From: Chen Ridong <chenridong@huawei.com>
>>>
>>> Extract the core partition enablement logic into a dedicated
>>> partition_enable() function. This refactoring centralizes updates to key
>>> cpuset data structures including remote_sibling, effective_xcpus,
>>> partition_root_state, and prs_err.
>>>
>>> The function handles the complete partition enablement workflow:
>>> - Adding exclusive CPUs via partition_xcpus_add()
>>> - Managing remote sibling relationships
>>> - Synchronizing effective exclusive CPUs mask
>>> - Updating partition state and error status
>>> - Triggering required scheduler domain rebuilds
>>>
>>> This creates a coherent interface for partition operations and establishes
>>> a foundation for future local partition support while maintaining existing
>>> remote partition behavior.
>>>
>>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>>> ---
>>>    kernel/cgroup/cpuset.c | 55 +++++++++++++++++++++++++++++++++---------
>>>    1 file changed, 44 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 0787904321a9..43ce62f4959c 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -1515,6 +1515,49 @@ static inline bool is_local_partition(struct cpuset *cs)
>>>        return is_partition_valid(cs) && !is_remote_partition(cs);
>>>    }
>>>    +static void partition_state_update(struct cpuset *cs, int new_prs,
>>> +                      enum prs_errcode prs_err)
>>> +{
>>> +    lockdep_assert_held(&callback_lock);
>>> +
>>> +    cs->partition_root_state = new_prs;
>>> +    WRITE_ONCE(cs->prs_err, prs_err);
>>> +    if (!is_partition_valid(cs))
>>> +        reset_partition_data(cs);
>>> +}
>>> +
>>> +/**
>>> + * partition_enable - Transitions a cpuset to a partition root
>>> + * @cs: The cpuset to enable partition for
>>> + * @parent: Parent cpuset of @cs, NULL for remote parent
>>> + * @new_prs: New partition root state to set
>>> + * @new_excpus: New exclusive CPUs mask for the partition
>>> + *
>>> + * Transitions a cpuset to a partition root, only for v2.
>>> + */
>>> +static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>>> +                 int new_prs, struct cpumask *new_excpus)
>>> +{
>>> +    bool isolcpus_updated;
>>> +
>>> +    lockdep_assert_held(&cpuset_mutex);
>>> +    WARN_ON_ONCE(new_prs <= 0);
>>> +    WARN_ON_ONCE(!cpuset_v2());
>>> +
>>> +    if (cs->partition_root_state == new_prs)
>>> +        return;
>>> +
>>> +    spin_lock_irq(&callback_lock);
>>> +    /* enable partition should only add exclusive cpus */
>>> +    isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
>>> +    list_add(&cs->remote_sibling, &remote_children);
>>> +    cpumask_copy(cs->effective_xcpus, new_excpus);
>>> +    partition_state_update(cs, new_prs, PERR_NONE);
>>> +    spin_unlock_irq(&callback_lock);
>>> +    update_unbound_workqueue_cpumask(isolcpus_updated);
>>> +    cpuset_force_rebuild();
>>> +}
>>> +
>> partition_enable() is supposed to be a common helper used for the creation of both local and remote
>> partitions. The one in this patch does work for remote partition but not for local partition. I
>> would prefer to make it good for both cases when you introduce it instead adding code in patch 6 to
>> make it work for local partition later in the series. It will make it easier to review instead of
>> jumping back and forth to make sure that it will do the right thing.
>>
>> Cheers,
>> Longman
>>
> Thank you, Longman.
>
> My original intention was to keep the changes easier to review. Patches 3–5 are meant to be pure
> refactoring moves of code from the remote partition logic, without altering any behavior.
>
> Would it be clearer to proceed in the following stages:
>
> 1. Introduce partition_enable(), partition_disable(), and partition_update() with their complete
> logic first.
> 2. Replace the corresponding logic in remote partitions with these new helpers.
> 3. Then, replace the logic in local partitions with the same helpers.
>
Yes, that should make it easier to review.

Thanks,
Longman


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 05/16] cpuset: factor out partition_update() function
  2025-10-20  8:05     ` Chen Ridong
@ 2025-10-20 19:45       ` Waiman Long
  0 siblings, 0 replies; 45+ messages in thread
From: Waiman Long @ 2025-10-20 19:45 UTC (permalink / raw)
  To: Chen Ridong, Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong

On 10/20/25 4:05 AM, Chen Ridong wrote:
>
> On 2025/10/20 10:43, Waiman Long wrote:
>> On 9/28/25 3:12 AM, Chen Ridong wrote:
>>> From: Chen Ridong <chenridong@huawei.com>
>>>
>>> Extract the core partition update logic into a dedicated partition_update()
>>> function. This refactoring centralizes updates to key cpuset data
>>> structures including remote_sibling, effective_xcpus, partition_root_state,
>>> and prs_err.
>>>
>>> The function handles the complete partition update workflow:
>>> - Adding and removing exclusive CPUs via partition_xcpus_add()/del()
>>> - Managing remote sibling relationships
>>> - Synchronizing effective exclusive CPUs mask
>>> - Updating partition state and error status
>>> - Triggering required system updates and workqueue synchronization
>>>
>>> This creates a coherent interface for partition operations and establishes
>>> a foundation for enhanced partition management while maintaining existing
>>> remote partition behavior.
>>>
>>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>>> ---
>>>    kernel/cgroup/cpuset.c | 71 ++++++++++++++++++++++++++++--------------
>>>    1 file changed, 47 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index 1944410ae872..0e2f95daf459 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -1587,6 +1587,49 @@ static void partition_disable(struct cpuset *cs, struct cpuset *parent,
>>>        cpuset_force_rebuild();
>>>    }
>>>    +/**
>>> + * partition_update - Update an existing partition configuration
>>> + * @cs: The cpuset to update
>>> + * @prs: Partition root state (must be positive)
>>> + * @xcpus: New exclusive CPUs mask for the partition (NULL to keep current)
>>> + * @excpus: New effective exclusive CPUs mask
>>> + * @tmp: Temporary masks
>>> + *
>>> + * Updates partition-related fields. The tmp->addmask is the CPU mask that
>>> + * will be added to the subpartitions_cpus and removed from parent's
>>> + * effective_cpus, and the tmp->delmask vice versa.
>>> + */
>>> +static void partition_update(struct cpuset *cs, int prs, struct cpumask *xcpus,
>>> +                  struct cpumask *excpus, struct tmpmasks *tmp)
>>> +{
>>> +    bool isolcpus_updated;
>>> +    bool excl_updated;
>>> +    struct cpuset *parent;
>>> +
>>> +    lockdep_assert_held(&cpuset_mutex);
>>> +    WARN_ON_ONCE(!cpuset_v2());
>>> +    WARN_ON_ONCE(prs <= 0);
>>> +
>>> +    parent = is_remote_partition(cs) ? NULL : parent_cs(cs);
>>> +    excl_updated = !cpumask_empty(tmp->addmask) ||
>>> +            !cpumask_empty(tmp->delmask);
>>> +
>>> +    spin_lock_irq(&callback_lock);
>>> +    isolcpus_updated = partition_xcpus_add(prs, parent, tmp->addmask);
>>> +    isolcpus_updated |= partition_xcpus_del(prs, parent, tmp->delmask);
>> The current partition_xcpus_add/del() functions assume the given cpumas is non-empty. In the new
>> partition_update() helper, you can pass an empty cpumask to them. This will cause useless work to be
>> done. Also isolcpus_update may not be correct because of that causing unneeded work to be done in
>> the workqueue code.
>>
>> -Longman
> Thank you, Longman.
>
> I think we can add a check for empty cpumask inputs in partition_xcpus_add() and
> partition_xcpus_del() to avoid unnecessary operations.
Yes, you should do an empty cpumask check if empty cpumask can be passed 
to the helper.
>
> To clarify, do you mean that passing an empty cpumask to these functions might lead to incorrect
> isolcpus_updated value? or are you referring to other potential logic issues?

My main concern is doing non-useful work. However, I am sure if there 
will be an undesirable side effects as well.

Cheers,
Longman


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function
  2025-10-20 19:42       ` Waiman Long
@ 2025-10-21  0:52         ` Chen Ridong
  0 siblings, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-21  0:52 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/21 3:42, Waiman Long wrote:
> On 10/20/25 3:48 AM, Chen Ridong wrote:
>>
>> On 2025/10/20 10:39, Waiman Long wrote:
>>> On 9/28/25 3:12 AM, Chen Ridong wrote:
>>>> From: Chen Ridong <chenridong@huawei.com>
>>>>
>>>> Extract the core partition enablement logic into a dedicated
>>>> partition_enable() function. This refactoring centralizes updates to key
>>>> cpuset data structures including remote_sibling, effective_xcpus,
>>>> partition_root_state, and prs_err.
>>>>
>>>> The function handles the complete partition enablement workflow:
>>>> - Adding exclusive CPUs via partition_xcpus_add()
>>>> - Managing remote sibling relationships
>>>> - Synchronizing effective exclusive CPUs mask
>>>> - Updating partition state and error status
>>>> - Triggering required scheduler domain rebuilds
>>>>
>>>> This creates a coherent interface for partition operations and establishes
>>>> a foundation for future local partition support while maintaining existing
>>>> remote partition behavior.
>>>>
>>>> Signed-off-by: Chen Ridong <chenridong@huawei.com>
>>>> ---
>>>>    kernel/cgroup/cpuset.c | 55 +++++++++++++++++++++++++++++++++---------
>>>>    1 file changed, 44 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>>> index 0787904321a9..43ce62f4959c 100644
>>>> --- a/kernel/cgroup/cpuset.c
>>>> +++ b/kernel/cgroup/cpuset.c
>>>> @@ -1515,6 +1515,49 @@ static inline bool is_local_partition(struct cpuset *cs)
>>>>        return is_partition_valid(cs) && !is_remote_partition(cs);
>>>>    }
>>>>    +static void partition_state_update(struct cpuset *cs, int new_prs,
>>>> +                      enum prs_errcode prs_err)
>>>> +{
>>>> +    lockdep_assert_held(&callback_lock);
>>>> +
>>>> +    cs->partition_root_state = new_prs;
>>>> +    WRITE_ONCE(cs->prs_err, prs_err);
>>>> +    if (!is_partition_valid(cs))
>>>> +        reset_partition_data(cs);
>>>> +}
>>>> +
>>>> +/**
>>>> + * partition_enable - Transitions a cpuset to a partition root
>>>> + * @cs: The cpuset to enable partition for
>>>> + * @parent: Parent cpuset of @cs, NULL for remote parent
>>>> + * @new_prs: New partition root state to set
>>>> + * @new_excpus: New exclusive CPUs mask for the partition
>>>> + *
>>>> + * Transitions a cpuset to a partition root, only for v2.
>>>> + */
>>>> +static void partition_enable(struct cpuset *cs, struct cpuset *parent,
>>>> +                 int new_prs, struct cpumask *new_excpus)
>>>> +{
>>>> +    bool isolcpus_updated;
>>>> +
>>>> +    lockdep_assert_held(&cpuset_mutex);
>>>> +    WARN_ON_ONCE(new_prs <= 0);
>>>> +    WARN_ON_ONCE(!cpuset_v2());
>>>> +
>>>> +    if (cs->partition_root_state == new_prs)
>>>> +        return;
>>>> +
>>>> +    spin_lock_irq(&callback_lock);
>>>> +    /* enable partition should only add exclusive cpus */
>>>> +    isolcpus_updated = partition_xcpus_add(new_prs, parent, new_excpus);
>>>> +    list_add(&cs->remote_sibling, &remote_children);
>>>> +    cpumask_copy(cs->effective_xcpus, new_excpus);
>>>> +    partition_state_update(cs, new_prs, PERR_NONE);
>>>> +    spin_unlock_irq(&callback_lock);
>>>> +    update_unbound_workqueue_cpumask(isolcpus_updated);
>>>> +    cpuset_force_rebuild();
>>>> +}
>>>> +
>>> partition_enable() is supposed to be a common helper used for the creation of both local and remote
>>> partitions. The one in this patch does work for remote partition but not for local partition. I
>>> would prefer to make it good for both cases when you introduce it instead adding code in patch 6 to
>>> make it work for local partition later in the series. It will make it easier to review instead of
>>> jumping back and forth to make sure that it will do the right thing.
>>>
>>> Cheers,
>>> Longman
>>>
>> Thank you, Longman.
>>
>> My original intention was to keep the changes easier to review. Patches 3–5 are meant to be pure
>> refactoring moves of code from the remote partition logic, without altering any behavior.
>>
>> Would it be clearer to proceed in the following stages:
>>
>> 1. Introduce partition_enable(), partition_disable(), and partition_update() with their complete
>> logic first.
>> 2. Replace the corresponding logic in remote partitions with these new helpers.
>> 3. Then, replace the logic in local partitions with the same helpers.
>>
> Yes, that should make it easier to review.
> 
> Thanks,
> Longman

Thank you for your feedback.
I will update in next version.

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

* Re: [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root
  2025-10-20  3:06   ` Waiman Long
  2025-10-20  9:13     ` Chen Ridong
@ 2025-10-22 10:49     ` Chen Ridong
  1 sibling, 0 replies; 45+ messages in thread
From: Chen Ridong @ 2025-10-22 10:49 UTC (permalink / raw)
  To: Waiman Long, tj, hannes, mkoutny
  Cc: cgups, linux-kernel, lujialin4, chenridong



On 2025/10/20 11:06, Waiman Long wrote:
> On 9/28/25 3:13 AM, Chen Ridong wrote:
>> From: Chen Ridong <chenridong@huawei.com>
>>
>> A bug was detected with the following steps:
>>
>>    # cd /sys/fs/cgroup/
>>    # mkdir test
>>    # echo 9 > test/cpuset.cpus
>>    # echo isolated > test/cpuset.cpus.partition
>>    # cat test/cpuset.cpus.partition
>>    isolated
>>    # cat test/cpuset.cpus
>>    9
>>    # echo root > test/cpuset.cpus.partition
>>    # cat test/cpuset.cpus
>>    9
>>    # cat test/cpuset.cpus.partition
>>    root
>>
>> CPU 9 was initially placed in an isolated partition. When the partition
>> type is changed from isolated to root, CPU 9 remains in what becomes a
>> valid root partition. This violates the rule that isolcpus can only be
>> assigned to isolated partitions.
> 
> I am a bit confused at the beginning about this as it does not clearly state that CPU 9 was listed
> in the "isolcpus" boot command line parameter, but I believe this is what you mean here. Yes, there
> is a restriction that a boot time isolcpus CPU cannot be put into a non-isolated partition, though
> that will likely to be relaxed in the near future.
> 

Hi Longman,

Just to confirm, does this restriction also apply to remote partitions?

-- 
Best regards,
Ridong


^ permalink raw reply	[flat|nested] 45+ messages in thread

end of thread, other threads:[~2025-10-22 10:49 UTC | newest]

Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-28  7:12 [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 01/16] cpuset: use update_partition_sd_lb in update_cpumasks_hier Chen Ridong
2025-10-20  2:37   ` Waiman Long
2025-10-20  7:30     ` Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 02/16] cpuset: generalize validate_partition() interface Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 03/16] cpuset: factor out partition_enable() function Chen Ridong
2025-10-20  2:39   ` Waiman Long
2025-10-20  7:48     ` Chen Ridong
2025-10-20 19:42       ` Waiman Long
2025-10-21  0:52         ` Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 04/16] cpuset: factor out partition_disable() function Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 05/16] cpuset: factor out partition_update() function Chen Ridong
2025-10-20  2:43   ` Waiman Long
2025-10-20  8:05     ` Chen Ridong
2025-10-20 19:45       ` Waiman Long
2025-09-28  7:12 ` [PATCH -next RFC 06/16] cpuset: introduce local_partition_enable() Chen Ridong
2025-10-20  2:44   ` Waiman Long
2025-10-20  8:06     ` Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 07/16] cpuset: introduce local_partition_disable() Chen Ridong
2025-10-20  2:46   ` Waiman Long
2025-10-20  8:06     ` Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 08/16] cpuset: introduce local_partition_invalidate() Chen Ridong
2025-10-20  2:48   ` Waiman Long
2025-10-20  8:28     ` Chen Ridong
2025-09-28  7:12 ` [PATCH -next RFC 09/16] cpuset: introduce local_partition_update() Chen Ridong
2025-10-20  2:57   ` Waiman Long
2025-10-20  9:24     ` Chen Ridong
2025-09-28  7:13 ` [PATCH -next RFC 10/16] cpuset: remove redundant partition field updates Chen Ridong
2025-09-28  7:13 ` [PATCH -next RFC 11/16] cpuset: simplify partition update logic for hotplug tasks Chen Ridong
2025-10-20  3:00   ` Waiman Long
2025-10-20  8:44     ` Chen Ridong
2025-09-28  7:13 ` [PATCH -next RFC 12/16] cpuset: unify local partition disable and invalidate Chen Ridong
2025-09-28  7:13 ` [PATCH -next RFC 13/16] cpuset: use partition_disable for compute_partition_effective_cpumask Chen Ridong
2025-10-20  3:02   ` Waiman Long
2025-10-20  8:47     ` Chen Ridong
2025-09-28  7:13 ` [PATCH -next RFC 14/16] cpuset: fix isolcpus stay in root when isolated partition changes to root Chen Ridong
2025-10-20  3:06   ` Waiman Long
2025-10-20  9:13     ` Chen Ridong
2025-10-22 10:49     ` Chen Ridong
2025-09-28  7:13 ` [PATCH -next RFC 15/16] cpuset: use partition_disable for update_prstate Chen Ridong
2025-09-28  7:13 ` [PATCH -next RFC 16/16] cpuset: remove prs_err clear when notify_partition_change Chen Ridong
2025-09-28  9:57 ` [PATCH -next RFC 00/16] cpuset: rework local partition logic Chen Ridong
2025-09-28 16:00 ` Waiman Long
2025-09-29  1:17   ` Chen Ridong
2025-10-17  1:05   ` Chen Ridong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®