Message-ID: <609a980b-cbe3-442b-a492-91722870b156@csgroup.eu>
Date: Tue, 26 Aug 2025 06:49:29 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: K Prateek Nayak <kprateek.nayak@....com>,
 Madhavan Srinivasan <maddy@...ux.ibm.com>,
 Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
 Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
 Alexander Gordeev <agordeev@...ux.ibm.com>,
 Christian Borntraeger <borntraeger@...ux.ibm.com>,
 Sven Schnelle <svens@...ux.ibm.com>, Thomas Gleixner <tglx@...utronix.de>,
 Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
 Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
 "H. Peter Anvin" <hpa@...or.com>, Peter Zijlstra <peterz@...radead.org>,
 Juri Lelli <juri.lelli@...hat.com>,
 Vincent Guittot <vincent.guittot@...aro.org>, linuxppc-dev@...ts.ozlabs.org,
 linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org
Cc: Dietmar Eggemann <dietmar.eggemann@....com>,
 Steven Rostedt <rostedt@...dmis.org>, Ben Segall <bsegall@...gle.com>,
 Mel Gorman <mgorman@...e.de>, Valentin Schneider <vschneid@...hat.com>,
 thomas.weissschuh@...utronix.de, Li Chen <chenl311@...natelecom.cn>,
 Bibo Mao <maobibo@...ngson.cn>, Mete Durlu <meted@...ux.ibm.com>,
 Tobias Huschle <huschle@...ux.ibm.com>,
 Easwar Hariharan <easwar.hariharan@...ux.microsoft.com>,
 Guo Weikang <guoweikang.kernel@...il.com>,
 "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
 Brian Gerst <brgerst@...il.com>,
 Patryk Wlazlyn <patryk.wlazlyn@...ux.intel.com>,
 Swapnil Sapkal <swapnil.sapkal@....com>,
 "Yury Norov [NVIDIA]" <yury.norov@...il.com>,
 Sudeep Holla <sudeep.holla@....com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 Andrea Righi <arighi@...dia.com>, Yicong Yang <yangyicong@...ilicon.com>,
 Ricardo Neri <ricardo.neri-calderon@...ux.intel.com>,
 Tim Chen <tim.c.chen@...ux.intel.com>,
 Vinicius Costa Gomes <vinicius.gomes@...el.com>
Subject: Re: [PATCH v7 4/8] powerpc/smp: Introduce CONFIG_SCHED_MC to guard MC
 scheduling bits



Le 26/08/2025 à 06:13, K Prateek Nayak a écrit :
> PowerPC enables the MC scheduling domain by default on systems with
> coregroup support without having a SCHED_MC config in Kconfig.
> 
> The scheduler uses CONFIG_SCHED_MC to introduce the MC domain in the
> default topology (core) and to optimize the default CPU selection
> routine (sched-ext).
> 
> Introduce CONFIG_SCHED_MC for powerpc and note that it should be
> preferably enabled given the current default behavior. This also ensures
> PowerPC is tested during future developments that come to depend on
> CONFIG_SCHED_MC.
> 
> Signed-off-by: K Prateek Nayak <kprateek.nayak@....com>
> ---
>   arch/powerpc/Kconfig           | 9 +++++++++
>   arch/powerpc/include/asm/smp.h | 2 ++
>   arch/powerpc/kernel/smp.c      | 4 ++++
>   3 files changed, 15 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 93402a1d9c9f..e954ab3f635f 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -971,6 +971,15 @@ config SCHED_SMT
>   	  when dealing with POWER5 cpus at a cost of slightly increased
>   	  overhead in some places. If unsure say N here.
>   
> +config SCHED_MC
> +	bool "Multi-Core Cache (MC) scheduler support"
> +	depends on PPC64 && SMP
> +	default y
> +	help
> +	  MC scheduler support improves the CPU scheduler's decision making
> +	  when dealing with POWER systems that contain multiple Last Level
> +	  Cache instances on the same socket. If unsure say Y here.
> +

You shouldn't duplicate CONFIG_SCHED_MC in every architecture, instead 
you should define a CONFIG_ARCH_HAS_SCHED_MC in arch/Kconfig that gets 
selected by architectures then have CONFIG_SCHED_MC defined in 
init/Kconfig or kernel/Kconfig or so.

>   config PPC_DENORMALISATION
>   	bool "PowerPC denormalisation exception handling"
>   	depends on PPC_BOOK3S_64
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 86de4d0dd0aa..9a320d96e891 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -148,7 +148,9 @@ static inline const struct cpumask *cpu_smt_mask(int cpu)
>   }
>   #endif /* CONFIG_SCHED_SMT */
>   
> +#ifdef CONFIG_SCHED_MC
>   extern const struct cpumask *cpu_coregroup_mask(int cpu);
> +#endif

Why do you need this ifdef ? Leaving it outside #ifdef allows you to do 
constructs like:

	if (IS_ENABLED(CONFIG_SCHED_MC))
		cpu_coregroup_mask(cpu);

Otherwise you'll need to ensure all calls to cpu_coregroup_mask() are 
also inside #ifdefs, which is not the recommended way nowadays.

>   
>   /* Since OpenPIC has only 4 IPIs, we use slightly different message numbers.
>    *
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index e623f2864dc4..7f79b853b221 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1059,6 +1059,7 @@ static bool has_coregroup_support(void)
>   	return coregroup_enabled;
>   }
>   
> +#ifdef CONFIG_SCHED_MC
>   const struct cpumask *cpu_coregroup_mask(int cpu)
>   {
>   	if (has_coregroup_support())
> @@ -1071,6 +1072,7 @@ static const struct cpumask *cpu_mc_mask(struct sched_domain_topology_level *tl,
>   {
>   	return cpu_corgrp_mask(cpu);
>   }
> +#endif
>   
>   static const struct cpumask *cpu_pkg_mask(struct sched_domain_topology_level *tl, int cpu)
>   {
> @@ -1729,10 +1731,12 @@ static void __init build_sched_topology(void)
>   			SDTL_INIT(shared_cache_mask, powerpc_shared_cache_flags, CACHE);
>   	}
>   
> +#ifdef CONFIG_SCHED_MC

As I said above, define the function prototype at all time in smp.h and 
use IS_ENABLED(CONFIG_SCHED_MC) here instead of a #ifdef

>   	if (has_coregroup_support()) {
>   		powerpc_topology[i++] =
>   			SDTL_INIT(cpu_mc_mask, powerpc_shared_proc_flags, MC);
>   	}
> +#endif
>   
>   	powerpc_topology[i++] = SDTL_INIT(cpu_pkg_mask, powerpc_shared_proc_flags, PKG);
>   

Powered by blists - more mailing lists