* Christophe Leroy (CS GROUP) <[email protected]> [2026-05-29 09:58:25]:
> Le 24/05/2026 à 03:00, Srikar Dronamraju a écrit :
> > Coregroup support was only available on PowerPC on PowerVM LPARs.
> > However if firmware were to expose coregroup-id to the kernel, then
> > coregroup can even be supported on PowerNV too.
> >
Thanks Christophe for taking a look.
> > +extern int cpu_to_coregroup_id(int cpu);
> No new 'extern' for function prototypes (allthough it is only a move),
> that's pointless. checkpatch.pl --strict will likely complain about it.
>
Ok, will do..
> > #else
> > static inline int early_cpu_to_node(int cpu) { return 0; }
> > @@ -107,14 +108,6 @@ static inline void map_cpu_to_node(int cpu, int node)
> > {}
> > static inline void unmap_cpu_from_node(unsigned long cpu) {}
> > #endif /* CONFIG_HOTPLUG_CPU */
> > #endif /* CONFIG_SMP */
> > -
> > -#endif /* CONFIG_NUMA */
> > -
> > -#if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
> > -void find_and_update_cpu_nid(int cpu);
> > -extern int cpu_to_coregroup_id(int cpu);
> > -#else
> > -static inline void find_and_update_cpu_nid(int cpu) {}
> > static inline int cpu_to_coregroup_id(int cpu)
> > {
> > #ifdef CONFIG_SMP
> > @@ -124,6 +117,12 @@ static inline int cpu_to_coregroup_id(int cpu)
> > #endif
> > }
> > +#endif /* CONFIG_NUMA */
> > +
> > +#if defined(CONFIG_NUMA) && defined(CONFIG_PPC_SPLPAR)
> > +void find_and_update_cpu_nid(int cpu);
> > +#else
> > +static inline void find_and_update_cpu_nid(int cpu) {}
> > #endif /* CONFIG_NUMA && CONFIG_PPC_SPLPAR */
> > #include <asm-generic/topology.h>
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index f4cf3ae036de..9b45cc9e1f27 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -432,7 +432,7 @@ static void __init
> > initialize_form2_numa_distance_lookup_table(void)
> > static int __init find_primary_domain_index(void)
> > {
> > - int index;
> > + int index = -1;
> > struct device_node *root;
> > /*
> > @@ -502,12 +502,9 @@ static int __init find_primary_domain_index(void)
> > distance_ref_points_depth = MAX_DISTANCE_REF_POINTS;
> > }
> > - of_node_put(root);
> > - return index;
> > -
> > err:
> > of_node_put(root);
> > - return -1;
> > + return index;
> > }
> > static void __init get_n_mem_cells(int *n_addr_cells, int *n_size_cells)
> > @@ -892,12 +889,32 @@ static int __init numa_setup_drmem_lmb(struct
> > drmem_lmb *lmb,
> > return 0;
> > }
> > +/*
> > + * If hierarchy extends beyond primary_domain_index + 1, then next
> > + * level corresponds to coregroup.
> > + */
> > +static int detect_and_enable_coregroup(const __be32 *associativity, int
> > index)
> > +{
> > + if (!associativity)
> > + return -1;
>
> Even if 'index' is not 0 ?
In the hypothetical case, where we don't have the associativity for a CPU,
then which coregroup should we place this CPU. Placing it in arbitrary group
can confuse the scheduler and cause more issues.
Hence I guess, its better to return -1.
>
> > +
> > + if (!index) {
> > + index = of_read_number(associativity, 1);
> > +
> > + if (index > primary_domain_index + 1)
> > + coregroup_enabled = 1;
> > + else
> > + index = -1;
> > + }
> > + return index;
> > +}
>
> I'd prefer something more flat:
>
> if (index)
> return index;
>
> index = of_read_number(associativity, 1);
>
> if (index <= primary_domain_index + 1)
> return -1;
>
> coregroup_enabled = 1;
> return index;
>
Again, if a particular associativity doesn't have coregroup info, we are
probably better of disabling coregroup for all. Since we can't enable
coregroup for some CPUs and disable for other CPUs. And we can't add default
coregroup too since that would confuse the scheduler. For example 2 CPUs
from 2 nodes may end up being part of the same coregroup because of default
numbering. This will lead to topology_span_sane() cribbing.
May be we should also reset coregroup_enabled, whenever index is set to -1.
> > +
--
Thanks and Regards
Srikar Dronamraju
