[cgroup/for-6.19 PATCH 0/3] cgroup/cpuset: Additional housekeeping check & cleanup

[cgroup/for-6.19 PATCH 0/3] cgroup/cpuset: Additional housekeeping check & cleanup

[Waiman Long]

The first two patches are based on the two cpuset patches from the
"timers: Exclude isolated cpus from timer migration" patch series [1]
with some minor modifications. They add additional nohz_full housekeeping
mask check to ensure that there is at least one common housekeeping
CPU that is not domain and nohz_full isolated.

The last patch is another cleanup patch to simplify the current
partition code.

[1] https://lore.kernel.org/lkml/20251020112802.102451-1-gmonaco@redhat.com/

Gabriele Monaco (2):
  cgroup/cpuset: Rename update_unbound_workqueue_cpumask() to
    update_isolation_cpumasks()
  cgroup/cpuset: Fail if isolated and nohz_full don't leave any
    housekeeping

Waiman Long (1):
  cgroup/cpuset: Globally track isolated_cpus update

 kernel/cgroup/cpuset.c | 139 +++++++++++++++++++++++++++++------------
 1 file changed, 100 insertions(+), 39 deletions(-)

-- 
2.51.1

[cgroup/for-6.19 PATCH 1/3] cgroup/cpuset: Rename update_unbound_workqueue_cpumask() to update_isolation_cpumasks()

[Waiman Long]

From: Gabriele Monaco <gmonaco@redhat.com>

update_unbound_workqueue_cpumask() updates unbound workqueues settings
when there's a change in isolated CPUs, but it can be used for other
subsystems requiring updated when isolated CPUs change.

Generalise the name to update_isolation_cpumasks() to prepare for other
functions unrelated to workqueues to be called in that spot.

[longman: Change the function name to update_isolation_cpumasks()]

Acked-by: Frederic Weisbecker <frederic@kernel.org>
Acked-by: Waiman Long <longman@redhat.com>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 7aef59ea9627..da770dac955e 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1393,7 +1393,7 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
 	return isolcpus_updated;
 }
 
-static void update_unbound_workqueue_cpumask(bool isolcpus_updated)
+static void update_isolation_cpumasks(bool isolcpus_updated)
 {
 	int ret;
 
@@ -1557,7 +1557,7 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
 	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);
+	update_isolation_cpumasks(isolcpus_updated);
 	cpuset_force_rebuild();
 	cs->prs_err = 0;
 
@@ -1598,7 +1598,7 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 	compute_excpus(cs, cs->effective_xcpus);
 	reset_partition_data(cs);
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
+	update_isolation_cpumasks(isolcpus_updated);
 	cpuset_force_rebuild();
 
 	/*
@@ -1667,7 +1667,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 	if (xcpus)
 		cpumask_copy(cs->exclusive_cpus, xcpus);
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
+	update_isolation_cpumasks(isolcpus_updated);
 	if (adding || deleting)
 		cpuset_force_rebuild();
 
@@ -2011,7 +2011,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 							tmp->delmask);
 
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
+	update_isolation_cpumasks(isolcpus_updated);
 
 	if ((old_prs != new_prs) && (cmd == partcmd_update))
 		update_partition_exclusive_flag(cs, new_prs);
@@ -3029,7 +3029,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	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);
+	update_isolation_cpumasks(isolcpus_updated);
 
 	/* Force update if switching back to member & update effective_xcpus */
 	update_cpumasks_hier(cs, &tmpmask, !new_prs);
-- 
2.51.1

[cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Waiman Long]

From: Gabriele Monaco <gmonaco@redhat.com>

Currently the user can set up isolated cpus via cpuset and nohz_full in
such a way that leaves no housekeeping CPU (i.e. no CPU that is neither
domain isolated nor nohz full). This can be a problem for other
subsystems (e.g. the timer wheel imgration).

Prevent this configuration by blocking any assignation that would cause
the union of domain isolated cpus and nohz_full to covers all CPUs.

Acked-by: Frederic Weisbecker <frederic@kernel.org>
Reviewed-by: Waiman Long <longman@redhat.com>
Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 67 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 66 insertions(+), 1 deletion(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index da770dac955e..d6d459c95d82 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -1329,6 +1329,19 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
 		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
 }
 
+/*
+ * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
+ * @prs: new or old partition_root_state
+ * @parent: parent cpuset
+ * Return: true if isolated_cpus needs modification, false otherwise
+ */
+static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
+{
+	if (!parent)
+		parent = &top_cpuset;
+	return prs != parent->partition_root_state;
+}
+
 /*
  * partition_xcpus_add - Add new exclusive CPUs to partition
  * @new_prs: new partition_root_state
@@ -1393,6 +1406,42 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
 	return isolcpus_updated;
 }
 
+/*
+ * isolated_cpus_can_update - check for isolated & nohz_full conflicts
+ * @add_cpus: cpu mask for cpus that are going to be isolated
+ * @del_cpus: cpu mask for cpus that are no longer isolated, can be NULL
+ * Return: false if there is conflict, true otherwise
+ *
+ * If nohz_full is enabled and we have isolated CPUs, their combination must
+ * still leave housekeeping CPUs.
+ */
+static bool isolated_cpus_can_update(struct cpumask *add_cpus,
+				     struct cpumask *del_cpus)
+{
+	cpumask_var_t full_hk_cpus;
+	int res = true;
+
+	if (!housekeeping_enabled(HK_TYPE_KERNEL_NOISE))
+		return true;
+
+	if (del_cpus && cpumask_weight_and(del_cpus,
+			housekeeping_cpumask(HK_TYPE_KERNEL_NOISE)))
+		return true;
+
+	if (!alloc_cpumask_var(&full_hk_cpus, GFP_KERNEL))
+		return false;
+
+	cpumask_and(full_hk_cpus, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
+		    housekeeping_cpumask(HK_TYPE_DOMAIN));
+	cpumask_andnot(full_hk_cpus, full_hk_cpus, isolated_cpus);
+	cpumask_and(full_hk_cpus, full_hk_cpus, cpu_active_mask);
+	if (!cpumask_weight_andnot(full_hk_cpus, add_cpus))
+		res = false;
+
+	free_cpumask_var(full_hk_cpus);
+	return res;
+}
+
 static void update_isolation_cpumasks(bool isolcpus_updated)
 {
 	int ret;
@@ -1551,6 +1600,9 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
 	if (!cpumask_intersects(tmp->new_cpus, cpu_active_mask) ||
 	    cpumask_subset(top_cpuset.effective_cpus, tmp->new_cpus))
 		return PERR_INVCPUS;
+	if (isolated_cpus_should_update(new_prs, NULL) &&
+	    !isolated_cpus_can_update(tmp->new_cpus, NULL))
+		return PERR_HKEEPING;
 
 	spin_lock_irq(&callback_lock);
 	isolcpus_updated = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
@@ -1650,6 +1702,9 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 		else if (cpumask_intersects(tmp->addmask, subpartitions_cpus) ||
 			 cpumask_subset(top_cpuset.effective_cpus, tmp->addmask))
 			cs->prs_err = PERR_NOCPUS;
+		else if (isolated_cpus_should_update(prs, NULL) &&
+			 !isolated_cpus_can_update(tmp->addmask, tmp->delmask))
+			cs->prs_err = PERR_HKEEPING;
 		if (cs->prs_err)
 			goto invalidate;
 	}
@@ -1988,6 +2043,12 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 			return err;
 	}
 
+	if (deleting && isolated_cpus_should_update(new_prs, parent) &&
+	    !isolated_cpus_can_update(tmp->delmask, tmp->addmask)) {
+		cs->prs_err = PERR_HKEEPING;
+		return PERR_HKEEPING;
+	}
+
 	/*
 	 * Change the parent's effective_cpus & effective_xcpus (top cpuset
 	 * only).
@@ -2994,7 +3055,11 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		 * A change in load balance state only, no change in cpumasks.
 		 * Need to update isolated_cpus.
 		 */
-		isolcpus_updated = true;
+		if ((new_prs == PRS_ISOLATED) &&
+		    !isolated_cpus_can_update(cs->effective_xcpus, NULL))
+			err = PERR_HKEEPING;
+		else
+			isolcpus_updated = true;
 	} else {
 		/*
 		 * Switching back to member is always allowed even if it
-- 
2.51.1

[cgroup/for-6.19 PATCH 3/3] cgroup/cpuset: Globally track isolated_cpus update

[Waiman Long]

The current cpuset code passes a local isolcpus_updated flag around in a
number of functions to determine if external isolation related cpumasks
like wq_unbound_cpumask should be updated. It is a bit cumbersome and
makes the code more complex. Simplify the code by using a global boolean
flag "isolated_cpus_updating" to track this. This flag will be set in
isolated_cpus_update() and cleared in update_isolation_cpumasks().

No functional change is expected.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 74 ++++++++++++++++++++----------------------
 1 file changed, 35 insertions(+), 39 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index d6d459c95d82..406a1c3789f5 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -81,6 +81,13 @@ static cpumask_var_t	subpartitions_cpus;
  */
 static cpumask_var_t	isolated_cpus;
 
+/*
+ * isolated_cpus updating flag (protected by cpuset_mutex)
+ * Set if isolated_cpus is going to be updated in the current
+ * cpuset_mutex crtical section.
+ */
+static bool isolated_cpus_updating;
+
 /*
  * Housekeeping (HK_TYPE_DOMAIN) CPUs at boot
  */
@@ -1327,13 +1334,14 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
 		cpumask_or(isolated_cpus, isolated_cpus, xcpus);
 	else
 		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
+
+	isolated_cpus_updating = true;
 }
 
 /*
  * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
  * @prs: new or old partition_root_state
  * @parent: parent cpuset
- * Return: true if isolated_cpus needs modification, false otherwise
  */
 static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
 {
@@ -1347,15 +1355,12 @@ static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
  * @new_prs: new partition_root_state
  * @parent: parent cpuset
  * @xcpus: exclusive CPUs to be added
- * Return: true if isolated_cpus modified, false otherwise
  *
  * Remote partition if parent == NULL
  */
-static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
+static void partition_xcpus_add(int new_prs, struct cpuset *parent,
 				struct cpumask *xcpus)
 {
-	bool isolcpus_updated;
-
 	WARN_ON_ONCE(new_prs < 0);
 	lockdep_assert_held(&callback_lock);
 	if (!parent)
@@ -1365,13 +1370,11 @@ static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
 	if (parent == &top_cpuset)
 		cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
 
-	isolcpus_updated = (new_prs != parent->partition_root_state);
-	if (isolcpus_updated)
+	if (new_prs != parent->partition_root_state)
 		isolated_cpus_update(parent->partition_root_state, new_prs,
 				     xcpus);
 
 	cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
-	return isolcpus_updated;
 }
 
 /*
@@ -1379,15 +1382,12 @@ static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
  * @old_prs: old partition_root_state
  * @parent: parent cpuset
  * @xcpus: exclusive CPUs to be removed
- * Return: true if isolated_cpus modified, false otherwise
  *
  * Remote partition if parent == NULL
  */
-static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
+static void partition_xcpus_del(int old_prs, struct cpuset *parent,
 				struct cpumask *xcpus)
 {
-	bool isolcpus_updated;
-
 	WARN_ON_ONCE(old_prs < 0);
 	lockdep_assert_held(&callback_lock);
 	if (!parent)
@@ -1396,14 +1396,12 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
 	if (parent == &top_cpuset)
 		cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
 
-	isolcpus_updated = (old_prs != parent->partition_root_state);
-	if (isolcpus_updated)
+	if (old_prs != parent->partition_root_state)
 		isolated_cpus_update(old_prs, parent->partition_root_state,
 				     xcpus);
 
 	cpumask_and(xcpus, xcpus, cpu_active_mask);
 	cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
-	return isolcpus_updated;
 }
 
 /*
@@ -1442,17 +1440,24 @@ static bool isolated_cpus_can_update(struct cpumask *add_cpus,
 	return res;
 }
 
-static void update_isolation_cpumasks(bool isolcpus_updated)
+/*
+ * update_isolation_cpumasks - Update external isolation related CPU masks
+ *
+ * The following external CPU masks will be updated if necessary:
+ * - workqueue unbound cpumask
+ */
+static void update_isolation_cpumasks(void)
 {
 	int ret;
 
-	lockdep_assert_cpus_held();
-
-	if (!isolcpus_updated)
+	if (!isolated_cpus_updating)
 		return;
 
+	lockdep_assert_cpus_held();
+
 	ret = workqueue_unbound_exclude_cpumask(isolated_cpus);
 	WARN_ON_ONCE(ret < 0);
+	isolated_cpus_updating = false;
 }
 
 /**
@@ -1577,8 +1582,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.
 	 */
@@ -1605,11 +1608,11 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
 		return PERR_HKEEPING;
 
 	spin_lock_irq(&callback_lock);
-	isolcpus_updated = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
+	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_isolation_cpumasks(isolcpus_updated);
+	update_isolation_cpumasks();
 	cpuset_force_rebuild();
 	cs->prs_err = 0;
 
@@ -1632,15 +1635,12 @@ 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;
-
 	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);
+	partition_xcpus_del(cs->partition_root_state, NULL, cs->effective_xcpus);
 	if (cs->prs_err)
 		cs->partition_root_state = -cs->partition_root_state;
 	else
@@ -1650,7 +1650,7 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 	compute_excpus(cs, cs->effective_xcpus);
 	reset_partition_data(cs);
 	spin_unlock_irq(&callback_lock);
-	update_isolation_cpumasks(isolcpus_updated);
+	update_isolation_cpumasks();
 	cpuset_force_rebuild();
 
 	/*
@@ -1675,7 +1675,6 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 {
 	bool adding, deleting;
 	int prs = cs->partition_root_state;
-	int isolcpus_updated = 0;
 
 	if (WARN_ON_ONCE(!is_remote_partition(cs)))
 		return;
@@ -1711,9 +1710,9 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 
 	spin_lock_irq(&callback_lock);
 	if (adding)
-		isolcpus_updated += partition_xcpus_add(prs, NULL, tmp->addmask);
+		partition_xcpus_add(prs, NULL, tmp->addmask);
 	if (deleting)
-		isolcpus_updated += partition_xcpus_del(prs, NULL, tmp->delmask);
+		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.
@@ -1722,7 +1721,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 	if (xcpus)
 		cpumask_copy(cs->exclusive_cpus, xcpus);
 	spin_unlock_irq(&callback_lock);
-	update_isolation_cpumasks(isolcpus_updated);
+	update_isolation_cpumasks();
 	if (adding || deleting)
 		cpuset_force_rebuild();
 
@@ -1803,7 +1802,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	int deleting;	/* Deleting cpus from parent's effective_cpus	*/
 	int old_prs, new_prs;
 	int part_error = PERR_NONE;	/* Partition error? */
-	int isolcpus_updated = 0;
 	struct cpumask *xcpus = user_xcpus(cs);
 	bool nocpu;
 
@@ -2065,14 +2063,12 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	 * and vice versa.
 	 */
 	if (adding)
-		isolcpus_updated += partition_xcpus_del(old_prs, parent,
-							tmp->addmask);
+		partition_xcpus_del(old_prs, parent, tmp->addmask);
 	if (deleting)
-		isolcpus_updated += partition_xcpus_add(new_prs, parent,
-							tmp->delmask);
+		partition_xcpus_add(new_prs, parent, tmp->delmask);
 
 	spin_unlock_irq(&callback_lock);
-	update_isolation_cpumasks(isolcpus_updated);
+	update_isolation_cpumasks();
 
 	if ((old_prs != new_prs) && (cmd == partcmd_update))
 		update_partition_exclusive_flag(cs, new_prs);
@@ -3094,7 +3090,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	else if (isolcpus_updated)
 		isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
 	spin_unlock_irq(&callback_lock);
-	update_isolation_cpumasks(isolcpus_updated);
+	update_isolation_cpumasks();
 
 	/* Force update if switching back to member & update effective_xcpus */
 	update_cpumasks_hier(cs, &tmpmask, !new_prs);
-- 
2.51.1

Re: [cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Chen Ridong]

On 2025/11/3 9:34, Waiman Long wrote:
> From: Gabriele Monaco <gmonaco@redhat.com>
> 
> Currently the user can set up isolated cpus via cpuset and nohz_full in
> such a way that leaves no housekeeping CPU (i.e. no CPU that is neither
> domain isolated nor nohz full). This can be a problem for other
> subsystems (e.g. the timer wheel imgration).
> 
> Prevent this configuration by blocking any assignation that would cause
> the union of domain isolated cpus and nohz_full to covers all CPUs.
> 
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> Reviewed-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  kernel/cgroup/cpuset.c | 67 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index da770dac955e..d6d459c95d82 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1329,6 +1329,19 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>  		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
>  }
>  
> +/*
> + * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
> + * @prs: new or old partition_root_state
> + * @parent: parent cpuset
> + * Return: true if isolated_cpus needs modification, false otherwise
> + */
> +static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
> +{
> +	if (!parent)
> +		parent = &top_cpuset;
> +	return prs != parent->partition_root_state;
> +}
> +

Hi Longman,

I am confused about this function.

Why do we need to compare the partition_root_state (prs) with the parent's partition_root_state?

For example, when a local partition is assigned to a member, I don't think the isolated cpumasks
should be updated in this case.

In my understanding, the isolated CPUs should only be updated when an isolated partition is being
disabled or enabled. I was thinking of something like this:

bool isolated_cpus_should_update(int new_prs, int old_prs)
{
    if (new_prs == old_prs)
        return false;
    if (old_prs == 2 || new_prs == 2)
        return true;
    return false;
}

I would really appreciate it if you could provide some further explanation on this.

-- 
Best regards,
Ridong

Re: [cgroup/for-6.19 PATCH 1/3] cgroup/cpuset: Rename update_unbound_workqueue_cpumask() to update_isolation_cpumasks()

[Chen Ridong]

On 2025/11/3 9:34, Waiman Long wrote:
> From: Gabriele Monaco <gmonaco@redhat.com>
> 
> update_unbound_workqueue_cpumask() updates unbound workqueues settings
> when there's a change in isolated CPUs, but it can be used for other
> subsystems requiring updated when isolated CPUs change.
> 
> Generalise the name to update_isolation_cpumasks() to prepare for other
> functions unrelated to workqueues to be called in that spot.
> 
> [longman: Change the function name to update_isolation_cpumasks()]
> 
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> Acked-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  kernel/cgroup/cpuset.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 7aef59ea9627..da770dac955e 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1393,7 +1393,7 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
>  	return isolcpus_updated;
>  }
>  
> -static void update_unbound_workqueue_cpumask(bool isolcpus_updated)
> +static void update_isolation_cpumasks(bool isolcpus_updated)
>  {
>  	int ret;
>  
> @@ -1557,7 +1557,7 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
>  	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);
> +	update_isolation_cpumasks(isolcpus_updated);
>  	cpuset_force_rebuild();
>  	cs->prs_err = 0;
>  
> @@ -1598,7 +1598,7 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
>  	compute_excpus(cs, cs->effective_xcpus);
>  	reset_partition_data(cs);
>  	spin_unlock_irq(&callback_lock);
> -	update_unbound_workqueue_cpumask(isolcpus_updated);
> +	update_isolation_cpumasks(isolcpus_updated);
>  	cpuset_force_rebuild();
>  
>  	/*
> @@ -1667,7 +1667,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>  	if (xcpus)
>  		cpumask_copy(cs->exclusive_cpus, xcpus);
>  	spin_unlock_irq(&callback_lock);
> -	update_unbound_workqueue_cpumask(isolcpus_updated);
> +	update_isolation_cpumasks(isolcpus_updated);
>  	if (adding || deleting)
>  		cpuset_force_rebuild();
>  
> @@ -2011,7 +2011,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  							tmp->delmask);
>  
>  	spin_unlock_irq(&callback_lock);
> -	update_unbound_workqueue_cpumask(isolcpus_updated);
> +	update_isolation_cpumasks(isolcpus_updated);
>  
>  	if ((old_prs != new_prs) && (cmd == partcmd_update))
>  		update_partition_exclusive_flag(cs, new_prs);
> @@ -3029,7 +3029,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>  	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);
> +	update_isolation_cpumasks(isolcpus_updated);
>  
>  	/* Force update if switching back to member & update effective_xcpus */
>  	update_cpumasks_hier(cs, &tmpmask, !new_prs);

LGTM

Reviewed-by: Chen Ridong<chenridong@huawei.com>

-- 
Best regards,
Ridong

Re: [cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Waiman Long]

On 11/2/25 9:55 PM, Chen Ridong wrote:
>
> On 2025/11/3 9:34, Waiman Long wrote:
>> From: Gabriele Monaco <gmonaco@redhat.com>
>>
>> Currently the user can set up isolated cpus via cpuset and nohz_full in
>> such a way that leaves no housekeeping CPU (i.e. no CPU that is neither
>> domain isolated nor nohz full). This can be a problem for other
>> subsystems (e.g. the timer wheel imgration).
>>
>> Prevent this configuration by blocking any assignation that would cause
>> the union of domain isolated cpus and nohz_full to covers all CPUs.
>>
>> Acked-by: Frederic Weisbecker <frederic@kernel.org>
>> Reviewed-by: Waiman Long <longman@redhat.com>
>> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   kernel/cgroup/cpuset.c | 67 +++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index da770dac955e..d6d459c95d82 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1329,6 +1329,19 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>>   		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
>>   }
>>   
>> +/*
>> + * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
>> + * @prs: new or old partition_root_state
>> + * @parent: parent cpuset
>> + * Return: true if isolated_cpus needs modification, false otherwise
>> + */
>> +static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
>> +{
>> +	if (!parent)
>> +		parent = &top_cpuset;
>> +	return prs != parent->partition_root_state;
>> +}
>> +
> Hi Longman,
>
> I am confused about this function.
>
> Why do we need to compare the partition_root_state (prs) with the parent's partition_root_state?
>
> For example, when a local partition is assigned to a member, I don't think the isolated cpumasks
> should be updated in this case.
>
> In my understanding, the isolated CPUs should only be updated when an isolated partition is being
> disabled or enabled. I was thinking of something like this:
>
> bool isolated_cpus_should_update(int new_prs, int old_prs)
> {
>      if (new_prs == old_prs)
>          return false;
>      if (old_prs == 2 || new_prs == 2)
>          return true;
>      return false;
> }
>
> I would really appreciate it if you could provide some further explanation on this.

This function should only be called when both the current and the parent 
(top_cpuset for remote) are valid partition roots. For both local and 
remote partition, the child cpuset takes CPUs from the parent. The list 
of isolated CPUs will only change if parent and child cpusets have 
different partition root types. If parent is an isolated partition, 
taking CPUs from parent to form another isolated partition will not 
change isolated_cpus. The same is true if both parent and child are root.

You are right that this check may not be obvious for a casual observer. 
I can add some more comments to clarify that.

Cheers,
Longman

>

Re: [cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Chen Ridong]

On 2025/11/3 11:12, Waiman Long wrote:
> On 11/2/25 9:55 PM, Chen Ridong wrote:
>>
>> On 2025/11/3 9:34, Waiman Long wrote:
>>> From: Gabriele Monaco <gmonaco@redhat.com>
>>>
>>> Currently the user can set up isolated cpus via cpuset and nohz_full in
>>> such a way that leaves no housekeeping CPU (i.e. no CPU that is neither
>>> domain isolated nor nohz full). This can be a problem for other
>>> subsystems (e.g. the timer wheel imgration).
>>>
>>> Prevent this configuration by blocking any assignation that would cause
>>> the union of domain isolated cpus and nohz_full to covers all CPUs.
>>>
>>> Acked-by: Frederic Weisbecker <frederic@kernel.org>
>>> Reviewed-by: Waiman Long <longman@redhat.com>
>>> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
>>> Signed-off-by: Waiman Long <longman@redhat.com>
>>> ---
>>>   kernel/cgroup/cpuset.c | 67 +++++++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 66 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>>> index da770dac955e..d6d459c95d82 100644
>>> --- a/kernel/cgroup/cpuset.c
>>> +++ b/kernel/cgroup/cpuset.c
>>> @@ -1329,6 +1329,19 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask
>>> *xcpus
>>>           cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
>>>   }
>>>   +/*
>>> + * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
>>> + * @prs: new or old partition_root_state
>>> + * @parent: parent cpuset
>>> + * Return: true if isolated_cpus needs modification, false otherwise
>>> + */
>>> +static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
>>> +{
>>> +    if (!parent)
>>> +        parent = &top_cpuset;
>>> +    return prs != parent->partition_root_state;
>>> +}
>>> +
>> Hi Longman,
>>
>> I am confused about this function.
>>
>> Why do we need to compare the partition_root_state (prs) with the parent's partition_root_state?
>>
>> For example, when a local partition is assigned to a member, I don't think the isolated cpumasks
>> should be updated in this case.
>>
>> In my understanding, the isolated CPUs should only be updated when an isolated partition is being
>> disabled or enabled. I was thinking of something like this:
>>
>> bool isolated_cpus_should_update(int new_prs, int old_prs)
>> {
>>      if (new_prs == old_prs)
>>          return false;
>>      if (old_prs == 2 || new_prs == 2)
>>          return true;
>>      return false;
>> }
>>
>> I would really appreciate it if you could provide some further explanation on this.
> 
> This function should only be called when both the current and the parent (top_cpuset for remote) are
> valid partition roots. For both local and remote partition, the child cpuset takes CPUs from the
> parent. The list of isolated CPUs will only change if parent and child cpusets have different
> partition root types. If parent is an isolated partition, taking CPUs from parent to form another
> isolated partition will not change isolated_cpus. The same is true if both parent and child are root.
> 
> You are right that this check may not be obvious for a casual observer. I can add some more comments
> to clarify that.
> 

Thank you for your explanation.

Yes, this can be a common scenario.

More comments will be helpful.

-- 
Best regards,
Ridong

Re: [cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Chen Ridong]

On 2025/11/3 9:34, Waiman Long wrote:
> From: Gabriele Monaco <gmonaco@redhat.com>
> 
> Currently the user can set up isolated cpus via cpuset and nohz_full in
> such a way that leaves no housekeeping CPU (i.e. no CPU that is neither
> domain isolated nor nohz full). This can be a problem for other
> subsystems (e.g. the timer wheel imgration).
> 
> Prevent this configuration by blocking any assignation that would cause
> the union of domain isolated cpus and nohz_full to covers all CPUs.
> 
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> Reviewed-by: Waiman Long <longman@redhat.com>
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  kernel/cgroup/cpuset.c | 67 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index da770dac955e..d6d459c95d82 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -1329,6 +1329,19 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>  		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
>  }
>  
> +/*
> + * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
> + * @prs: new or old partition_root_state
> + * @parent: parent cpuset
> + * Return: true if isolated_cpus needs modification, false otherwise
> + */
> +static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
> +{
> +	if (!parent)
> +		parent = &top_cpuset;
> +	return prs != parent->partition_root_state;
> +}
> +
>  /*
>   * partition_xcpus_add - Add new exclusive CPUs to partition
>   * @new_prs: new partition_root_state
> @@ -1393,6 +1406,42 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
>  	return isolcpus_updated;
>  }
>  
> +/*
> + * isolated_cpus_can_update - check for isolated & nohz_full conflicts
> + * @add_cpus: cpu mask for cpus that are going to be isolated
> + * @del_cpus: cpu mask for cpus that are no longer isolated, can be NULL
> + * Return: false if there is conflict, true otherwise
> + *
> + * If nohz_full is enabled and we have isolated CPUs, their combination must
> + * still leave housekeeping CPUs.
> + */
> +static bool isolated_cpus_can_update(struct cpumask *add_cpus,
> +				     struct cpumask *del_cpus)
> +{
> +	cpumask_var_t full_hk_cpus;
> +	int res = true;
> +
> +	if (!housekeeping_enabled(HK_TYPE_KERNEL_NOISE))
> +		return true;
> +
> +	if (del_cpus && cpumask_weight_and(del_cpus,
> +			housekeeping_cpumask(HK_TYPE_KERNEL_NOISE)))
> +		return true;
> +
> +	if (!alloc_cpumask_var(&full_hk_cpus, GFP_KERNEL))
> +		return false;
> +
> +	cpumask_and(full_hk_cpus, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
> +		    housekeeping_cpumask(HK_TYPE_DOMAIN));
> +	cpumask_andnot(full_hk_cpus, full_hk_cpus, isolated_cpus);
> +	cpumask_and(full_hk_cpus, full_hk_cpus, cpu_active_mask);
> +	if (!cpumask_weight_andnot(full_hk_cpus, add_cpus))
> +		res = false;
> +
> +	free_cpumask_var(full_hk_cpus);
> +	return res;
> +}
> +
>  static void update_isolation_cpumasks(bool isolcpus_updated)
>  {
>  	int ret;
> @@ -1551,6 +1600,9 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
>  	if (!cpumask_intersects(tmp->new_cpus, cpu_active_mask) ||
>  	    cpumask_subset(top_cpuset.effective_cpus, tmp->new_cpus))
>  		return PERR_INVCPUS;
> +	if (isolated_cpus_should_update(new_prs, NULL) &&
> +	    !isolated_cpus_can_update(tmp->new_cpus, NULL))
> +		return PERR_HKEEPING;
>  
>  	spin_lock_irq(&callback_lock);
>  	isolcpus_updated = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
> @@ -1650,6 +1702,9 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>  		else if (cpumask_intersects(tmp->addmask, subpartitions_cpus) ||
>  			 cpumask_subset(top_cpuset.effective_cpus, tmp->addmask))
>  			cs->prs_err = PERR_NOCPUS;
> +		else if (isolated_cpus_should_update(prs, NULL) &&
> +			 !isolated_cpus_can_update(tmp->addmask, tmp->delmask))
> +			cs->prs_err = PERR_HKEEPING;
>  		if (cs->prs_err)
>  			goto invalidate;
>  	}
> @@ -1988,6 +2043,12 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  			return err;
>  	}
>  
> +	if (deleting && isolated_cpus_should_update(new_prs, parent) &&
> +	    !isolated_cpus_can_update(tmp->delmask, tmp->addmask)) {
> +		cs->prs_err = PERR_HKEEPING;
> +		return PERR_HKEEPING;
> +	}
> +
>  	/*
>  	 * Change the parent's effective_cpus & effective_xcpus (top cpuset
>  	 * only).
> @@ -2994,7 +3055,11 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>  		 * A change in load balance state only, no change in cpumasks.
>  		 * Need to update isolated_cpus.
>  		 */
> -		isolcpus_updated = true;
> +		if ((new_prs == PRS_ISOLATED) &&
> +		    !isolated_cpus_can_update(cs->effective_xcpus, NULL))
> +			err = PERR_HKEEPING;
> +		else
> +			isolcpus_updated = true;
>  	} else {
>  		/*
>  		 * Switching back to member is always allowed even if it

I'm considering whether I should introduce a new function that consolidates
isolated_cpus_should_update, isolated_cpus_can_update, and prstate_housekeeping_conflict.

Just like:

bool housekeeping_conflict(...)
{
	if (isolated_cpus_should_update && !isolated_cpus_can_update) {
		return ture;
	}
	return prstate_housekeeping_conflict();
}

Since all of these are related to isolated CPUs, putting them into a centralized function would make
the code much easier to maintain.

-- 
Best regards,
Ridong

Re: [cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Chen Ridong]

> +
>  	/*
>  	 * Change the parent's effective_cpus & effective_xcpus (top cpuset
>  	 * only).
> @@ -2994,7 +3055,11 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>  		 * A change in load balance state only, no change in cpumasks.
>  		 * Need to update isolated_cpus.
>  		 */
> -		isolcpus_updated = true;
> +		if ((new_prs == PRS_ISOLATED) &&
> +		    !isolated_cpus_can_update(cs->effective_xcpus, NULL))
> +			err = PERR_HKEEPING;
> +		else
> +			isolcpus_updated = true;
>  	} else {
>  		/*
>  		 * Switching back to member is always allowed even if it

This is an issue I am trying to fix, the prstate_housekeeping_conflict check is necessary.

https://lore.kernel.org/cgroups/20251025064844.495525-2-chenridong@huaweicloud.com/

-- 
Best regards,
Ridong

Re: [cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Chen Ridong]

On 2025/11/3 11:47, Chen Ridong wrote:
> 
> 
> On 2025/11/3 9:34, Waiman Long wrote:
>> From: Gabriele Monaco <gmonaco@redhat.com>
>>
>> Currently the user can set up isolated cpus via cpuset and nohz_full in
>> such a way that leaves no housekeeping CPU (i.e. no CPU that is neither
>> domain isolated nor nohz full). This can be a problem for other
>> subsystems (e.g. the timer wheel imgration).
>>
>> Prevent this configuration by blocking any assignation that would cause
>> the union of domain isolated cpus and nohz_full to covers all CPUs.
>>
>> Acked-by: Frederic Weisbecker <frederic@kernel.org>
>> Reviewed-by: Waiman Long <longman@redhat.com>
>> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>  kernel/cgroup/cpuset.c | 67 +++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index da770dac955e..d6d459c95d82 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -1329,6 +1329,19 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>>  		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
>>  }
>>  
>> +/*
>> + * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
>> + * @prs: new or old partition_root_state
>> + * @parent: parent cpuset
>> + * Return: true if isolated_cpus needs modification, false otherwise
>> + */
>> +static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
>> +{
>> +	if (!parent)
>> +		parent = &top_cpuset;
>> +	return prs != parent->partition_root_state;
>> +}
>> +
>>  /*
>>   * partition_xcpus_add - Add new exclusive CPUs to partition
>>   * @new_prs: new partition_root_state
>> @@ -1393,6 +1406,42 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
>>  	return isolcpus_updated;
>>  }
>>  
>> +/*
>> + * isolated_cpus_can_update - check for isolated & nohz_full conflicts
>> + * @add_cpus: cpu mask for cpus that are going to be isolated
>> + * @del_cpus: cpu mask for cpus that are no longer isolated, can be NULL
>> + * Return: false if there is conflict, true otherwise
>> + *
>> + * If nohz_full is enabled and we have isolated CPUs, their combination must
>> + * still leave housekeeping CPUs.
>> + */
>> +static bool isolated_cpus_can_update(struct cpumask *add_cpus,
>> +				     struct cpumask *del_cpus)
>> +{
>> +	cpumask_var_t full_hk_cpus;
>> +	int res = true;
>> +
>> +	if (!housekeeping_enabled(HK_TYPE_KERNEL_NOISE))
>> +		return true;
>> +
>> +	if (del_cpus && cpumask_weight_and(del_cpus,
>> +			housekeeping_cpumask(HK_TYPE_KERNEL_NOISE)))
>> +		return true;
>> +
>> +	if (!alloc_cpumask_var(&full_hk_cpus, GFP_KERNEL))
>> +		return false;
>> +
>> +	cpumask_and(full_hk_cpus, housekeeping_cpumask(HK_TYPE_KERNEL_NOISE),
>> +		    housekeeping_cpumask(HK_TYPE_DOMAIN));
>> +	cpumask_andnot(full_hk_cpus, full_hk_cpus, isolated_cpus);
>> +	cpumask_and(full_hk_cpus, full_hk_cpus, cpu_active_mask);
>> +	if (!cpumask_weight_andnot(full_hk_cpus, add_cpus))
>> +		res = false;
>> +
>> +	free_cpumask_var(full_hk_cpus);
>> +	return res;
>> +}
>> +
>>  static void update_isolation_cpumasks(bool isolcpus_updated)
>>  {
>>  	int ret;
>> @@ -1551,6 +1600,9 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
>>  	if (!cpumask_intersects(tmp->new_cpus, cpu_active_mask) ||
>>  	    cpumask_subset(top_cpuset.effective_cpus, tmp->new_cpus))
>>  		return PERR_INVCPUS;
>> +	if (isolated_cpus_should_update(new_prs, NULL) &&
>> +	    !isolated_cpus_can_update(tmp->new_cpus, NULL))
>> +		return PERR_HKEEPING;
>>  
>>  	spin_lock_irq(&callback_lock);
>>  	isolcpus_updated = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
>> @@ -1650,6 +1702,9 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>>  		else if (cpumask_intersects(tmp->addmask, subpartitions_cpus) ||
>>  			 cpumask_subset(top_cpuset.effective_cpus, tmp->addmask))
>>  			cs->prs_err = PERR_NOCPUS;
>> +		else if (isolated_cpus_should_update(prs, NULL) &&
>> +			 !isolated_cpus_can_update(tmp->addmask, tmp->delmask))
>> +			cs->prs_err = PERR_HKEEPING;
>>  		if (cs->prs_err)
>>  			goto invalidate;
>>  	}
>> @@ -1988,6 +2043,12 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>>  			return err;
>>  	}
>>  
>> +	if (deleting && isolated_cpus_should_update(new_prs, parent) &&
>> +	    !isolated_cpus_can_update(tmp->delmask, tmp->addmask)) {
>> +		cs->prs_err = PERR_HKEEPING;
>> +		return PERR_HKEEPING;
>> +	}
>> +
>>  	/*
>>  	 * Change the parent's effective_cpus & effective_xcpus (top cpuset
>>  	 * only).
>> @@ -2994,7 +3055,11 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>  		 * A change in load balance state only, no change in cpumasks.
>>  		 * Need to update isolated_cpus.
>>  		 */
>> -		isolcpus_updated = true;
>> +		if ((new_prs == PRS_ISOLATED) &&
>> +		    !isolated_cpus_can_update(cs->effective_xcpus, NULL))
>> +			err = PERR_HKEEPING;
>> +		else
>> +			isolcpus_updated = true;
>>  	} else {
>>  		/*
>>  		 * Switching back to member is always allowed even if it
> 
> I'm considering whether I should introduce a new function that consolidates
> isolated_cpus_should_update, isolated_cpus_can_update, and prstate_housekeeping_conflict.
> 

Sorry, we should introduce a new ...

> Just like:
> 
> bool housekeeping_conflict(...)
> {
> 	if (isolated_cpus_should_update && !isolated_cpus_can_update) {
> 		return ture;
> 	}
> 	return prstate_housekeeping_conflict();
> }
> 
> Since all of these are related to isolated CPUs, putting them into a centralized function would make
> the code much easier to maintain.
> 

-- 
Best regards,
Ridong

Re: [cgroup/for-6.19 PATCH 3/3] cgroup/cpuset: Globally track isolated_cpus update

[Chen Ridong]

On 2025/11/3 9:34, Waiman Long wrote:
> The current cpuset code passes a local isolcpus_updated flag around in a
> number of functions to determine if external isolation related cpumasks
> like wq_unbound_cpumask should be updated. It is a bit cumbersome and
> makes the code more complex. Simplify the code by using a global boolean

Agree.

> flag "isolated_cpus_updating" to track this. This flag will be set in
> isolated_cpus_update() and cleared in update_isolation_cpumasks().
> 
> No functional change is expected.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  kernel/cgroup/cpuset.c | 74 ++++++++++++++++++++----------------------
>  1 file changed, 35 insertions(+), 39 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index d6d459c95d82..406a1c3789f5 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -81,6 +81,13 @@ static cpumask_var_t	subpartitions_cpus;
>   */
>  static cpumask_var_t	isolated_cpus;
>  

Is isolated_cpus protected by cpuset_mutex or callback_lock?

If isolated_cpus is indeed protected by cpuset_mutex, perhaps we can move the update of
isolated_cpus outside the critical section of callback_lock. This would allow us to call
update_isolation_cpumasks in isolated_cpus_update, making the isolated_cpus_updating variable
unnecessary. Reducing a global variable would be beneficial.

> +/*
> + * isolated_cpus updating flag (protected by cpuset_mutex)
> + * Set if isolated_cpus is going to be updated in the current
> + * cpuset_mutex crtical section.
> + */
> +static bool isolated_cpus_updating;
> +
>  /*
>   * Housekeeping (HK_TYPE_DOMAIN) CPUs at boot
>   */
> @@ -1327,13 +1334,14 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>  		cpumask_or(isolated_cpus, isolated_cpus, xcpus);
>  	else
>  		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
> +
> +	isolated_cpus_updating = true;
>  }
>  
>  /*
>   * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
>   * @prs: new or old partition_root_state
>   * @parent: parent cpuset
> - * Return: true if isolated_cpus needs modification, false otherwise
>   */
>  static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
>  {
> @@ -1347,15 +1355,12 @@ static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
>   * @new_prs: new partition_root_state
>   * @parent: parent cpuset
>   * @xcpus: exclusive CPUs to be added
> - * Return: true if isolated_cpus modified, false otherwise
>   *
>   * Remote partition if parent == NULL
>   */
> -static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
> +static void partition_xcpus_add(int new_prs, struct cpuset *parent,
>  				struct cpumask *xcpus)
>  {
> -	bool isolcpus_updated;
> -
>  	WARN_ON_ONCE(new_prs < 0);
>  	lockdep_assert_held(&callback_lock);
>  	if (!parent)
> @@ -1365,13 +1370,11 @@ static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
>  	if (parent == &top_cpuset)
>  		cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>  
> -	isolcpus_updated = (new_prs != parent->partition_root_state);
> -	if (isolcpus_updated)
> +	if (new_prs != parent->partition_root_state)

Can this if statement be replaced with new helper isolated_cpus_should_update？

>  		isolated_cpus_update(parent->partition_root_state, new_prs,
>  				     xcpus);
>  
>  	cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
> -	return isolcpus_updated;
>  }
>  
>  /*
> @@ -1379,15 +1382,12 @@ static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
>   * @old_prs: old partition_root_state
>   * @parent: parent cpuset
>   * @xcpus: exclusive CPUs to be removed
> - * Return: true if isolated_cpus modified, false otherwise
>   *
>   * Remote partition if parent == NULL
>   */
> -static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
> +static void partition_xcpus_del(int old_prs, struct cpuset *parent,
>  				struct cpumask *xcpus)
>  {
> -	bool isolcpus_updated;
> -
>  	WARN_ON_ONCE(old_prs < 0);
>  	lockdep_assert_held(&callback_lock);
>  	if (!parent)
> @@ -1396,14 +1396,12 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
>  	if (parent == &top_cpuset)
>  		cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
>  
> -	isolcpus_updated = (old_prs != parent->partition_root_state);
> -	if (isolcpus_updated)
> +	if (old_prs != parent->partition_root_state)
>  		isolated_cpus_update(old_prs, parent->partition_root_state,
>  				     xcpus);
>  

Ditto

>  	cpumask_and(xcpus, xcpus, cpu_active_mask);
>  	cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
> -	return isolcpus_updated;
>  }
>  
>  /*
> @@ -1442,17 +1440,24 @@ static bool isolated_cpus_can_update(struct cpumask *add_cpus,
>  	return res;
>  }
>  
> -static void update_isolation_cpumasks(bool isolcpus_updated)
> +/*
> + * update_isolation_cpumasks - Update external isolation related CPU masks
> + *
> + * The following external CPU masks will be updated if necessary:
> + * - workqueue unbound cpumask
> + */
> +static void update_isolation_cpumasks(void)
>  {
>  	int ret;
>  
> -	lockdep_assert_cpus_held();
> -
> -	if (!isolcpus_updated)
> +	if (!isolated_cpus_updating)
>  		return;
>  
> +	lockdep_assert_cpus_held();
> +
>  	ret = workqueue_unbound_exclude_cpumask(isolated_cpus);
>  	WARN_ON_ONCE(ret < 0);
> +	isolated_cpus_updating = false;
>  }
>  
>  /**
> @@ -1577,8 +1582,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.
>  	 */
> @@ -1605,11 +1608,11 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
>  		return PERR_HKEEPING;
>  
>  	spin_lock_irq(&callback_lock);
> -	isolcpus_updated = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
> +	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_isolation_cpumasks(isolcpus_updated);
> +	update_isolation_cpumasks();
>  	cpuset_force_rebuild();
>  	cs->prs_err = 0;
>  
> @@ -1632,15 +1635,12 @@ 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;
> -
>  	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);
> +	partition_xcpus_del(cs->partition_root_state, NULL, cs->effective_xcpus);
>  	if (cs->prs_err)
>  		cs->partition_root_state = -cs->partition_root_state;
>  	else
> @@ -1650,7 +1650,7 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
>  	compute_excpus(cs, cs->effective_xcpus);
>  	reset_partition_data(cs);
>  	spin_unlock_irq(&callback_lock);
> -	update_isolation_cpumasks(isolcpus_updated);
> +	update_isolation_cpumasks();
>  	cpuset_force_rebuild();
>  
>  	/*
> @@ -1675,7 +1675,6 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>  {
>  	bool adding, deleting;
>  	int prs = cs->partition_root_state;
> -	int isolcpus_updated = 0;
>  
>  	if (WARN_ON_ONCE(!is_remote_partition(cs)))
>  		return;
> @@ -1711,9 +1710,9 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>  
>  	spin_lock_irq(&callback_lock);
>  	if (adding)
> -		isolcpus_updated += partition_xcpus_add(prs, NULL, tmp->addmask);
> +		partition_xcpus_add(prs, NULL, tmp->addmask);
>  	if (deleting)
> -		isolcpus_updated += partition_xcpus_del(prs, NULL, tmp->delmask);
> +		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.
> @@ -1722,7 +1721,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
>  	if (xcpus)
>  		cpumask_copy(cs->exclusive_cpus, xcpus);
>  	spin_unlock_irq(&callback_lock);
> -	update_isolation_cpumasks(isolcpus_updated);
> +	update_isolation_cpumasks();
>  	if (adding || deleting)
>  		cpuset_force_rebuild();
>  
> @@ -1803,7 +1802,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  	int deleting;	/* Deleting cpus from parent's effective_cpus	*/
>  	int old_prs, new_prs;
>  	int part_error = PERR_NONE;	/* Partition error? */
> -	int isolcpus_updated = 0;
>  	struct cpumask *xcpus = user_xcpus(cs);
>  	bool nocpu;
>  
> @@ -2065,14 +2063,12 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  	 * and vice versa.
>  	 */
>  	if (adding)
> -		isolcpus_updated += partition_xcpus_del(old_prs, parent,
> -							tmp->addmask);
> +		partition_xcpus_del(old_prs, parent, tmp->addmask);
>  	if (deleting)
> -		isolcpus_updated += partition_xcpus_add(new_prs, parent,
> -							tmp->delmask);
> +		partition_xcpus_add(new_prs, parent, tmp->delmask);
>  
>  	spin_unlock_irq(&callback_lock);
> -	update_isolation_cpumasks(isolcpus_updated);
> +	update_isolation_cpumasks();
>  
>  	if ((old_prs != new_prs) && (cmd == partcmd_update))
>  		update_partition_exclusive_flag(cs, new_prs);
> @@ -3094,7 +3090,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>  	else if (isolcpus_updated)
>  		isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
>  	spin_unlock_irq(&callback_lock);
> -	update_isolation_cpumasks(isolcpus_updated);
> +	update_isolation_cpumasks();
>  
>  	/* Force update if switching back to member & update effective_xcpus */
>  	update_cpumasks_hier(cs, &tmpmask, !new_prs);

-- 
Best regards,
Ridong

Re: [cgroup/for-6.19 PATCH 3/3] cgroup/cpuset: Globally track isolated_cpus update

[Waiman Long]

On 11/3/25 1:46 AM, Chen Ridong wrote:
>
> On 2025/11/3 9:34, Waiman Long wrote:
>> The current cpuset code passes a local isolcpus_updated flag around in a
>> number of functions to determine if external isolation related cpumasks
>> like wq_unbound_cpumask should be updated. It is a bit cumbersome and
>> makes the code more complex. Simplify the code by using a global boolean
> Agree.
>
>> flag "isolated_cpus_updating" to track this. This flag will be set in
>> isolated_cpus_update() and cleared in update_isolation_cpumasks().
>>
>> No functional change is expected.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
>> ---
>>   kernel/cgroup/cpuset.c | 74 ++++++++++++++++++++----------------------
>>   1 file changed, 35 insertions(+), 39 deletions(-)
>>
>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
>> index d6d459c95d82..406a1c3789f5 100644
>> --- a/kernel/cgroup/cpuset.c
>> +++ b/kernel/cgroup/cpuset.c
>> @@ -81,6 +81,13 @@ static cpumask_var_t	subpartitions_cpus;
>>    */
>>   static cpumask_var_t	isolated_cpus;
>>   
> Is isolated_cpus protected by cpuset_mutex or callback_lock?
>
> If isolated_cpus is indeed protected by cpuset_mutex, perhaps we can move the update of
> isolated_cpus outside the critical section of callback_lock. This would allow us to call
> update_isolation_cpumasks in isolated_cpus_update, making the isolated_cpus_updating variable
> unnecessary. Reducing a global variable would be beneficial.

isolated_cpus is a user visible cpumask. So I would like to protect it 
with both cpuset_mutex and callback_lock like the other user visible 
cpumasks.


>
>> +/*
>> + * isolated_cpus updating flag (protected by cpuset_mutex)
>> + * Set if isolated_cpus is going to be updated in the current
>> + * cpuset_mutex crtical section.
>> + */
>> +static bool isolated_cpus_updating;
>> +
>>   /*
>>    * Housekeeping (HK_TYPE_DOMAIN) CPUs at boot
>>    */
>> @@ -1327,13 +1334,14 @@ static void isolated_cpus_update(int old_prs, int new_prs, struct cpumask *xcpus
>>   		cpumask_or(isolated_cpus, isolated_cpus, xcpus);
>>   	else
>>   		cpumask_andnot(isolated_cpus, isolated_cpus, xcpus);
>> +
>> +	isolated_cpus_updating = true;
>>   }
>>   
>>   /*
>>    * isolated_cpus_should_update - Returns if the isolated_cpus mask needs update
>>    * @prs: new or old partition_root_state
>>    * @parent: parent cpuset
>> - * Return: true if isolated_cpus needs modification, false otherwise
>>    */
>>   static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
>>   {
>> @@ -1347,15 +1355,12 @@ static bool isolated_cpus_should_update(int prs, struct cpuset *parent)
>>    * @new_prs: new partition_root_state
>>    * @parent: parent cpuset
>>    * @xcpus: exclusive CPUs to be added
>> - * Return: true if isolated_cpus modified, false otherwise
>>    *
>>    * Remote partition if parent == NULL
>>    */
>> -static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
>> +static void partition_xcpus_add(int new_prs, struct cpuset *parent,
>>   				struct cpumask *xcpus)
>>   {
>> -	bool isolcpus_updated;
>> -
>>   	WARN_ON_ONCE(new_prs < 0);
>>   	lockdep_assert_held(&callback_lock);
>>   	if (!parent)
>> @@ -1365,13 +1370,11 @@ static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
>>   	if (parent == &top_cpuset)
>>   		cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
>>   
>> -	isolcpus_updated = (new_prs != parent->partition_root_state);
>> -	if (isolcpus_updated)
>> +	if (new_prs != parent->partition_root_state)
> Can this if statement be replaced with new helper isolated_cpus_should_update？

The isolated_cpus_should_update() helper is for validating the change. 
It is too late to call in partition_xcpus_add/del().

Cheers, Longman

Re: [cgroup/for-6.19 PATCH 2/3] cgroup/cpuset: Fail if isolated and nohz_full don't leave any housekeeping

[Waiman Long]

On 11/2/25 10:53 PM, Chen Ridong wrote:
>
>
>> +
>>   	/*
>>   	 * Change the parent's effective_cpus & effective_xcpus (top cpuset
>>   	 * only).
>> @@ -2994,7 +3055,11 @@ static int update_prstate(struct cpuset *cs, int new_prs)
>>   		 * A change in load balance state only, no change in cpumasks.
>>   		 * Need to update isolated_cpus.
>>   		 */
>> -		isolcpus_updated = true;
>> +		if ((new_prs == PRS_ISOLATED) &&
>> +		    !isolated_cpus_can_update(cs->effective_xcpus, NULL))
>> +			err = PERR_HKEEPING;
>> +		else
>> +			isolcpus_updated = true;
>>   	} else {
>>   		/*
>>   		 * Switching back to member is always allowed even if it
> This is an issue I am trying to fix, the prstate_housekeeping_conflict check is necessary.
>
> https://lore.kernel.org/cgroups/20251025064844.495525-2-chenridong@huaweicloud.com/
>
You are right. We should add prstate_housekeeping_conflict() check here.

BTW, I found some issues when I looked further at the patch. So I am 
going to rewrite part of it and will send out a v2 after some testing.

Cheers,
Longman