[PATCH 0/2] cgroup/cpuset: Miscellaneous cleanup

[PATCH 0/2] cgroup/cpuset: Miscellaneous cleanup

[Waiman Long]

This series consists of 2 independent cleanup patches. The first one
eliminates the tracking of the number of child partitions as that value
isn't that useful anymore simplifying the code. The second one change
callback_lock into raw_spinlock_t to reduce cpuset operation overhead in
a PREEMPT_RT setting.

Waiman Long (2):
  cgroup/cpuset: Don't track # of local child partitions
  cgroup/cpuset: Change callback_lock to raw_spinlock_t

 kernel/cgroup/cpuset-internal.h |   3 -
 kernel/cgroup/cpuset.c          | 135 ++++++++++++++------------------
 2 files changed, 60 insertions(+), 78 deletions(-)

-- 
2.51.0

[PATCH 1/2] cgroup/cpuset: Don't track # of local child partitions

[Waiman Long]

The cpuset structure has a nr_subparts field which tracks the number
of child local partitions underneath a particular cpuset. Right now,
nr_subparts is only used in partition_is_populated() to avoid iteration
of child cpusets if the condition is right. So by always performing the
child iteration, we can avoid tracking the number of child partitions
and simplify the code a bit.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset-internal.h |  3 ---
 kernel/cgroup/cpuset.c          | 41 +++++++++++----------------------
 2 files changed, 13 insertions(+), 31 deletions(-)

diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
index 337608f408ce..5cac42c5fd97 100644
--- a/kernel/cgroup/cpuset-internal.h
+++ b/kernel/cgroup/cpuset-internal.h
@@ -155,9 +155,6 @@ struct cpuset {
 	/* for custom sched domain */
 	int relax_domain_level;
 
-	/* number of valid local child partitions */
-	int nr_subparts;
-
 	/* partition root state */
 	int partition_root_state;
 
diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 52468d2c178a..7aef59ea9627 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -358,8 +358,13 @@ static inline bool is_in_v2_mode(void)
  * @excluded_child: a child cpuset to be excluded in task checking
  * Return: true if there are tasks, false otherwise
  *
- * It is assumed that @cs is a valid partition root. @excluded_child should
- * be non-NULL when this cpuset is going to become a partition itself.
+ * @cs should be a valid partition root or going to become a partition root.
+ * @excluded_child should be non-NULL when this cpuset is going to become a
+ * partition itself.
+ *
+ * Note that a remote partition is not allowed underneath a valid local
+ * or remote partition. So if a non-partition root child is populated,
+ * the whole partition is considered populated.
  */
 static inline bool partition_is_populated(struct cpuset *cs,
 					  struct cpuset *excluded_child)
@@ -369,8 +374,6 @@ static inline bool partition_is_populated(struct cpuset *cs,
 
 	if (cs->css.cgroup->nr_populated_csets)
 		return true;
-	if (!excluded_child && !cs->nr_subparts)
-		return cgroup_is_populated(cs->css.cgroup);
 
 	rcu_read_lock();
 	cpuset_for_each_child(child, css, cs) {
@@ -1302,7 +1305,6 @@ static void reset_partition_data(struct cpuset *cs)
 
 	lockdep_assert_held(&callback_lock);
 
-	cs->nr_subparts = 0;
 	if (cpumask_empty(cs->exclusive_cpus)) {
 		cpumask_clear(cs->effective_xcpus);
 		if (is_cpu_exclusive(cs))
@@ -1746,7 +1748,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 subparts_delta = 0;
 	int isolcpus_updated = 0;
 	struct cpumask *xcpus = user_xcpus(cs);
 	bool nocpu;
@@ -1771,10 +1772,9 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		if (is_partition_valid(parent))
 			adding = cpumask_and(tmp->addmask,
 					     xcpus, parent->effective_xcpus);
-		if (old_prs > 0) {
+		if (old_prs > 0)
 			new_prs = -old_prs;
-			subparts_delta--;
-		}
+
 		goto write_error;
 	}
 
@@ -1829,7 +1829,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
 
 		deleting = true;
-		subparts_delta++;
 	} else if (cmd == partcmd_disable) {
 		/*
 		 * May need to add cpus back to parent's effective_cpus
@@ -1840,7 +1839,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		if (is_partition_valid(cs)) {
 			cpumask_copy(tmp->addmask, cs->effective_xcpus);
 			adding = true;
-			subparts_delta--;
 		}
 		new_prs = PRS_MEMBER;
 	} else if (newmask) {
@@ -1963,17 +1961,13 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		switch (cs->partition_root_state) {
 		case PRS_ROOT:
 		case PRS_ISOLATED:
-			if (part_error) {
+			if (part_error)
 				new_prs = -old_prs;
-				subparts_delta--;
-			}
 			break;
 		case PRS_INVALID_ROOT:
 		case PRS_INVALID_ISOLATED:
-			if (!part_error) {
+			if (!part_error)
 				new_prs = -old_prs;
-				subparts_delta++;
-			}
 			break;
 		}
 	}
@@ -2002,11 +1996,9 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	 * newly deleted ones will be added back to effective_cpus.
 	 */
 	spin_lock_irq(&callback_lock);
-	if (old_prs != new_prs) {
+	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.
@@ -2018,10 +2010,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		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);
 
@@ -2105,8 +2093,6 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 			 */
 			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);
 			continue;
@@ -4021,7 +4007,6 @@ static void cpuset_handle_hotplug(void)
 		 */
 		if (!cpumask_empty(subpartitions_cpus)) {
 			if (cpumask_subset(&new_cpus, subpartitions_cpus)) {
-				top_cpuset.nr_subparts = 0;
 				cpumask_clear(subpartitions_cpus);
 			} else {
 				cpumask_andnot(&new_cpus, &new_cpus,
-- 
2.51.0

[PATCH 2/2] cgroup/cpuset: Change callback_lock to raw_spinlock_t

[Waiman Long]

The callback_lock is acquired either to read a stable set of cpu
or node masks or to modify those masks when cpuset_mutex is also
acquired. Sometime, it may need to go up the cgroup hierarchy while
holding the lock to find the right set of masks to use. Assuming that
the depth of the cgroup hierarch is finite and typically small, the
lock hold time should be limited.

As a result, it can be converted to raw_spinlock_t to reduce overhead
in a PREEMPT_RT setting without introducing excessive latency.

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

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 7aef59ea9627..026eb1a17ad2 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c
@@ -285,16 +285,16 @@ void cpuset_full_unlock(void)
 	cpus_read_unlock();
 }
 
-static DEFINE_SPINLOCK(callback_lock);
+static DEFINE_RAW_SPINLOCK(callback_lock);
 
 void cpuset_callback_lock_irq(void)
 {
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 }
 
 void cpuset_callback_unlock_irq(void)
 {
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 }
 
 static struct workqueue_struct *cpuset_migrate_mm_wq;
@@ -1552,11 +1552,11 @@ 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);
+	raw_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);
+	raw_spin_unlock_irq(&callback_lock);
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 	cpuset_force_rebuild();
 	cs->prs_err = 0;
@@ -1585,7 +1585,7 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 	WARN_ON_ONCE(!is_remote_partition(cs));
 	WARN_ON_ONCE(!cpumask_subset(cs->effective_xcpus, subpartitions_cpus));
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 	list_del_init(&cs->remote_sibling);
 	isolcpus_updated = partition_xcpus_del(cs->partition_root_state,
 					       NULL, cs->effective_xcpus);
@@ -1597,7 +1597,7 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 	/* effective_xcpus may need to be changed */
 	compute_excpus(cs, cs->effective_xcpus);
 	reset_partition_data(cs);
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 	cpuset_force_rebuild();
 
@@ -1654,7 +1654,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 			goto invalidate;
 	}
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 	if (adding)
 		isolcpus_updated += partition_xcpus_add(prs, NULL, tmp->addmask);
 	if (deleting)
@@ -1666,7 +1666,7 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *xcpus,
 	cpumask_copy(cs->effective_xcpus, excpus);
 	if (xcpus)
 		cpumask_copy(cs->exclusive_cpus, xcpus);
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 	if (adding || deleting)
 		cpuset_force_rebuild();
@@ -1995,7 +1995,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	 * 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);
+	raw_spin_lock_irq(&callback_lock);
 	if (old_prs != new_prs)
 		cs->partition_root_state = new_prs;
 
@@ -2010,7 +2010,7 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 		isolcpus_updated += partition_xcpus_add(new_prs, parent,
 							tmp->delmask);
 
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 
 	if ((old_prs != new_prs) && (cmd == partcmd_update))
@@ -2091,9 +2091,9 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
 			/*
 			 * Invalidate child partition
 			 */
-			spin_lock_irq(&callback_lock);
+			raw_spin_lock_irq(&callback_lock);
 			make_partition_invalid(child);
-			spin_unlock_irq(&callback_lock);
+			raw_spin_unlock_irq(&callback_lock);
 			notify_partition_change(child, old_prs);
 			continue;
 		}
@@ -2239,7 +2239,7 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
 			new_prs = cp->partition_root_state;
 		}
 
-		spin_lock_irq(&callback_lock);
+		raw_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))
@@ -2254,7 +2254,7 @@ static void update_cpumasks_hier(struct cpuset *cs, struct tmpmasks *tmp,
 				    cp->cpus_allowed, parent->effective_xcpus);
 		else if (new_prs < 0)
 			reset_partition_data(cp);
-		spin_unlock_irq(&callback_lock);
+		raw_spin_unlock_irq(&callback_lock);
 
 		notify_partition_change(cp, old_prs);
 
@@ -2507,12 +2507,12 @@ static int update_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 
 	partition_cpus_change(cs, trialcs, &tmp);
 
-	spin_lock_irq(&callback_lock);
+	raw_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);
+	raw_spin_unlock_irq(&callback_lock);
 
 	/* effective_cpus/effective_xcpus will be updated here */
 	update_cpumasks_hier(cs, &tmp, force);
@@ -2572,12 +2572,12 @@ static int update_exclusive_cpumask(struct cpuset *cs, struct cpuset *trialcs,
 	trialcs->prs_err = PERR_NONE;
 	partition_cpus_change(cs, trialcs, &tmp);
 
-	spin_lock_irq(&callback_lock);
+	raw_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);
+	raw_spin_unlock_irq(&callback_lock);
 
 	/*
 	 * Call update_cpumasks_hier() to update effective_cpus/effective_xcpus
@@ -2792,9 +2792,9 @@ static void update_nodemasks_hier(struct cpuset *cs, nodemask_t *new_mems)
 			continue;
 		rcu_read_unlock();
 
-		spin_lock_irq(&callback_lock);
+		raw_spin_lock_irq(&callback_lock);
 		cp->effective_mems = *new_mems;
-		spin_unlock_irq(&callback_lock);
+		raw_spin_unlock_irq(&callback_lock);
 
 		WARN_ON(!is_in_v2_mode() &&
 			!nodes_equal(cp->mems_allowed, cp->effective_mems));
@@ -2849,9 +2849,9 @@ static int update_nodemask(struct cpuset *cs, struct cpuset *trialcs,
 
 	check_insane_mems_config(&trialcs->mems_allowed);
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 	cs->mems_allowed = trialcs->mems_allowed;
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 
 	/* use trialcs->mems_allowed as a temp variable */
 	update_nodemasks_hier(cs, &trialcs->mems_allowed);
@@ -2906,9 +2906,9 @@ int cpuset_update_flag(cpuset_flagbits_t bit, struct cpuset *cs,
 	spread_flag_changed = ((is_spread_slab(cs) != is_spread_slab(trialcs))
 			|| (is_spread_page(cs) != is_spread_page(trialcs)));
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 	cs->flags = trialcs->flags;
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 
 	if (!cpumask_empty(trialcs->cpus_allowed) && balance_flag_changed) {
 		if (cpuset_v2())
@@ -3021,14 +3021,14 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		update_partition_exclusive_flag(cs, new_prs);
 	}
 
-	spin_lock_irq(&callback_lock);
+	raw_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);
 	else if (isolcpus_updated)
 		isolated_cpus_update(old_prs, new_prs, cs->effective_xcpus);
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 	update_unbound_workqueue_cpumask(isolcpus_updated);
 
 	/* Force update if switching back to member & update effective_xcpus */
@@ -3342,7 +3342,7 @@ int cpuset_common_seq_show(struct seq_file *sf, void *v)
 	cpuset_filetype_t type = seq_cft(sf)->private;
 	int ret = 0;
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 
 	switch (type) {
 	case FILE_CPULIST:
@@ -3373,7 +3373,7 @@ int cpuset_common_seq_show(struct seq_file *sf, void *v)
 		ret = -EINVAL;
 	}
 
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 	return ret;
 }
 
@@ -3567,12 +3567,12 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 
 	cpuset_inc();
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 	if (is_in_v2_mode()) {
 		cpumask_copy(cs->effective_cpus, parent->effective_cpus);
 		cs->effective_mems = parent->effective_mems;
 	}
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 
 	if (!test_bit(CGRP_CPUSET_CLONE_CHILDREN, &css->cgroup->flags))
 		goto out_unlock;
@@ -3599,12 +3599,12 @@ static int cpuset_css_online(struct cgroup_subsys_state *css)
 	}
 	rcu_read_unlock();
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 	cs->mems_allowed = parent->mems_allowed;
 	cs->effective_mems = parent->mems_allowed;
 	cpumask_copy(cs->cpus_allowed, parent->cpus_allowed);
 	cpumask_copy(cs->effective_cpus, parent->cpus_allowed);
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 out_unlock:
 	cpuset_full_unlock();
 	return 0;
@@ -3655,7 +3655,7 @@ static void cpuset_css_free(struct cgroup_subsys_state *css)
 static void cpuset_bind(struct cgroup_subsys_state *root_css)
 {
 	mutex_lock(&cpuset_mutex);
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 
 	if (is_in_v2_mode()) {
 		cpumask_copy(top_cpuset.cpus_allowed, cpu_possible_mask);
@@ -3667,7 +3667,7 @@ static void cpuset_bind(struct cgroup_subsys_state *root_css)
 		top_cpuset.mems_allowed = top_cpuset.effective_mems;
 	}
 
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 	mutex_unlock(&cpuset_mutex);
 }
 
@@ -3831,10 +3831,10 @@ hotplug_update_tasks(struct cpuset *cs,
 	if (nodes_empty(*new_mems))
 		*new_mems = parent_cs(cs)->effective_mems;
 
-	spin_lock_irq(&callback_lock);
+	raw_spin_lock_irq(&callback_lock);
 	cpumask_copy(cs->effective_cpus, new_cpus);
 	cs->effective_mems = *new_mems;
-	spin_unlock_irq(&callback_lock);
+	raw_spin_unlock_irq(&callback_lock);
 
 	if (cpus_updated)
 		cpuset_update_tasks_cpumask(cs, new_cpus);
@@ -3996,7 +3996,7 @@ static void cpuset_handle_hotplug(void)
 	/* For v1, synchronize cpus_allowed to cpu_active_mask */
 	if (cpus_updated) {
 		cpuset_force_rebuild();
-		spin_lock_irq(&callback_lock);
+		raw_spin_lock_irq(&callback_lock);
 		if (!on_dfl)
 			cpumask_copy(top_cpuset.cpus_allowed, &new_cpus);
 		/*
@@ -4014,17 +4014,17 @@ static void cpuset_handle_hotplug(void)
 			}
 		}
 		cpumask_copy(top_cpuset.effective_cpus, &new_cpus);
-		spin_unlock_irq(&callback_lock);
+		raw_spin_unlock_irq(&callback_lock);
 		/* we don't mess with cpumasks of tasks in top_cpuset */
 	}
 
 	/* synchronize mems_allowed to N_MEMORY */
 	if (mems_updated) {
-		spin_lock_irq(&callback_lock);
+		raw_spin_lock_irq(&callback_lock);
 		if (!on_dfl)
 			top_cpuset.mems_allowed = new_mems;
 		top_cpuset.effective_mems = new_mems;
-		spin_unlock_irq(&callback_lock);
+		raw_spin_unlock_irq(&callback_lock);
 		cpuset_update_tasks_nodemask(&top_cpuset);
 	}
 
@@ -4117,7 +4117,7 @@ void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
 	unsigned long flags;
 	struct cpuset *cs;
 
-	spin_lock_irqsave(&callback_lock, flags);
+	raw_spin_lock_irqsave(&callback_lock, flags);
 
 	cs = task_cs(tsk);
 	if (cs != &top_cpuset)
@@ -4139,7 +4139,7 @@ void cpuset_cpus_allowed(struct task_struct *tsk, struct cpumask *pmask)
 			cpumask_copy(pmask, possible_mask);
 	}
 
-	spin_unlock_irqrestore(&callback_lock, flags);
+	raw_spin_unlock_irqrestore(&callback_lock, flags);
 }
 
 /**
@@ -4210,9 +4210,9 @@ nodemask_t cpuset_mems_allowed(struct task_struct *tsk)
 	nodemask_t mask;
 	unsigned long flags;
 
-	spin_lock_irqsave(&callback_lock, flags);
+	raw_spin_lock_irqsave(&callback_lock, flags);
 	guarantee_online_mems(task_cs(tsk), &mask);
-	spin_unlock_irqrestore(&callback_lock, flags);
+	raw_spin_unlock_irqrestore(&callback_lock, flags);
 
 	return mask;
 }
@@ -4304,12 +4304,12 @@ bool cpuset_current_node_allowed(int node, gfp_t gfp_mask)
 		return true;
 
 	/* Not hardwall and node outside mems_allowed: scan up cpusets */
-	spin_lock_irqsave(&callback_lock, flags);
+	raw_spin_lock_irqsave(&callback_lock, flags);
 
 	cs = nearest_hardwall_ancestor(task_cs(current));
 	allowed = node_isset(node, cs->mems_allowed);
 
-	spin_unlock_irqrestore(&callback_lock, flags);
+	raw_spin_unlock_irqrestore(&callback_lock, flags);
 	return allowed;
 }
 
-- 
2.51.0

Re: [PATCH 1/2] cgroup/cpuset: Don't track # of local child partitions

[Chen Ridong]

On 2025/10/20 10:32, Waiman Long wrote:
> The cpuset structure has a nr_subparts field which tracks the number
> of child local partitions underneath a particular cpuset. Right now,
> nr_subparts is only used in partition_is_populated() to avoid iteration
> of child cpusets if the condition is right. So by always performing the
> child iteration, we can avoid tracking the number of child partitions
> and simplify the code a bit.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>
> ---
>  kernel/cgroup/cpuset-internal.h |  3 ---
>  kernel/cgroup/cpuset.c          | 41 +++++++++++----------------------
>  2 files changed, 13 insertions(+), 31 deletions(-)
> 
> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
> index 337608f408ce..5cac42c5fd97 100644
> --- a/kernel/cgroup/cpuset-internal.h
> +++ b/kernel/cgroup/cpuset-internal.h
> @@ -155,9 +155,6 @@ struct cpuset {
>  	/* for custom sched domain */
>  	int relax_domain_level;
>  
> -	/* number of valid local child partitions */
> -	int nr_subparts;
> -
>  	/* partition root state */
>  	int partition_root_state;
>  
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 52468d2c178a..7aef59ea9627 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -358,8 +358,13 @@ static inline bool is_in_v2_mode(void)
>   * @excluded_child: a child cpuset to be excluded in task checking
>   * Return: true if there are tasks, false otherwise
>   *
> - * It is assumed that @cs is a valid partition root. @excluded_child should
> - * be non-NULL when this cpuset is going to become a partition itself.
> + * @cs should be a valid partition root or going to become a partition root.
> + * @excluded_child should be non-NULL when this cpuset is going to become a
> + * partition itself.
> + *
> + * Note that a remote partition is not allowed underneath a valid local
> + * or remote partition. So if a non-partition root child is populated,
> + * the whole partition is considered populated.
>   */
>  static inline bool partition_is_populated(struct cpuset *cs,
>  					  struct cpuset *excluded_child)
> @@ -369,8 +374,6 @@ static inline bool partition_is_populated(struct cpuset *cs,
>  
>  	if (cs->css.cgroup->nr_populated_csets)
>  		return true;
> -	if (!excluded_child && !cs->nr_subparts)
> -		return cgroup_is_populated(cs->css.cgroup);
>  
>  	rcu_read_lock();
>  	cpuset_for_each_child(child, css, cs) {
> @@ -1302,7 +1305,6 @@ static void reset_partition_data(struct cpuset *cs)
>  
>  	lockdep_assert_held(&callback_lock);
>  
> -	cs->nr_subparts = 0;
>  	if (cpumask_empty(cs->exclusive_cpus)) {
>  		cpumask_clear(cs->effective_xcpus);
>  		if (is_cpu_exclusive(cs))
> @@ -1746,7 +1748,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 subparts_delta = 0;
>  	int isolcpus_updated = 0;
>  	struct cpumask *xcpus = user_xcpus(cs);
>  	bool nocpu;
> @@ -1771,10 +1772,9 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  		if (is_partition_valid(parent))
>  			adding = cpumask_and(tmp->addmask,
>  					     xcpus, parent->effective_xcpus);
> -		if (old_prs > 0) {
> +		if (old_prs > 0)
>  			new_prs = -old_prs;
> -			subparts_delta--;
> -		}
> +
>  		goto write_error;
>  	}
>  
> @@ -1829,7 +1829,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  		WARN_ON_ONCE(!cpumask_subset(tmp->new_cpus, parent->effective_cpus));
>  
>  		deleting = true;
> -		subparts_delta++;
>  	} else if (cmd == partcmd_disable) {
>  		/*
>  		 * May need to add cpus back to parent's effective_cpus
> @@ -1840,7 +1839,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  		if (is_partition_valid(cs)) {
>  			cpumask_copy(tmp->addmask, cs->effective_xcpus);
>  			adding = true;
> -			subparts_delta--;
>  		}
>  		new_prs = PRS_MEMBER;
>  	} else if (newmask) {
> @@ -1963,17 +1961,13 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  		switch (cs->partition_root_state) {
>  		case PRS_ROOT:
>  		case PRS_ISOLATED:
> -			if (part_error) {
> +			if (part_error)
>  				new_prs = -old_prs;
> -				subparts_delta--;
> -			}
>  			break;
>  		case PRS_INVALID_ROOT:
>  		case PRS_INVALID_ISOLATED:
> -			if (!part_error) {
> +			if (!part_error)
>  				new_prs = -old_prs;
> -				subparts_delta++;
> -			}
>  			break;
>  		}
>  	}
> @@ -2002,11 +1996,9 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  	 * newly deleted ones will be added back to effective_cpus.
>  	 */
>  	spin_lock_irq(&callback_lock);
> -	if (old_prs != new_prs) {
> +	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.
> @@ -2018,10 +2010,6 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
>  		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);
>  
> @@ -2105,8 +2093,6 @@ static void compute_partition_effective_cpumask(struct cpuset *cs,
>  			 */
>  			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);
>  			continue;
> @@ -4021,7 +4007,6 @@ static void cpuset_handle_hotplug(void)
>  		 */
>  		if (!cpumask_empty(subpartitions_cpus)) {
>  			if (cpumask_subset(&new_cpus, subpartitions_cpus)) {
> -				top_cpuset.nr_subparts = 0;
>  				cpumask_clear(subpartitions_cpus);
>  			} else {
>  				cpumask_andnot(&new_cpus, &new_cpus,

LGTM

-- 
Best regards,
Ridong

Re: [PATCH 2/2] cgroup/cpuset: Change callback_lock to raw_spinlock_t

[Tejun Heo]

On Sun, Oct 19, 2025 at 10:32:07PM -0400, Waiman Long wrote:
> The callback_lock is acquired either to read a stable set of cpu
> or node masks or to modify those masks when cpuset_mutex is also
> acquired. Sometime, it may need to go up the cgroup hierarchy while
> holding the lock to find the right set of masks to use. Assuming that
> the depth of the cgroup hierarch is finite and typically small, the
> lock hold time should be limited.
> 
> As a result, it can be converted to raw_spinlock_t to reduce overhead
> in a PREEMPT_RT setting without introducing excessive latency.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>

Is this something that RT people would like? Why does the overhead of the
lock matter? I think this patch requires more justifications.

Thanks.

-- 
tejun

Re: [PATCH 1/2] cgroup/cpuset: Don't track # of local child partitions

[Tejun Heo]

On Sun, Oct 19, 2025 at 10:32:06PM -0400, Waiman Long wrote:
> The cpuset structure has a nr_subparts field which tracks the number
> of child local partitions underneath a particular cpuset. Right now,
> nr_subparts is only used in partition_is_populated() to avoid iteration
> of child cpusets if the condition is right. So by always performing the
> child iteration, we can avoid tracking the number of child partitions
> and simplify the code a bit.
> 
> Signed-off-by: Waiman Long <longman@redhat.com>

Applied to cgroup/for-6.19.

Thanks.

-- 
tejun

Re: [PATCH 1/2] cgroup/cpuset: Don't track # of local child partitions

[Tejun Heo]

On Mon, Oct 20, 2025 at 02:57:38PM +0800, Chen Ridong wrote:
> LGTM

Can you please use either Acked-by or Reviewed-by in the future?

Thanks.

-- 
tejun

Re: [PATCH 2/2] cgroup/cpuset: Change callback_lock to raw_spinlock_t

[Waiman Long]

On 10/20/25 1:00 PM, Tejun Heo wrote:
> On Sun, Oct 19, 2025 at 10:32:07PM -0400, Waiman Long wrote:
>> The callback_lock is acquired either to read a stable set of cpu
>> or node masks or to modify those masks when cpuset_mutex is also
>> acquired. Sometime, it may need to go up the cgroup hierarchy while
>> holding the lock to find the right set of masks to use. Assuming that
>> the depth of the cgroup hierarch is finite and typically small, the
>> lock hold time should be limited.
>>
>> As a result, it can be converted to raw_spinlock_t to reduce overhead
>> in a PREEMPT_RT setting without introducing excessive latency.
>>
>> Signed-off-by: Waiman Long <longman@redhat.com>
> Is this something that RT people would like? Why does the overhead of the
> lock matter? I think this patch requires more justifications.

Yes, I will push a new patch and cc the RT guys to see if they have any 
objection. I had seen that callback_lock was changed to raw_spinlock_t 
in one or more of the releases of the linux-rt-devel tree before the 
mainline merge.

Thanks,
Longman

Re: [PATCH 1/2] cgroup/cpuset: Don't track # of local child partitions

[Chen Ridong]

On 2025/10/21 1:02, Tejun Heo wrote:
> On Mon, Oct 20, 2025 at 02:57:38PM +0800, Chen Ridong wrote:
>> LGTM
> 
> Can you please use either Acked-by or Reviewed-by in the future?
> 
> Thanks.
> 

Sure, will do.

-- 
Best regards,
Ridong