Messages in this thread | | | From | Vincent Guittot <> | Date | Fri, 22 Jan 2021 14:22:42 +0100 | Subject | Re: [PATCH v3 0/5] Scan for an idle sibling in a single pass |
| |
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
| |