diff mbox series

sched: topology: add input parameter for sched_domain_flags_f()

Message ID 1648461219-4333-1-git-send-email-wangqing@vivo.com (mailing list archive)
State Handled Elsewhere, archived
Headers show
Series sched: topology: add input parameter for sched_domain_flags_f() | expand

Checks

Context Check Description
snowpatch_ozlabs/github-powerpc_ppctests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_selftests success Successfully ran 8 jobs.
snowpatch_ozlabs/github-powerpc_kernel_qemu success Successfully ran 24 jobs.
snowpatch_ozlabs/github-powerpc_sparse success Successfully ran 4 jobs.
snowpatch_ozlabs/github-powerpc_clang success Successfully ran 7 jobs.

Commit Message

王擎 March 28, 2022, 9:53 a.m. UTC 
From: Wang Qing <wangqing@vivo.com>

sched_domain_flags_f() are statically set now, but actually, we can get a
lot of necessary information based on the cpu_map. e.g. we can know whether
its cache is shared.

Allows custom extension without affecting current.

Signed-off-by: Wang Qing <wangqing@vivo.com>
---
 arch/powerpc/kernel/smp.c      |  4 ++--
 arch/x86/kernel/smpboot.c      |  8 ++++----
 include/linux/sched/topology.h | 10 +++++-----
 kernel/sched/topology.c        |  2 +-
 4 files changed, 12 insertions(+), 12 deletions(-)

Comments

Srikar Dronamraju March 28, 2022, 11:23 a.m. UTC | #1 
* Qing Wang <wangqing@vivo.com> [2022-03-28 02:53:37]:

> From: Wang Qing <wangqing@vivo.com>
> 
> sched_domain_flags_f() are statically set now, but actually, we can get a
> lot of necessary information based on the cpu_map. e.g. we can know whether
> its cache is shared.
> 
> Allows custom extension without affecting current.
> 

can you elaborate on how passing a cpumask pointer will help us to know if
cache is shared or not? With this patch, we are just passing the cpumask,
but dont seem to be using it in any of the cases. 

or did I miss anything?

> Signed-off-by: Wang Qing <wangqing@vivo.com>
> ---
>  arch/powerpc/kernel/smp.c      |  4 ++--
>  arch/x86/kernel/smpboot.c      |  8 ++++----
>  include/linux/sched/topology.h | 10 +++++-----
>  kernel/sched/topology.c        |  2 +-
>  4 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index de0f6f0..e503d23
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -1000,7 +1000,7 @@ static bool shared_caches;
>  
>  #ifdef CONFIG_SCHED_SMT
>  /* cpumask of CPUs with asymmetric SMT dependency */
> -static int powerpc_smt_flags(void)
> +static int powerpc_smt_flags(const struct cpumask *cpu_map)
>  {
>  	int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
>  
> @@ -1018,7 +1018,7 @@ static int powerpc_smt_flags(void)
>   * since the migrated task remains cache hot. We want to take advantage of this
>   * at the scheduler level so an extra topology level is required.
>   */
> -static int powerpc_shared_cache_flags(void)
> +static int powerpc_shared_cache_flags(const struct cpumask *cpu_map)
>  {
>  	return SD_SHARE_PKG_RESOURCES;
>  }
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 2ef1477..c005a8e
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -535,25 +535,25 @@ static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
>  
>  
>  #if defined(CONFIG_SCHED_SMT) || defined(CONFIG_SCHED_CLUSTER) || defined(CONFIG_SCHED_MC)
> -static inline int x86_sched_itmt_flags(void)
> +static inline int x86_sched_itmt_flags(const struct cpumask *cpu_map)
>  {
>  	return sysctl_sched_itmt_enabled ? SD_ASYM_PACKING : 0;
>  }
>  
>  #ifdef CONFIG_SCHED_MC
> -static int x86_core_flags(void)
> +static int x86_core_flags(const struct cpumask *cpu_map)
>  {
>  	return cpu_core_flags() | x86_sched_itmt_flags();
>  }
>  #endif
>  #ifdef CONFIG_SCHED_SMT
> -static int x86_smt_flags(void)
> +static int x86_smt_flags(const struct cpumask *cpu_map)
>  {
>  	return cpu_smt_flags() | x86_sched_itmt_flags();
>  }
>  #endif
>  #ifdef CONFIG_SCHED_CLUSTER
> -static int x86_cluster_flags(void)
> +static int x86_cluster_flags(const struct cpumask *cpu_map)
>  {
>  	return cpu_cluster_flags() | x86_sched_itmt_flags();
>  }
> diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
> index 56cffe4..6aa985a
> --- a/include/linux/sched/topology.h
> +++ b/include/linux/sched/topology.h
> @@ -36,28 +36,28 @@ extern const struct sd_flag_debug sd_flag_debug[];
>  #endif
>  
>  #ifdef CONFIG_SCHED_SMT
> -static inline int cpu_smt_flags(void)
> +static inline int cpu_smt_flags(const struct cpumask *cpu_map)
>  {
>  	return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
>  }
>  #endif
>  
>  #ifdef CONFIG_SCHED_CLUSTER
> -static inline int cpu_cluster_flags(void)
> +static inline int cpu_cluster_flags(const struct cpumask *cpu_map)
>  {
>  	return SD_SHARE_PKG_RESOURCES;
>  }
>  #endif
>  
>  #ifdef CONFIG_SCHED_MC
> -static inline int cpu_core_flags(void)
> +static inline int cpu_core_flags(const struct cpumask *cpu_map)
>  {
>  	return SD_SHARE_PKG_RESOURCES;
>  }
>  #endif
>  
>  #ifdef CONFIG_NUMA
> -static inline int cpu_numa_flags(void)
> +static inline int cpu_numa_flags(const struct cpumask *cpu_map)
>  {
>  	return SD_NUMA;
>  }
> @@ -180,7 +180,7 @@ void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms);
>  bool cpus_share_cache(int this_cpu, int that_cpu);
>  
>  typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
> -typedef int (*sched_domain_flags_f)(void);
> +typedef int (*sched_domain_flags_f)(const struct cpumask *cpu_map);
>  
>  #define SDTL_OVERLAP	0x01
>  
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 05b6c2a..34dfec4
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1556,7 +1556,7 @@ sd_init(struct sched_domain_topology_level *tl,
>  	sd_weight = cpumask_weight(tl->mask(cpu));
>  
>  	if (tl->sd_flags)
> -		sd_flags = (*tl->sd_flags)();
> +		sd_flags = (*tl->sd_flags)(tl->mask(cpu));
>  	if (WARN_ONCE(sd_flags & ~TOPOLOGY_SD_FLAGS,
>  			"wrong sd_flags in topology description\n"))
>  		sd_flags &= TOPOLOGY_SD_FLAGS;
> -- 
> 2.7.4
> 
>
Peter Zijlstra March 28, 2022, 11:58 a.m. UTC | #2 
On Mon, Mar 28, 2022 at 02:53:37AM -0700, Qing Wang wrote:
> From: Wang Qing <wangqing@vivo.com>
> 
> sched_domain_flags_f() are statically set now, but actually, we can get a
> lot of necessary information based on the cpu_map. e.g. we can know whether
> its cache is shared.
> 
> Allows custom extension without affecting current.

This all still makes absolutely no sense. The architecture builds these
masks, the architecture is in charge of which flags function is called
on which mask.

Passing the mask back in means it lost the plot somewhere and doens't
know wth it's doing anymore.

NAK
王擎 March 28, 2022, 12:19 p.m. UTC | #3 
>> From: Wang Qing <wangqing@vivo.com>
>> 
>> sched_domain_flags_f() are statically set now, but actually, we can get a
>> lot of necessary information based on the cpu_map. e.g. we can know whether
>> its cache is shared.
>> 
>> Allows custom extension without affecting current.
>
>This all still makes absolutely no sense. The architecture builds these
>masks, the architecture is in charge of which flags function is called
>on which mask.
>
>Passing the mask back in means it lost the plot somewhere and doens't
>know wth it's doing anymore.

It is not passing the mask back, sched_domain_flags_f() doesn't use cpumask
at all, it always return fixed values. sd_topology_level select different 
sd_flag() to config, it's too primitive.

If an architecture can describe its cache topology clearly in every level,
the sd only need sched_domain_flags_f(cpumask) to get its ShPR flag.

Thanks,
Wang

>
>NAK
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index de0f6f0..e503d23
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -1000,7 +1000,7 @@  static bool shared_caches;
 
 #ifdef CONFIG_SCHED_SMT
 /* cpumask of CPUs with asymmetric SMT dependency */
-static int powerpc_smt_flags(void)
+static int powerpc_smt_flags(const struct cpumask *cpu_map)
 {
 	int flags = SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
 
@@ -1018,7 +1018,7 @@  static int powerpc_smt_flags(void)
  * since the migrated task remains cache hot. We want to take advantage of this
  * at the scheduler level so an extra topology level is required.
  */
-static int powerpc_shared_cache_flags(void)
+static int powerpc_shared_cache_flags(const struct cpumask *cpu_map)
 {
 	return SD_SHARE_PKG_RESOURCES;
 }
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 2ef1477..c005a8e
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -535,25 +535,25 @@  static bool match_llc(struct cpuinfo_x86 *c, struct cpuinfo_x86 *o)
 
 
 #if defined(CONFIG_SCHED_SMT) || defined(CONFIG_SCHED_CLUSTER) || defined(CONFIG_SCHED_MC)
-static inline int x86_sched_itmt_flags(void)
+static inline int x86_sched_itmt_flags(const struct cpumask *cpu_map)
 {
 	return sysctl_sched_itmt_enabled ? SD_ASYM_PACKING : 0;
 }
 
 #ifdef CONFIG_SCHED_MC
-static int x86_core_flags(void)
+static int x86_core_flags(const struct cpumask *cpu_map)
 {
 	return cpu_core_flags() | x86_sched_itmt_flags();
 }
 #endif
 #ifdef CONFIG_SCHED_SMT
-static int x86_smt_flags(void)
+static int x86_smt_flags(const struct cpumask *cpu_map)
 {
 	return cpu_smt_flags() | x86_sched_itmt_flags();
 }
 #endif
 #ifdef CONFIG_SCHED_CLUSTER
-static int x86_cluster_flags(void)
+static int x86_cluster_flags(const struct cpumask *cpu_map)
 {
 	return cpu_cluster_flags() | x86_sched_itmt_flags();
 }
diff --git a/include/linux/sched/topology.h b/include/linux/sched/topology.h
index 56cffe4..6aa985a
--- a/include/linux/sched/topology.h
+++ b/include/linux/sched/topology.h
@@ -36,28 +36,28 @@  extern const struct sd_flag_debug sd_flag_debug[];
 #endif
 
 #ifdef CONFIG_SCHED_SMT
-static inline int cpu_smt_flags(void)
+static inline int cpu_smt_flags(const struct cpumask *cpu_map)
 {
 	return SD_SHARE_CPUCAPACITY | SD_SHARE_PKG_RESOURCES;
 }
 #endif
 
 #ifdef CONFIG_SCHED_CLUSTER
-static inline int cpu_cluster_flags(void)
+static inline int cpu_cluster_flags(const struct cpumask *cpu_map)
 {
 	return SD_SHARE_PKG_RESOURCES;
 }
 #endif
 
 #ifdef CONFIG_SCHED_MC
-static inline int cpu_core_flags(void)
+static inline int cpu_core_flags(const struct cpumask *cpu_map)
 {
 	return SD_SHARE_PKG_RESOURCES;
 }
 #endif
 
 #ifdef CONFIG_NUMA
-static inline int cpu_numa_flags(void)
+static inline int cpu_numa_flags(const struct cpumask *cpu_map)
 {
 	return SD_NUMA;
 }
@@ -180,7 +180,7 @@  void free_sched_domains(cpumask_var_t doms[], unsigned int ndoms);
 bool cpus_share_cache(int this_cpu, int that_cpu);
 
 typedef const struct cpumask *(*sched_domain_mask_f)(int cpu);
-typedef int (*sched_domain_flags_f)(void);
+typedef int (*sched_domain_flags_f)(const struct cpumask *cpu_map);
 
 #define SDTL_OVERLAP	0x01
 
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 05b6c2a..34dfec4
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1556,7 +1556,7 @@  sd_init(struct sched_domain_topology_level *tl,
 	sd_weight = cpumask_weight(tl->mask(cpu));
 
 	if (tl->sd_flags)
-		sd_flags = (*tl->sd_flags)();
+		sd_flags = (*tl->sd_flags)(tl->mask(cpu));
 	if (WARN_ONCE(sd_flags & ~TOPOLOGY_SD_FLAGS,
 			"wrong sd_flags in topology description\n"))
 		sd_flags &= TOPOLOGY_SD_FLAGS;