| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Sun, 24 Apr 2022 02:53:42 +0000
From: 王擎 <wangqing@...o.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC: Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Sudeep Holla <sudeep.holla@....com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
"linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"vincent.guittot@...aro.org" <vincent.guittot@...aro.org>,
"dietmar.eggemann@....com" <dietmar.eggemann@....com>
Subject: [PATCH V2 RESEND 1/2] arch_topology: support for parsing cache
topology from DT
>> From: Wang Qing <wangqing@...o.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 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 <wangqing@...o.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
>> +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
Powered by blists - more mailing lists