From: Wang Qing <[email protected]>
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 by "next-level-cache",
which can describe the cache topology of every level.
MAX_CACHE_LEVEL is strictly corresponding to the cache level from L2.
V2:
make function name more sense
Signed-off-by: Wang Qing <[email protected]>
---
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
+static 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 largest subset of the shared cache in the range of cpu_mask
+ */
+void find_subset_of_share_cache(const struct cpumask *cpu_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, cpu_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_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,
+ struct cpumask *sc_mask);
#endif
#endif /* _LINUX_ARCH_TOPOLOGY_H_ */
--
2.7.4
On Fri, Apr 22, 2022 at 04:51:25AM -0700, Qing Wang wrote:
> From: Wang Qing <[email protected]>
>
> 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 by "next-level-cache",
> which can describe the cache topology of every level.
>
> MAX_CACHE_LEVEL is strictly corresponding to the cache level from L2.
I have no idea what this changelog means at all.
What are you trying to do? What problem are you solving? Why are you
doing any of this?
>
> V2:
> make function name more sense
As per the documentation this goes below the --- line, right?
>
> Signed-off-by: Wang Qing <[email protected]>
> ---
> 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
> +static struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
So for a normal big system of 4k cpus * 7 levels, that's a lot of
memory? are you sure?
How big of a box did you test this on?
> +
> +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;
> + }
No locking anywhere? What could go wrong :(
> +}
> +
> +/*
> + * find the largest subset of the shared cache in the range of cpu_mask
> + */
> +void find_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,
> + struct cpumask *sc_mask)
Again, horrid global function name.
And no kernel documentation for how this works?
> +{
> + int cache_level, cpu_id;
> +
> + for (cache_level = MAX_CACHE_LEVEL - 1; cache_level >= 0; cache_level--) {
> + if (!cache_topology[cpu][cache_level])
> + continue;
No locking???
> +
> + 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, cpu_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_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,
> + struct cpumask *sc_mask);
I still have no idea what this last function is supposed to do.
And very odd indentation, did you run checkpatch on this?
totally confused,
greg k-h
>> From: Wang Qing <[email protected]>
>>
>> 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 by "next-level-cache",
>> which can describe the cache topology of every level.
>>
>> MAX_CACHE_LEVEL is strictly corresponding to the cache level from L2.
>
>I have no idea what this changelog means at all.
>
>What are you trying to do? What problem are you solving? Why are you
>doing any of this?
This patch just supports parsing cache topology from DT in the early
(before sched domain initialization). Then we can use the information
about shared cache level to build the sched domain level.
>
>
>>
>> V2:
>> make function name more sense
>
>As per the documentation this goes below the --- line, right?
>
>
>>
>> Signed-off-by: Wang Qing <[email protected]>
>> ---
>> 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
>> +static struct device_node *cache_topology[NR_CPUS][MAX_CACHE_LEVEL];
>
>So for a normal big system of 4k cpus * 7 levels, that's a lot of
>memory? are you sure?
Yes, if CONFIG_NR_CPUS is configured as 4k, it will take up 224KB memory
in 4k cpus system.
>
>How big of a box did you test this on?
I tested on android phone.
>
>> +
>> +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;
>> + }
>
>No locking anywhere? What could go wrong :(
The calling process of the function has guaranteed no data racing condition.
cache_topology[] will only be modified by parse_dt_topology().
cache_topology[] behaves like cpu_topology[].
>
>> +}
>> +
>> +/*
>> + * find the largest subset of the shared cache in the range of cpu_mask
>> + */
>> +void find_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,
>> + struct cpumask *sc_mask)
>
>Again, horrid global function name.
>
>And no kernel documentation for how this works?
Maybe I should post an FRC patch first.
>
>
>
>> +{
>> + int cache_level, cpu_id;
>> +
>> + for (cache_level = MAX_CACHE_LEVEL - 1; cache_level >= 0; cache_level--) {
>> + if (!cache_topology[cpu][cache_level])
>> + continue;
>
>No locking???
>
>
>> +
>> + 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, cpu_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_subset_of_share_cache(const struct cpumask *cpu_mask, int cpu,
>> + struct cpumask *sc_mask);
>
>I still have no idea what this last function is supposed to do.
This function is to find the largest shared cache subset from the
specified cpumask, so we can make them a new level sched domain,
which will directly benefit a lot of workload.
>
>And very odd indentation, did you run checkpatch on this?
Yes, I modify it manually after running checkpatch, this is my problem.
>
>totally confused,
Thanks for your correction,
Qing
>
>greg k-h