diff mbox series

[1/2] arch_topology: support for describing cache topology from DT

Message ID 1650552960-60165-2-git-send-email-wangqing@vivo.com (mailing list archive)
State New, archived
Headers show
Series Add complex scheduler level for arm64 | expand

Commit Message

王擎 April 21, 2022, 2:55 p.m. UTC
From: Wang Qing <wangqing@vivo.com>

When ACPI is not enabled, we can get cache topolopy from DT like:
*		cpu0: cpu@000 {
*			next-level-cache = <&L2_1>;
*			L2_1: l2-cache {
* 				compatible = "cache";
*				next-level-cache = <&L3_1>;
* 			};
*			L3_1: l3-cache {
* 				compatible = "cache";
* 			};
*		};
*
*		cpu1: cpu@001 {
*			next-level-cache = <&L2_1>;
*		};
*		...
*		};
cache_topology hold the pointer describing "next-level-cache", 
it can describe the cache topology of every level.

Signed-off-by: Wang Qing <wangqing@vivo.com>
---
 drivers/base/arch_topology.c  | 47 ++++++++++++++++++++++++++++++++++-
 include/linux/arch_topology.h |  3 +++
 2 files changed, 49 insertions(+), 1 deletion(-)

Comments

Greg KH April 21, 2022, 3:47 p.m. UTC | #1
On Thu, Apr 21, 2022 at 07:55:57AM -0700, Qing Wang wrote:
> From: Wang Qing <wangqing@vivo.com>
> 
> When ACPI is not enabled, we can get cache topolopy from DT like:
> *		cpu0: cpu@000 {
> *			next-level-cache = <&L2_1>;
> *			L2_1: l2-cache {
> * 				compatible = "cache";
> *				next-level-cache = <&L3_1>;
> * 			};
> *			L3_1: l3-cache {
> * 				compatible = "cache";
> * 			};
> *		};
> *
> *		cpu1: cpu@001 {
> *			next-level-cache = <&L2_1>;
> *		};
> *		...
> *		};
> cache_topology hold the pointer describing "next-level-cache", 
> it can describe the cache topology of every level.
> 
> Signed-off-by: Wang Qing <wangqing@vivo.com>
> ---
>  drivers/base/arch_topology.c  | 47 ++++++++++++++++++++++++++++++++++-
>  include/linux/arch_topology.h |  3 +++
>  2 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 1d6636ebaac5..46e84ce2ec0c 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -480,8 +480,10 @@ static int __init get_cpu_for_node(struct device_node *node)
>  		return -1;
>  
>  	cpu = of_cpu_node_to_id(cpu_node);
> -	if (cpu >= 0)
> +	if (cpu >= 0) {
>  		topology_parse_cpu_capacity(cpu_node, cpu);
> +		topology_parse_cpu_caches(cpu_node, cpu);
> +	}
>  	else
>  		pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
>  			cpu_node, cpumask_pr_args(cpu_possible_mask));
> @@ -647,6 +649,49 @@ static int __init parse_dt_topology(void)
>  }
>  #endif
>  
> +/*
> + * cpu cache topology table
> + */
> +#define MAX_CACHE_LEVEL 7
> +staic struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
> +
> +void topology_parse_cpu_caches(struct device_node *cpu_node, int cpu)
> +{
> +	struct device_node *node_cache = cpu_node;
> +	int level = 0;
> +
> +	while (level < MAX_CACHE_LEVEL) {
> +		node_cache = of_parse_phandle(node_cache, "next-level-cache", 0);
> +		if (!node_cache)
> +			break;
> +
> +		cache_topology[cpu][level++] = node_cache;
> +	}
> +}
> +
> +/*
> + * find the maximum level shared cache under giving mask
> + */
> +void find_max_sub_sc(const struct cpumask *giving_mask, int cpu,
> +					  struct cpumask *sc_mask)

This is not a good global function name.  No one will know what this
means when they read it.  Please make it make more sense.

thanks,

greg k-h
kernel test robot April 22, 2022, 2:30 a.m. UTC | #2
Hi Qing,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on driver-core/driver-core-testing linus/master arm-perf/for-next/perf v5.18-rc3 next-20220421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Qing-Wang/Add-complex-scheduler-level-for-arm64/20220421-225748
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: arm-keystone_defconfig (https://download.01.org/0day-ci/archive/20220422/202204221053.QFSQGEXr-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/854ee80a8c32ea98203c96ba25cae2e87eeb43b1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Qing-Wang/Add-complex-scheduler-level-for-arm64/20220421-225748
        git checkout 854ee80a8c32ea98203c96ba25cae2e87eeb43b1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/base/arch_topology.c:617:6: error: expected ';' before 'struct'
     617 | staic struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
         |      ^~~~~~~
         |      ;


vim +617 drivers/base/arch_topology.c

   612	
   613	/*
   614	 * cpu cache topology table
   615	 */
   616	#define MAX_CACHE_LEVEL 7
 > 617	staic struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
   618
Sudeep Holla April 22, 2022, 9:22 a.m. UTC | #3
On Thu, Apr 21, 2022 at 07:55:57AM -0700, Qing Wang wrote:
> From: Wang Qing <wangqing@vivo.com>
> 
> When ACPI is not enabled, we can get cache topolopy from DT like:
> *		cpu0: cpu@000 {
> *			next-level-cache = <&L2_1>;
> *			L2_1: l2-cache {
> * 				compatible = "cache";
> *				next-level-cache = <&L3_1>;
> * 			};
> *			L3_1: l3-cache {
> * 				compatible = "cache";
> * 			};
> *		};
> *
> *		cpu1: cpu@001 {
> *			next-level-cache = <&L2_1>;
> *		};
> *		...
> *		};
> cache_topology hold the pointer describing "next-level-cache",
> it can describe the cache topology of every level.

As I mentioned before, I would like to avoid any duplication and see
what can be reused from drivers/base/cacheinfo.c

We can discuss and see how to proceed on that once we settle/agree on
2/2. I don't want to waste your or my time if we don't end up using this.
So let us look at this once we agree to push the sched related changes
as we have used generic ones so far and you want to introduce arm64 specific
levels. That requires some discussions and thoughts before we can finalise.

Also I have mentioned you to keep Dietmar and Vincent in cc for all sched
related changes which you failed to do again. I expect you fix that next
time if you want them to help you in discussions and make any progress on
this. Otherwise it may get ignored as you don't have all the right
people in cc.

--
Regards,
Sudeep
kernel test robot April 22, 2022, 9:27 a.m. UTC | #4
Hi Qing,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on arm64/for-next/core]
[also build test ERROR on driver-core/driver-core-testing linus/master arm-perf/for-next/perf v5.18-rc3 next-20220421]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Qing-Wang/Add-complex-scheduler-level-for-arm64/20220421-225748
base:   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
config: riscv-randconfig-c006-20220421 (https://download.01.org/0day-ci/archive/20220422/202204221720.cO70SkrD-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 5bd87350a5ae429baf8f373cb226a57b62f87280)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/intel-lab-lkp/linux/commit/854ee80a8c32ea98203c96ba25cae2e87eeb43b1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Qing-Wang/Add-complex-scheduler-level-for-arm64/20220421-225748
        git checkout 854ee80a8c32ea98203c96ba25cae2e87eeb43b1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/ drivers/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/base/arch_topology.c:617:1: error: unknown type name 'staic'; did you mean 'static'?
   staic struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
   ^~~~~
   static
   1 error generated.


vim +617 drivers/base/arch_topology.c

   612	
   613	/*
   614	 * cpu cache topology table
   615	 */
   616	#define MAX_CACHE_LEVEL 7
 > 617	staic struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
   618
王擎 April 22, 2022, 9:48 a.m. UTC | #5

>On Thu, Apr 21, 2022 at 07:55:57AM -0700, Qing Wang wrote:
>> From: Wang Qing <wangqing@vivo.com>
>> 
>> When ACPI is not enabled, we can get cache topolopy from DT like:
>> *             cpu0: cpu@000 {
>> *                     next-level-cache = <&L2_1>;
>> *                     L2_1: l2-cache {
>> *                              compatible = "cache";
>> *                             next-level-cache = <&L3_1>;
>> *                      };
>> *                     L3_1: l3-cache {
>> *                              compatible = "cache";
>> *                      };
>> *             };
>> *
>> *             cpu1: cpu@001 {
>> *                     next-level-cache = <&L2_1>;
>> *             };
>> *             ...
>> *             };
>> cache_topology hold the pointer describing "next-level-cache",
>> it can describe the cache topology of every level.
>
>As I mentioned before, I would like to avoid any duplication and see
>what can be reused from drivers/base/cacheinfo.c

I tried, but cacheinfo initialization is too late, 
we can discuss this part after 2/2 is approved.

>
>We can discuss and see how to proceed on that once we settle/agree on
>2/2. I don't want to waste your or my time if we don't end up using this.
>So let us look at this once we agree to push the sched related changes
>as we have used generic ones so far and you want to introduce arm64 specific
>levels. That requires some discussions and thoughts before we can finalise.
>
>Also I have mentioned you to keep Dietmar and Vincent in cc for all sched
>related changes which you failed to do again. I expect you fix that next
>time if you want them to help you in discussions and make any progress on
>this. Otherwise it may get ignored as you don't have all the right
>people in cc.

I found this, and have forwarded it to Dietmar and Vincent, 
I definitely will fix that next time.

Thanks,
Qing
>
>--
>Regards,
>Sudeep
diff mbox series

Patch

diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 1d6636ebaac5..46e84ce2ec0c 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -480,8 +480,10 @@  static int __init get_cpu_for_node(struct device_node *node)
 		return -1;
 
 	cpu = of_cpu_node_to_id(cpu_node);
-	if (cpu >= 0)
+	if (cpu >= 0) {
 		topology_parse_cpu_capacity(cpu_node, cpu);
+		topology_parse_cpu_caches(cpu_node, cpu);
+	}
 	else
 		pr_info("CPU node for %pOF exist but the possible cpu range is :%*pbl\n",
 			cpu_node, cpumask_pr_args(cpu_possible_mask));
@@ -647,6 +649,49 @@  static int __init parse_dt_topology(void)
 }
 #endif
 
+/*
+ * cpu cache topology table
+ */
+#define MAX_CACHE_LEVEL 7
+staic struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
+
+void topology_parse_cpu_caches(struct device_node *cpu_node, int cpu)
+{
+	struct device_node *node_cache = cpu_node;
+	int level = 0;
+
+	while (level < MAX_CACHE_LEVEL) {
+		node_cache = of_parse_phandle(node_cache, "next-level-cache", 0);
+		if (!node_cache)
+			break;
+
+		cache_topology[cpu][level++] = node_cache;
+	}
+}
+
+/*
+ * find the maximum level shared cache under giving mask
+ */
+void find_max_sub_sc(const struct cpumask *giving_mask, int cpu,
+					  struct cpumask *sc_mask)
+{
+	int cache_level, cpu_id;
+
+	for (cache_level = MAX_CACHE_LEVEL - 1; cache_level >= 0; cache_level--) {
+		if (!cache_topology[cpu][cache_level])
+			continue;
+
+		cpumask_clear(sc_mask);
+		for (cpu_id = 0; cpu_id < NR_CPUS; cpu_id++) {
+			if (cache_topology[cpu][cache_level] == cache_topology[cpu_id][cache_level])
+				cpumask_set_cpu(cpu_id, sc_mask);
+		}
+
+		if (cpumask_subset(sc_mask, giving_mask))
+			break;
+	}
+}
+
 /*
  * cpu topology table
  */
diff --git a/include/linux/arch_topology.h b/include/linux/arch_topology.h
index 58cbe18d825c..c6ed727e453c 100644
--- a/include/linux/arch_topology.h
+++ b/include/linux/arch_topology.h
@@ -93,6 +93,9 @@  void update_siblings_masks(unsigned int cpu);
 void remove_cpu_topology(unsigned int cpuid);
 void reset_cpu_topology(void);
 int parse_acpi_topology(void);
+void topology_parse_cpu_caches(struct device_node *cpu_node, int cpu);
+void find_max_sub_sc(const struct cpumask *giving_mask, int cpu,
+					  struct cpumask *sc_mask);
 #endif
 
 #endif /* _LINUX_ARCH_TOPOLOGY_H_ */