Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210119112211.3196-1-mgorman@techsingularity.net>
 <CAKfTPtAsuY4aN6J2C+KCOpyJDULd6yEBZ_8zTLWRXwhakCq8oQ@mail.gmail.com>
 <20210119120220.GS3592@techsingularity.net> <CAKfTPtAWcVu5y_L93h47WHS1wkUZh=EPxyMDi5vSeNvx14Y_kQ@mail.gmail.com>
 <20210122101451.GV3592@techsingularity.net>
In-Reply-To: <20210122101451.GV3592@techsingularity.net>
From:   Vincent Guittot <vincent.guittot@linaro.org>
Date:   Fri, 22 Jan 2021 14:22:42 +0100
Message-ID: <CAKfTPtADmw_RVL-VTZgOMr2-4-AG0m4KeQLFsNXEEioyARif8A@mail.gmail.com>
Subject: Re: [PATCH v3 0/5] Scan for an idle sibling in a single pass
To:     Mel Gorman <mgorman@techsingularity.net>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Li Aubrey <aubrey.li@linux.intel.com>,
        Qais Yousef <qais.yousef@arm.com>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 22 Jan 2021 at 11:14, Mel Gorman <mgorman@techsingularity.net> wrote:
>
> On Fri, Jan 22, 2021 at 10:30:52AM +0100, Vincent Guittot wrote:
> > Hi Mel,
> >
> > On Tue, 19 Jan 2021 at 13:02, Mel Gorman <mgorman@techsingularity.net> wrote:
> > >
> > > On Tue, Jan 19, 2021 at 12:33:04PM +0100, Vincent Guittot wrote:
> > > > On Tue, 19 Jan 2021 at 12:22, Mel Gorman <mgorman@techsingularity.net> wrote:
> > > > >
> > > > > Changelog since v2
> > > > > o Remove unnecessary parameters
> > > > > o Update nr during scan only when scanning for cpus
> > > >
> > > > Hi Mel,
> > > >
> > > > I haven't looked at your previous version mainly because I'm chasing a
> > > > performance regression on v5.11-rcx which prevents me from testing the
> > > > impact of your patchset on my !SMT2 system.
> > > > Will do this as soon as this problem is fixed
> > > >
> > >
> > > Thanks, that would be appreciated as I do not have access to a !SMT2
> > > system to do my own evaluation.
> >
> > I have been able to run tests with your patchset on both large arm64
> > SMT4 system and small arm64 !SMT system and patch 3 is still a source
> > of regression on both. Decreasing min number of loops to 2 instead of
> > 4 and scaling it with smt weight doesn't seem to be a good option as
> > regressions disappear when I remove them as I tested with the patch
> > below
> >
> > hackbench -l 2560 -g 1 on 8 cores arm64
> > v5.11-rc4 : 1.355 (+/- 7.96)
> > + sis improvement : 1.923 (+/- 25%)
> > + the patch below : 1.332 (+/- 4.95)
> >
> > hackbench -l 2560 -g 256 on 8 cores arm64
> > v5.11-rc4 : 2.116 (+/- 4.62%)
> > + sis improvement : 2.216 (+/- 3.84%)
> > + the patch below : 2.113 (+/- 3.01%)
> >
> > So starting with a min of 2 loops instead of 4 currently and scaling
> > nr loop with smt weight doesn't seem to be a good option and we should
> > remove it for now
> >
>
> Ok
>
> Note that this is essentially reverting the patch. As you remove "nr *=
> sched_smt_weight", the scan is no longer proportional to cores, it's

Yes. My goal above was to narrow the changes only to lines that
generate the regressions but i agree that removing patch 3 is the
right solution


> proportial to logical CPUs and the rest of the patch and changelog becomes
> meaningless. On that basis, I'll queue tests over the weekend that remove
> this patch entirely and keep the CPU scan as a single pass.
>
> --
> Mel Gorman
> SUSE Labs