[PATCH v3] topology: make core_mask include at least cluster_siblings

[Dietmar Eggemann]

- Barry Song <song.bao.hua at hisilicon.com> (always get undelivered mail
  returned to sender)
+ Barry Song <21cnbao at gmail.com>

On 14/03/2022 17:54, Darren Hart wrote:
> On Mon, Mar 14, 2022 at 05:35:05PM +0100, Dietmar Eggemann wrote:
>> On 09/03/2022 19:26, Darren Hart wrote:
>>> On Wed, Mar 09, 2022 at 01:50:07PM +0100, Dietmar Eggemann wrote:
>>>> On 08/03/2022 18:49, Darren Hart wrote:
>>>>> On Tue, Mar 08, 2022 at 05:03:07PM +0100, Dietmar Eggemann wrote:
>>>>>> On 08/03/2022 12:04, Vincent Guittot wrote:
>>>>>>> On Tue, 8 Mar 2022 at 11:30, Will Deacon <will at kernel.org> wrote:

[...]

> Ultimately, this delivers the same result. I do think it imposes more complexity
> for everyone to address what as far as I'm aware only affect the one system.
> 
> I don't think the term "Cluster" has a clear and universally understood
> definition, so I don't think it's a given that "CLS should be sub-SD of MC". I

I agree, the term 'cluster' is overloaded but default_topology[] clearly
says (with direction up means smaller SD spans).

  #ifdef CONFIG_SCHED_CLUSTER
        { cpu_clustergroup_mask, cpu_cluster_flags, SD_INIT_NAME(CLS) },
  #endif

  #ifdef CONFIG_SCHED_MC
        { cpu_coregroup_mask, cpu_core_flags, SD_INIT_NAME(MC) },
  #endif

In ACPI code we have `cluster_node = fetch_pptt_node(... ,
cpu_node->parent) but then the cache information (via
llc_id/llc_sibling) can change things which make this less easy to grasp.

> think this has been assumed, and that assumption has mostly held up, but this is
> an abstraction, and the abstraction should follow the physical topologies rather
> than the other way around in my opinion. If that's the primary motivation for
> this approach, I don't think it justifies the additional complexity.
> 
> All told, I prefer the 2 line change contained within cpu_coregroup_mask() which
> handles the one known exception with minimal impact. It's easy enough to come
> back to this to address more cases with a more complex solution if needed in the
> future - but I prefer to introduce the least amount of complexity as possible to
> address the known issues, especially if the end result is the same and the cost
> is paid by the affected systems.
> 
> Thanks,

Yeah, I can see your point. It's the smaller hack. My solution just
prevents us to manipulate the coregroup mask only to get the MC layer
degenerated by the core topology code. But people might say that's a
clever thing to do here. So I'm fine with your original solution as well.

[...]