Re: [PATCH v4 1/4] smpboot: introduce SDTL() helper to tidy sched topology setup

[K Prateek Nayak <kprateek.nayak@amd.com>]

Hello Li,

Apart from few comments inline below, feel free to include:

Tested-by: K Prateek Nayak <kprateek.nayak@amd.com>

for the entire series.

On 7/6/2025 8:36 AM, Li Chen wrote:
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index 198bb5cc1774b..0b53e372c445c 100644
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -197,9 +197,9 @@ struct sched_domain_topology_level {
>  extern void __init set_sched_topology(struct sched_domain_topology_level *tl);
>  extern void sched_update_asym_prefer_cpu(int cpu, int old_prio, int new_prio);
>  
> -
> -# define SD_INIT_NAME(type)		.name = #type
> -
> +#define SDTL(maskfn, flagsfn, dname) \
> +	((struct sched_domain_topology_level) \
> +	    { .mask = maskfn, .sd_flags = flagsfn, .name = #dname, .numa_level = 0 })

I prefer the following alignment:

#define SDTL(maskfn, flagsfn, dname) ((struct sched_domain_topology_level) \
	{ .mask = maskfn, .sd_flags = flagsfn, .name = #dname })

instead of having 3 lines. "numa_level" is 0 by default so I don't think
we need to explicitly specify it again.

Also perhaps the macro can be named "SDTL_INIT()" to keep consistent
with the naming convention.

>  #else /* CONFIG_SMP */

A bunch of the CONFIG_SMP related ifdeffry is being removed for the
next cycle. You can perhaps rebase the series on top of the tip tree
(git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git)

>  
>  struct sched_domain_attr;

[..snip..]

> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index b958fe48e0205..e6ec65ae4b75d 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -2025,7 +2021,7 @@ void sched_init_numa(int offline_node)
>  			.sd_flags = cpu_numa_flags,
>  			.flags = SDTL_OVERLAP,
>  			.numa_level = j,
> -			SD_INIT_NAME(NUMA)
> +			.name = "NUMA",

This can use SDTL() macro too. Just explicitly set "tl[i].numa_level" to
"j" after.

>  		};
>  	}
>  

-- 
Thanks and Regards,
Prateek