* Gautham R Shenoy <e...@linux.vnet.ibm.com> [2020-07-17 13:56:53]:
> On Tue, Jul 14, 2020 at 10:06:23AM +0530, Srikar Dronamraju wrote:
> > Lookup the coregroup id from the associativity array.
> >
> > If unable to detect the coregroup id, fallback on the core id.
> > This way, ensure sched_domain degenerates and an extra sched domain is
> > not created.
> >
> > Ideally this function should have been implemented in
> > arch/powerpc/kernel/smp.c. However if its implemented in mm/numa.c, we
> > don't need to find the primary domain again.
> >
> > If the device-tree mentions more than one coregroup, then kernel
> > implements only the last or the smallest coregroup, which currently
> > corresponds to the penultimate domain in the device-tree.
> >
> > Cc: linuxppc-dev <linuxppc-dev@lists.ozlabs.org>
> > Cc: Michael Ellerman <micha...@au1.ibm.com>
> > Cc: Nick Piggin <npig...@au1.ibm.com>
> > Cc: Oliver OHalloran <olive...@au1.ibm.com>
> > Cc: Nathan Lynch <nath...@linux.ibm.com>
> > Cc: Michael Neuling <mi...@linux.ibm.com>
> > Cc: Anton Blanchard <an...@au1.ibm.com>
> > Cc: Gautham R Shenoy <e...@linux.vnet.ibm.com>
> > Cc: Vaidyanathan Srinivasan <sva...@linux.ibm.com>
> > Signed-off-by: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/mm/numa.c | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index d9ab9da85eab..4e85564ef62a 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -1697,6 +1697,23 @@ static const struct proc_ops topology_proc_ops = {
> >
> > int cpu_to_coregroup_id(int cpu)
> > {
> > + __be32 associativity[VPHN_ASSOC_BUFSIZE] = {0};
> > + int index;
> > +
>
> It would be good to have an assert here to ensure that we are calling
> this function only when coregroups are enabled.
>
> Else, we may end up returning the penultimate index which maps to the
> chip-id.
>
We have a check below exactly for the same reason. Please look below.
>
>
> > + if (cpu < 0 || cpu > nr_cpu_ids)
> > + return -1;
> > +
> > + if (!firmware_has_feature(FW_FEATURE_VPHN))
> > + goto out;
> > +
> > + if (vphn_get_associativity(cpu, associativity))
> > + goto out;
> > +
> > + index = of_read_number(associativity, 1);
> > + if ((index > min_common_depth + 1) && coregroup_enabled)
> > + return of_read_number(&associativity[index - 1], 1);
See ^above.
index would be the all the domains in the associativity array,
min_common_depth would be where the primary domain or the chip-id is
defined. So we are reading the penultimate domain if and only if the
min_common_depth isn't the primary domain aka chip-id.
What other check /assertions can we add?
> > +
> > +out:
> > return cpu_to_core_id(cpu);
> > }
> >
> > --
> > 2.17.1
> >
--
Thanks and Regards
Srikar Dronamraju
