Re: [PATCH 2/2] sched/fair: Adjust the allowed NUMA imbalance when SD_NUMA spans multiple LLCs

From: Barry Song <21cnbao@gmail.com>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Valentin Schneider <valentin.schneider@arm.com>,
	Aubrey Li <aubrey.li@linux.intel.com>,
	Barry Song <song.bao.hua@hisilicon.com>,
	Mike Galbraith <efault@gmx.de>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] sched/fair: Adjust the allowed NUMA imbalance when SD_NUMA spans multiple LLCs
Date: Sat, 4 Dec 2021 00:14:33 +1300	[thread overview]
Message-ID: <CAGsJ_4zigz0rs9ubPjJKn0s9FsHp6A1Gpoh+cyc=cQnnMEMgYg@mail.gmail.com> (raw)
In-Reply-To: <20211203105055.GB3366@techsingularity.net>

On Fri, Dec 3, 2021 at 11:50 PM Mel Gorman <mgorman@techsingularity.net> wrote:
>
> On Fri, Dec 03, 2021 at 09:15:15PM +1300, Barry Song wrote:
> > > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > > index d201a7052a29..fee2930745ab 100644
> > > --- a/kernel/sched/topology.c
> > > +++ b/kernel/sched/topology.c
> > > @@ -2242,6 +2242,26 @@ build_sched_domains(const struct cpumask *cpu_map, struct sched_domain_attr *att
> > >                 }
> > >         }
> > >
> > > +       /* Calculate allowed NUMA imbalance */
> > > +       for_each_cpu(i, cpu_map) {
> > > +               int imb_numa_nr = 0;
> > > +
> > > +               for (sd = *per_cpu_ptr(d.sd, i); sd; sd = sd->parent) {
> > > +                       struct sched_domain *child = sd->child;
> > > +
> > > +                       if (!(sd->flags & SD_SHARE_PKG_RESOURCES) && child &&
> > > +                           (child->flags & SD_SHARE_PKG_RESOURCES)) {
> > > +                               int nr_groups;
> > > +
> > > +                               nr_groups = sd->span_weight / child->span_weight;
> > > +                               imb_numa_nr = max(1U, ((child->span_weight) >> 1) /
> > > +                                               (nr_groups * num_online_nodes()));
> >
> > Hi Mel, you used to have 25% * numa_weight if node has only one LLC.
> > for a system with 4 numa,  In case sd has 2 nodes, child is 1 numa node,
> > then  nr_groups=2, num_online_nodes()=4,  imb_numa_nr will be
> > child->span_weight/2/2/4?
> >
> > Does this patch change the behaviour for machines whose numa equals LLC?
> >
>
> Yes, it changes behaviour. Instead of a flat 25%, it takes into account
> the number of LLCs per node and the number of nodes overall.

Considering  the number of nodes overall seems to be quite weird to me.
for example, for the below machines

1P * 2DIE = 2NUMA:    node1 - node0
2P * 2DIE = 4NUMA:    node1 - node0  ------ node2 - node3
4P * 2DIE = 8NUMA:    node1 - node0  ------ node2 - node3
                                      node5 - node4  ------ node6 - node7

if one service pins node1 and node0 in all above configurations, it seems in all
different machines, the app will result in different behavior.

the other example is:
in a 2P machine, if one app pins the first two NUMAs, the other app pins
the last two NUMAs, why would the  num_online_nodes() matter to them?
there is no balance requirement between the two P.

>
> --
> Mel Gorman
> SUSE Labs

Thanks
Barry