| 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
| ||
|
Message-ID: <8736585djw.fsf@mpe.ellerman.id.au>
Date: Fri, 31 Jul 2020 17:49:55 +1000
From: Michael Ellerman <mpe@...erman.id.au>
To: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
Cc: linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
Srikar Dronamraju <srikar@...ux.vnet.ibm.com>,
LKML <linux-kernel@...r.kernel.org>,
Nicholas Piggin <npiggin@...il.com>,
Anton Blanchard <anton@...abs.org>,
Oliver O'Halloran <oohall@...il.com>,
Nathan Lynch <nathanl@...ux.ibm.com>,
Michael Neuling <mikey@...ling.org>,
Gautham R Shenoy <ego@...ux.vnet.ibm.com>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Valentin Schneider <valentin.schneider@....com>,
Jordan Niethe <jniethe5@...il.com>
Subject: Re: [PATCH v4 07/10] Powerpc/numa: Detect support for coregroup
Srikar Dronamraju <srikar@...ux.vnet.ibm.com> writes:
> Add support for grouping cores based on the device-tree classification.
> - The last domain in the associativity domains always refers to the
> core.
> - If primary reference domain happens to be the penultimate domain in
> the associativity domains device-tree property, then there are no
> coregroups. However if its not a penultimate domain, then there are
> coregroups. There can be more than one coregroup. For now we would be
> interested in the last or the smallest coregroups.
This still doesn't tell me what a coregroup actually represents.
I get that it's a grouping of cores, and that the device tree specifies
it for us, but grouping based on what?
I think the answer is we aren't being told by firmware, it's just a
grouping based on some opaque performance characteristic and we just
have to take that as given.
But please explain that clearly in the change log and the code comments.
cheers
> Cc: linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>
> Cc: LKML <linux-kernel@...r.kernel.org>
> Cc: Michael Ellerman <mpe@...erman.id.au>
> Cc: Nicholas Piggin <npiggin@...il.com>
> Cc: Anton Blanchard <anton@...abs.org>
> Cc: Oliver O'Halloran <oohall@...il.com>
> Cc: Nathan Lynch <nathanl@...ux.ibm.com>
> Cc: Michael Neuling <mikey@...ling.org>
> Cc: Gautham R Shenoy <ego@...ux.vnet.ibm.com>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Valentin Schneider <valentin.schneider@....com>
> Cc: Jordan Niethe <jniethe5@...il.com>
> Reviewed-by: Gautham R. Shenoy <ego@...ux.vnet.ibm.com>
> Signed-off-by: Srikar Dronamraju <srikar@...ux.vnet.ibm.com>
> ---
> Changelog v1 -> v2:
> Explained Coregroup in commit msg (Michael Ellerman)
>
> arch/powerpc/include/asm/smp.h | 1 +
> arch/powerpc/kernel/smp.c | 1 +
> arch/powerpc/mm/numa.c | 34 +++++++++++++++++++++-------------
> 3 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/smp.h b/arch/powerpc/include/asm/smp.h
> index 49a25e2400f2..5bdc17a7049f 100644
> --- a/arch/powerpc/include/asm/smp.h
> +++ b/arch/powerpc/include/asm/smp.h
> @@ -28,6 +28,7 @@
> extern int boot_cpuid;
> extern int spinning_secondaries;
> extern u32 *cpu_to_phys_id;
> +extern bool coregroup_enabled;
>
> extern void cpu_die(void);
> extern int cpu_to_chip_id(int cpu);
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 3c5ccf6d2b1c..698000c7f76f 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -74,6 +74,7 @@ static DEFINE_PER_CPU(int, cpu_state) = { 0 };
>
> struct task_struct *secondary_current;
> bool has_big_cores;
> +bool coregroup_enabled;
>
> DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map);
> DEFINE_PER_CPU(cpumask_var_t, cpu_smallcore_map);
> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 2298899a0f0a..51cb672f113b 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -886,7 +886,9 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
> static void __init find_possible_nodes(void)
> {
> struct device_node *rtas;
> - u32 numnodes, i;
> + const __be32 *domains;
> + int prop_length, max_nodes;
> + u32 i;
>
> if (!numa_enabled)
> return;
> @@ -895,25 +897,31 @@ static void __init find_possible_nodes(void)
> if (!rtas)
> return;
>
> - if (of_property_read_u32_index(rtas, "ibm,current-associativity-domains",
> - min_common_depth, &numnodes)) {
> - /*
> - * ibm,current-associativity-domains is a fairly recent
> - * property. If it doesn't exist, then fallback on
> - * ibm,max-associativity-domains. Current denotes what the
> - * platform can support compared to max which denotes what the
> - * Hypervisor can support.
> - */
> - if (of_property_read_u32_index(rtas, "ibm,max-associativity-domains",
> - min_common_depth, &numnodes))
> + /*
> + * ibm,current-associativity-domains is a fairly recent property. If
> + * it doesn't exist, then fallback on ibm,max-associativity-domains.
> + * Current denotes what the platform can support compared to max
> + * which denotes what the Hypervisor can support.
> + */
> + domains = of_get_property(rtas, "ibm,current-associativity-domains",
> + &prop_length);
> + if (!domains) {
> + domains = of_get_property(rtas, "ibm,max-associativity-domains",
> + &prop_length);
> + if (!domains)
> goto out;
> }
>
> - for (i = 0; i < numnodes; i++) {
> + max_nodes = of_read_number(&domains[min_common_depth], 1);
> + for (i = 0; i < max_nodes; i++) {
> if (!node_possible(i))
> node_set(i, node_possible_map);
> }
>
> + prop_length /= sizeof(int);
> + if (prop_length > min_common_depth + 2)
> + coregroup_enabled = 1;
> +
> out:
> of_node_put(rtas);
> }
> --
> 2.17.1
Powered by blists - more mailing lists