Re: [PATCH v3 0/5] Scan for an idle sibling in a single pass

[Mel Gorman]

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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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
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