Re: [PATCH] sched/topology: Set SD_PREFER_SIBLING consistently on non-NUMA levels

[Morten Rasmussen]

[Resent without the disclaimer at the bottom. Sorry!]

On Wed, Nov 29, 2017 at 10:24:57AM +0100, Vincent Guittot wrote:
> Hi Morten,
>
> On 27 November 2017 at 18:29, Morten Rasmussen <morten.rasmussen@xxxxxxx> wrote:
> > SD_PREFER_SIBLING adds an additional bias towards spreading tasks on the
> > _parent_ sched_domain even if a sched_group isn't overloaded. It is
> > currently set on:
> >
> >    1. SMT level to promote spreading to sibling cores rather than using
> >       sibling HW-threads (caff37ef96eac7fe96a).
> >
> >    2. Non-NUMA levels which don't have the SD_SHARE_PKG_RESOURCES flag
> >       set (= DIE level in the default topology) as it was found to
> >       improve benchmarks on certain NUMA systems (6956dc568f34107f1d02b).
>
> So the goal is to have the last non-NUMA level with the flag so we can
> spread between "DIE"

The flag is already set on "DIE", the goal is to be consistent and set
the flag consistently. As it is, it is set on SMT and DIE, but not MC:

SMT: SD_PREFER_SIBLING
MC:  !SD_PREFER_SIBLING
DIE: SD_PREFER_SIBLING

So the goal is to spread consistently at all non-NUMA levels regardless
of which levels are present as currently it is not that obvious when
spreading happens on MC level as this list illustrates.

>
> >
> >    3. Any non-NUMA level that inherits the flag due to elimination of
> >       its parent sched_domain level in the de-generate step of the
> >       sched_domain hierarchy set up (= MC level in the default
> >       topology).
>
> This is to ensure that the last non NUMA level has the flag when the
> DIE one disappears but the goal is the same as 2.

My point was just summarize the conditions for enabling spreading. It
doesn't guarantee that the last non-NUMA level is spreading. For
example, x86 numa-in-package topology doesn't have DIE at all, so there
is nothing to inherit. Going the argument used to introduce the flag on
DIE level, it should be set on MC for those systems.

The whole thing really boils down to: Why do we want to not always
spread at all levels inside a single node? It seems that we almost
always do that already. So why not make it truly always?

> > Preferring siblings seems to be a useful tweak for all non-NUMA levels,
> > so we should enable it on all non-NUMA levels. As it is, it is possible
> > to have it SMT and DIE, but not MC in between when using the default
> > topology.
>
> So you want to extend it to all non NUMA level. And especially you
> want to spread tasks in each MC groups when we have DIE and MC levels.
> Have you got benchmark results to show improvement or is it just to
> align topology configuration?

It is purely to have consistent scheduling behaviour and make it clearer
how system topology affect scheduling. SD_PREFER_SIBLING is currently a
behavioral flag, which means that each architecture cannot modify it, it
is set by a set of rules based on the number of levels used by the
architecture and the topology flags set. So should an architecture have
platform with two or more non-SMT non-NUMA levels it is currently not
possible to make the scheduler spread on both levels.

> The fact that this flag improves bench for SMT and NUMA level doesn't
> mean that it will improve for MC level as well. We have the
> wake_wide/wake_affine stuffs that tries to do similar thing
> dynamically and it regularly improves/regresses benchmark like
> sysbench or hackbench

True. wake_wide/wake_affine in the wake-up path has some of the same
behaviour as SD_PREFER_SIBLING has for periodic/idle/nohz balancing.
However, that is already consistently enabled for all levels apart for
NUMA levels where the reclaim distance is too high. So currently we do
allow some spreading, but not consistently.

I can't say that this patch won't cause any regressions. I was hoping
someone would say that it is definitely a bad idea with a good reason,
or we try it for the benefit of consistent flag setting. I'm happy to
drop the patch if there is good explanation for why the flag is set like
it is, but I think we have to make it controllable by each architecture
then.