RE: [PATCH 1/1] sched/topology: Make sched_init_numa() use a set for the deduplicating sort

[Valentin Schneider <valentin.schneider@arm.com>]

On 29/01/21 02:02, Song Bao Hua (Barry Song) wrote:
>> -----Original Message-----
>> From: Valentin Schneider [mailto:valentin.schneider@arm.com]
>> That is another thing that's worth considering. Morten was arguing that if
>> the distance between two nodes is so tiny, it might not be worth
>> representing it at all in the scheduler topology.
>
> Yes. I agree it is a different thing. Anyway, I saw your patch has been
> in sched tree. One side effect your patch is the one more sched_domain
> level is imported for this topology:
>
>                             24
>                       X X XXX X X  X X X X XXX
>              XX XX X                          XXXXX
>          XXX                                        X
>        XX                                             XXX
>      XX                                 22              XXX
>      X                           XXXXXXX                   XX
>     X                        XXXXX      XXXXXXXXX           XXXX
>    XX                      XXX                    XX X XX X    XX
> +--------+           +---------+          +---------+      XX+---------+
> | 0      |   12      | 1       | 20       | 2       |   12   |3        |
> |        +-----------+         +----------+         +--------+         |
> +---X----+           +---------+          +--X------+        +---------+
>     X                                        X
>     XX                                      X
>      X                                     XX
>       XX                                  XX
>        XX                                X
>         X XXX                         XXX
>              X XXXXXX XX XX X X X XXXX
>                        22
> Without the patch, Linux will use 10,12,22,24 to build sched_domain;
> With your patch, Linux will use 10,12,20,22,24 to build sched_domain.
>
> So one more layer is added. What I have seen is that:
>
> For node0 sched_domain <=12 and sched_domain <=20 span the same range
> (node0, node1). So one of them is redundant. then in cpu_attach_domain,
> the redundant one is dropped due to "remove the sched domains which
> do not contribute to scheduling".
>
> For node1&2, the origin code had no "20", thus built one less sched_domain
> level.
>

Right, that domain degeneration should get you to the same result. We do
want to make sure we're handling every distance value in the table; the
gist is to "stupidly" build every domain / level we can, and if some are
redundant we can remove them after the fact.

> What is really interesting is that removing 20 actually gives better
> benchmark in speccpu :-)
>
>
>>
>> > Thanks
>> > Barry
>
> Thanks
> Barry