Re: [PATCH -next 6/6] cpuset: remove v1-specific code from generate_sched_domains

[Waiman Long]

On 12/17/25 3:49 AM, Chen Ridong wrote:
> From: Chen Ridong <chenridong@xxxxxxxxxx>
>
> Following the introduction of cpuset1_generate_sched_domains() for v1
> in the previous patch, v1-specific logic can now be removed from the
> generic generate_sched_domains(). This patch cleans up the v1-only
> code and ensures uf_node is only visible when CONFIG_CPUSETS_V1=y.
>
> Signed-off-by: Chen Ridong <chenridong@xxxxxxxxxx>
> ---
>   kernel/cgroup/cpuset-internal.h |  10 +--
>   kernel/cgroup/cpuset-v1.c       |   2 +-
>   kernel/cgroup/cpuset.c          | 144 +++++---------------------------
>   3 files changed, 27 insertions(+), 129 deletions(-)
>
> diff --git a/kernel/cgroup/cpuset-internal.h b/kernel/cgroup/cpuset-internal.h
> index bd767f8cb0ed..ef7b7c5afd4c 100644
> --- a/kernel/cgroup/cpuset-internal.h
> +++ b/kernel/cgroup/cpuset-internal.h
> @@ -175,14 +175,14 @@ struct cpuset {
>   	/* Handle for cpuset.cpus.partition */
>   	struct cgroup_file partition_file;
>   
> -	/* Used to merge intersecting subsets for generate_sched_domains */
> -	struct uf_node node;
> -
>   #ifdef CONFIG_CPUSETS_V1
>   	struct fmeter fmeter;		/* memory_pressure filter */
>   
>   	/* for custom sched domain */
>   	int relax_domain_level;
> +
> +	/* Used to merge intersecting subsets for generate_sched_domains */
> +	struct uf_node node;
>   #endif
>   };
>   
> @@ -315,8 +315,6 @@ void cpuset1_hotplug_update_tasks(struct cpuset *cs,
>   int cpuset1_validate_change(struct cpuset *cur, struct cpuset *trial);
>   void cpuset1_init(struct cpuset *cs);
>   void cpuset1_online_css(struct cgroup_subsys_state *css);
> -void update_domain_attr_tree(struct sched_domain_attr *dattr,
> -				    struct cpuset *root_cs);
>   int cpuset1_generate_sched_domains(cpumask_var_t **domains,
>   			struct sched_domain_attr **attributes);
>   
> @@ -331,8 +329,6 @@ static inline int cpuset1_validate_change(struct cpuset *cur,
>   				struct cpuset *trial) { return 0; }
>   static inline void cpuset1_init(struct cpuset *cs) {}
>   static inline void cpuset1_online_css(struct cgroup_subsys_state *css) {}
> -static inline void update_domain_attr_tree(struct sched_domain_attr *dattr,
> -				    struct cpuset *root_cs) {}
>   static inline int cpuset1_generate_sched_domains(cpumask_var_t **domains,
>   			struct sched_domain_attr **attributes) { return 0; };
>   
> diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
> index 5c0bded46a7c..0226350e704f 100644
> --- a/kernel/cgroup/cpuset-v1.c
> +++ b/kernel/cgroup/cpuset-v1.c
> @@ -560,7 +560,7 @@ update_domain_attr(struct sched_domain_attr *dattr, struct cpuset *c)
>   		dattr->relax_domain_level = c->relax_domain_level;
>   }
>   
> -void update_domain_attr_tree(struct sched_domain_attr *dattr,
> +static void update_domain_attr_tree(struct sched_domain_attr *dattr,
>   				    struct cpuset *root_cs)
>   {
>   	struct cpuset *cp;
> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 6bb0b201c34b..3e3468d928f3 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c
> @@ -789,18 +789,13 @@ static int generate_sched_domains(cpumask_var_t **domains,
>   {
>   	struct cpuset *cp;	/* top-down scan of cpusets */
>   	struct cpuset **csa;	/* array of all cpuset ptrs */
> -	int csn;		/* how many cpuset ptrs in csa so far */
>   	int i, j;		/* indices for partition finding loops */
>   	cpumask_var_t *doms;	/* resulting partition; i.e. sched domains */
>   	struct sched_domain_attr *dattr;  /* attributes for custom domains */
>   	int ndoms = 0;		/* number of sched domains in result */
> -	int nslot;		/* next empty doms[] struct cpumask slot */
>   	struct cgroup_subsys_state *pos_css;
> -	bool root_load_balance = is_sched_load_balance(&top_cpuset);
> -	bool cgrpv2 = cpuset_v2();
> -	int nslot_update;
>   
> -	if (!cgrpv2)
> +	if (!cpuset_v2())
>   		return cpuset1_generate_sched_domains(domains, attributes);
>   
>   	doms = NULL;
> @@ -808,70 +803,25 @@ static int generate_sched_domains(cpumask_var_t **domains,
>   	csa = NULL;
>   
>   	/* Special case for the 99% of systems with one, full, sched domain */
> -	if (root_load_balance && cpumask_empty(subpartitions_cpus)) {
> -single_root_domain:
> +	if (cpumask_empty(subpartitions_cpus)) {
>   		ndoms = 1;
> -		doms = alloc_sched_domains(ndoms);
> -		if (!doms)
> -			goto done;
> -
> -		dattr = kmalloc(sizeof(struct sched_domain_attr), GFP_KERNEL);
> -		if (dattr) {
> -			*dattr = SD_ATTR_INIT;
> -			update_domain_attr_tree(dattr, &top_cpuset);
> -		}
> -		cpumask_and(doms[0], top_cpuset.effective_cpus,
> -			    housekeeping_cpumask(HK_TYPE_DOMAIN));
> -
> -		goto done;
> +		goto generate_doms;

That is not correct. The code under the generate_doms label will need to 
access csa[0] which is not allocated yet and may cause panic. You either 
need to keep the current code or move it after the csa allocation and 
assign top_cpuset to csa[0].

>   	}
>   
>   	csa = kmalloc_array(nr_cpusets(), sizeof(cp), GFP_KERNEL);
>   	if (!csa)
>   		goto done;
> -	csn = 0;
>   
> +	/* Find how many partitions and cache them to csa[] */
>   	rcu_read_lock();
> -	if (root_load_balance)
> -		csa[csn++] = &top_cpuset;
>   	cpuset_for_each_descendant_pre(cp, pos_css, &top_cpuset) {

The cpuset_for_each_descendant_pre() macro will visit the root 
(top_cpuset) first and so it should be OK to remove the above 2 lines of 
code.

Cheers,
Longman