* Gautham R Shenoy <e...@linux.vnet.ibm.com> [2020-07-27 10:09:41]: > > > > static void fixup_topology(void) > > { > > + if (!has_coregroup_support()) > > + powerpc_topology[mc_idx].mask = cpu_bigcore_mask; > > + > > if (shared_caches) { > > pr_info("Using shared cache scheduler topology\n"); > > powerpc_topology[bigcore_idx].mask = shared_cache_mask; > > > Suppose we consider a topology which does not have coregroup_support, > but has shared_caches. In that case, we would want our coregroup > domain to degenerate. > > From the above code, after the fixup, our topology will look as > follows: > > static struct sched_domain_topology_level powerpc_topology[] = { > { cpu_smt_mask, powerpc_smt_flags, SD_INIT_NAME(SMT) }, > { shared_cache_mask, powerpc_shared_cache_flags, SD_INIT_NAME(CACHE) }, > { cpu_bigcore_mask, SD_INIT_NAME(MC) }, > { cpu_cpu_mask, SD_INIT_NAME(DIE) }, > { NULL, }, > > So, in this case, the core-group domain (identified by MC) will > degenerate only if cpu_bigcore_mask() and shared_cache_mask() return > the same value. This may work for existing platforms, because either > shared_caches don't exist, or when they do, cpu_bigcore_mask and > shared_cache_mask return the same set of CPUs. But this may or may not > continue to hold good in the future. > > Furthermore, if that is always going to be the case that in the > presence of shared_caches the cpu_bigcore_mask() and > shared_cache_mask() will always be the same, then why even define two > separate masks and not just have only the cpu_bigcore_mask() ? >
Your two statements are contradicting. In the former you are saying we should be future proof and in the latter, you are asking for why add if they are both going to be the same. > The correct way would be to set the powerpc_topology[mc_idx].mask to > powerpc_topology[bigcore_idx].mask *after* we have fixedup the > big_core level. The reason I modified it in v4 is not for degeneration or for future case but for the current PowerNV/SMT 4 case. I could have as well detected the the same and modified bigcore but thought fixup at one place would be better. -- Thanks and Regards Srikar Dronamraju