[PATCH 09/11] Powerpc/smp: Create coregroup domain

[Srikar Dronamraju]

* Gautham R Shenoy <ego at linux.vnet.ibm.com> [2020-07-17 13:49:26]:

> On Tue, Jul 14, 2020 at 10:06:22AM +0530, Srikar Dronamraju wrote:
> > Add percpu coregroup maps and masks to create coregroup domain.
> > If a coregroup doesn't exist, the coregroup domain will be degenerated
> > in favour of SMT/CACHE domain.
> > 
> > Cc: linuxppc-dev <linuxppc-dev at lists.ozlabs.org>
> > Cc: Michael Ellerman <michaele at au1.ibm.com>
> > Cc: Nick Piggin <npiggin at au1.ibm.com>
> > Cc: Oliver OHalloran <oliveroh at au1.ibm.com>
> > Cc: Nathan Lynch <nathanl at linux.ibm.com>
> > Cc: Michael Neuling <mikey at linux.ibm.com>
> > Cc: Anton Blanchard <anton at au1.ibm.com>
> > Cc: Gautham R Shenoy <ego at linux.vnet.ibm.com>
> > Cc: Vaidyanathan Srinivasan <svaidy at linux.ibm.com>
> > Signed-off-by: Srikar Dronamraju <srikar at linux.vnet.ibm.com>
> > ---
> >  arch/powerpc/include/asm/topology.h | 10 ++++++++
> >  arch/powerpc/kernel/smp.c           | 37 +++++++++++++++++++++++++++++
> >  arch/powerpc/mm/numa.c              |  5 ++++
> >  3 files changed, 52 insertions(+)
> > 
> > @@ -950,6 +972,11 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> >  	cpumask_set_cpu(boot_cpuid, cpu_l2_cache_mask(boot_cpuid));
> >  	cpumask_set_cpu(boot_cpuid, cpu_core_mask(boot_cpuid));
> > 
> > +	if (has_coregroup_support())
> > +		cpumask_set_cpu(boot_cpuid, cpu_coregroup_mask(boot_cpuid));
> > +	else
> > +		powerpc_topology[mc_idx].mask = cpu_bigcore_mask;
> > +
> 
> The else part could be moved to the common function where we are
> modifying the other attributes of the topology array.
> 

My intent is to make all the changes to the topology attributes in
smp_prepare_cpus. It makes more sense to change the attributes immediately
after we define / detect a particular topology change.

The only thing that is left out currently is shared_cache,
We should be able to detect shared_cache also around this time. Just that it
needs more cleanups. Once we do update the topology attributes even for
shared_cache here itself. 

> >  	init_big_cores();
> >  	if (has_big_cores) {
> >  		cpumask_set_cpu(boot_cpuid,
> > @@ -1241,6 +1268,8 @@ static void remove_cpu_from_masks(int cpu)
> >  		set_cpus_unrelated(cpu, i, cpu_sibling_mask);
> >  		if (has_big_cores)
> >  			set_cpus_unrelated(cpu, i, cpu_smallcore_mask);
> > +		if (has_coregroup_support())
> > +			set_cpus_unrelated(cpu, i, cpu_coregroup_mask);
> >  	}
> >  }
> >  #endif
> > @@ -1301,6 +1330,14 @@ static void add_cpu_to_masks(int cpu)
> >  	add_cpu_to_smallcore_masks(cpu);
> >  	update_mask_by_l2(cpu, cpu_l2_cache_mask);
> > 
> > +	if (has_coregroup_support()) {
> > +		cpumask_set_cpu(cpu, cpu_coregroup_mask(cpu));
> > +		for_each_cpu(i, cpu_online_mask) {
> > +			if (cpu_to_coregroup_id(cpu) == cpu_to_coregroup_id(i))
> > +				set_cpus_related(cpu, i, cpu_coregroup_mask);
> > +		}
> > +	}
> > +
> >  	if (pkg_id == -1) {
> >  		struct cpumask *(*mask)(int) = cpu_sibling_mask;
> > 
> > diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> > index a43eab455be4..d9ab9da85eab 100644
> > --- a/arch/powerpc/mm/numa.c
> > +++ b/arch/powerpc/mm/numa.c
> > @@ -1695,6 +1695,11 @@ static const struct proc_ops topology_proc_ops = {
> >  	.proc_release	= single_release,
> >  };
> > 
> > +int cpu_to_coregroup_id(int cpu)
> > +{
> > +	return cpu_to_core_id(cpu);
> > +}
> 
> 
> So, if has_coregroup_support() returns true, then since the core_group
> identification is currently done through the core-id, the
> coregroup_mask is going to be the same as the
> cpu_core_mask/cpu_cpu_mask. Thus, we will be degenerating the DIE
> domain. Right ? Instead we could have kept the core-group to be a
> single bigcore by default, so that those domains can get degenerated
> preserving the legacy SMT, DIE, NUMA hierarchy.
> 
> 

I think you have confused between cpu_core_mask and cpu_to_core_id.
cpu_to_core_id() returns a unique id for every SMT8 thread group.
If coregroup_support is true and the system doesn't support coregroup, then
We would not be degenerating the DIE but degenerating new MC domain only.
This is because the MC domain and the previous domain (SHARED_CACHE/BIGCORE/ 
SMT) would match the MC domain.

-- 
Thanks and Regards
Srikar Dronamraju