mbox series

[v3,0/1] AMD EPYC: fix schedutil perf regression (freq-invariance)

Message ID 20210203135321.12253-1-ggherdovich@suse.cz
Headers show
Series AMD EPYC: fix schedutil perf regression (freq-invariance) | expand

Message

Giovanni Gherdovich Feb. 3, 2021, 1:53 p.m. UTC
v2 at https://lore.kernel.org/lkml/20210122204038.3238-1-ggherdovich@suse.cz

Changes wrt v2:

- removed redundant "#ifdef CONFIG_ACPI_CPPC_LIB"

Giovanni Gherdovich (1):
  x86,sched: On AMD EPYC set freq_max = max_boost in schedutil invariant
    formula

 drivers/cpufreq/acpi-cpufreq.c   | 61 ++++++++++++++++++++++++++++++--
 drivers/cpufreq/cpufreq.c        |  3 ++
 include/linux/cpufreq.h          |  5 +++
 kernel/sched/cpufreq_schedutil.c |  8 +++--
 4 files changed, 73 insertions(+), 4 deletions(-)

Comments

Rafael J. Wysocki Feb. 4, 2021, 1:40 p.m. UTC | #1
On Thu, Feb 4, 2021 at 12:36 AM Michael Larabel <Michael@phoronix.com> wrote:
>
> On 2/3/21 12:25 PM, Rafael J. Wysocki wrote:
> > On Wednesday, February 3, 2021 3:11:37 PM CET Rafael J. Wysocki wrote:
> >> On Wed, Feb 3, 2021 at 2:53 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
> >> [cut]
> >>
> >>> Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> >>> Fixes: 976df7e5730e ("x86, sched: Use midpoint of max_boost and max_P for frequency invariance on AMD EPYC")
> >>> Reported-by: Michael Larabel <Michael@phoronix.com>
> >>> Tested-by: Michael Larabel <Michael@phoronix.com>
> >>> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
> >>> ---
> >>>   drivers/cpufreq/acpi-cpufreq.c   | 61 ++++++++++++++++++++++++++++++--
> >>>   drivers/cpufreq/cpufreq.c        |  3 ++
> >>>   include/linux/cpufreq.h          |  5 +++
> >>>   kernel/sched/cpufreq_schedutil.c |  8 +++--
> >> I don't really think that it is necessary to modify schedutil to
> >> address this issue.
> > So below is a prototype of an alternative fix for the issue at hand.
> >
> > I can't really test it here, because there's no _CPC in the ACPI tables of my
> > test machines, so testing it would be appreciated.  However, AFAICS these
> > machines are affected by the performance issue related to the scale-invariance
> > when they are running acpi-cpufreq, so what we are doing here is not entirely
> > sufficient.
>
>
> I have benchmarks running on several Ryzen and EPYC systems with this
> patch. The full batch of tests won't be done until tomorrow, but in
> looking at the data so far from an AMD EPYC 7F72 2P server over the past
> few hours, this patch does provide fairly comparable numbers to
> Giovanni's patch. There were a few outliers so far but waiting to see
> with the complete set of results. At the very least it's clear enough
> already this new patch is at least an improvement over the current 5.11
> upstream state with schedutil on AMD.

Thanks for the feedback, much appreciated!

Let me submit the patch properly, then.
Rafael J. Wysocki Feb. 4, 2021, 1:55 p.m. UTC | #2
On Thu, Feb 4, 2021 at 2:49 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
>

> On Wed, 2021-02-03 at 19:25 +0100, Rafael J. Wysocki wrote:

> > [cut]

> >

> > So below is a prototype of an alternative fix for the issue at hand.

> >

> > I can't really test it here, because there's no _CPC in the ACPI tables of my

> > test machines, so testing it would be appreciated.  However, AFAICS these

> > machines are affected by the performance issue related to the scale-invariance

> > when they are running acpi-cpufreq, so what we are doing here is not entirely

> > sufficient.

> >

> > It looks like the scale-invariance code should ask the cpufreq driver about

> > the maximum frequency and note that cpufreq drivers may be changed on the

> > fly.

> >

> > What the patch below does is to add an extra entry to the frequency table for

> > each CPU to represent the maximum "boost" frequency, so as to cause that

> > frequency to be used as cpuinfo.max_freq.

> >

> > The reason why I think it is better to extend the frequency tables instead

> > of simply increasing the frequency for the "P0" entry is because the latter

> > may cause "turbo" frequency to be asked for less often.

> >

> > ---

> >  drivers/cpufreq/acpi-cpufreq.c |  107 ++++++++++++++++++++++++++++++++++++-----

> >  1 file changed, 95 insertions(+), 12 deletions(-)

>

> Hello Rafael,

>

> thanks for looking at this. Your patch is indeed cleaner than the one I proposed.

>

> Preliminary testing is favorable; more tests are running.

>

> Results from your patch are in the fourth column below; the performance from

> v5.10 looks restored.

>

> I'll follow up once the tests I queued are completed.


Thank you!

> TEST        : Intel Open Image Denoise, www.openimagedenoise.org

> INVOCATION  : ./denoise -hdr memorial.pfm -out out.pfm -bench 200 -threads $NTHREADS

> CPU         : MODEL            : 2x AMD EPYC 7742

>               FREQUENCY TABLE  : P2: 1.50 GHz

>                                  P1: 2.00 GHz

>                                  P0: 2.25 GHz

>               MAX BOOST        :     3.40 GHz

>

> Results: threads, msecs (ratio). Lower is better.

>

>                v5.10          v5.11-rc4   v5.11-rc4-ggherdov v5.11-rc6-rafael

>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>       1   1069.85 (1.00)   1071.84 (1.00)   1070.42 (1.00)   1069.12 (1.00)

>       2    542.24 (1.00)    544.40 (1.00)    544.48 (1.00)    540.81 (1.00)

>       4    278.00 (1.00)    278.44 (1.00)    277.72 (1.00)    277.79 (1.00)

>       8    149.81 (1.00)    149.61 (1.00)    149.87 (1.00)    149.51 (1.00)

>      16     79.01 (1.00)     79.31 (1.00)     78.94 (1.00)     79.02 (1.00)

>      24     58.01 (1.00)     58.51 (1.01)     58.15 (1.00)     57.84 (1.00)

>      32     46.58 (1.00)     48.30 (1.04)     46.66 (1.00)     46.70 (1.00)

>      48     37.29 (1.00)     51.29 (1.38)     37.27 (1.00)     38.10 (1.02)

>      64     34.01 (1.00)     49.59 (1.46)     33.71 (0.99)     34.51 (1.01)

>      80     31.09 (1.00)     44.27 (1.42)     31.33 (1.01)     31.11 (1.00)

>      96     28.56 (1.00)     40.82 (1.43)     28.47 (1.00)     28.65 (1.00)

>     112     28.09 (1.00)     40.06 (1.43)     28.63 (1.02)     28.38 (1.01)

>     120     28.73 (1.00)     39.78 (1.38)     28.14 (0.98)     28.16 (0.98)

>     128     28.93 (1.00)     39.60 (1.37)     29.38 (1.02)     28.55 (0.99)
Michael Larabel Feb. 4, 2021, 11:04 p.m. UTC | #3
On 2/4/21 7:40 AM, Rafael J. Wysocki wrote:
> On Thu, Feb 4, 2021 at 12:36 AM Michael Larabel <Michael@phoronix.com> wrote:
>> On 2/3/21 12:25 PM, Rafael J. Wysocki wrote:
>>> On Wednesday, February 3, 2021 3:11:37 PM CET Rafael J. Wysocki wrote:
>>>> On Wed, Feb 3, 2021 at 2:53 PM Giovanni Gherdovich <ggherdovich@suse.cz> wrote:
>>>> [cut]
>>>>
>>>>> Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
>>>>> Fixes: 976df7e5730e ("x86, sched: Use midpoint of max_boost and max_P for frequency invariance on AMD EPYC")
>>>>> Reported-by: Michael Larabel <Michael@phoronix.com>
>>>>> Tested-by: Michael Larabel <Michael@phoronix.com>
>>>>> Signed-off-by: Giovanni Gherdovich <ggherdovich@suse.cz>
>>>>> ---
>>>>>    drivers/cpufreq/acpi-cpufreq.c   | 61 ++++++++++++++++++++++++++++++--
>>>>>    drivers/cpufreq/cpufreq.c        |  3 ++
>>>>>    include/linux/cpufreq.h          |  5 +++
>>>>>    kernel/sched/cpufreq_schedutil.c |  8 +++--
>>>> I don't really think that it is necessary to modify schedutil to
>>>> address this issue.
>>> So below is a prototype of an alternative fix for the issue at hand.
>>>
>>> I can't really test it here, because there's no _CPC in the ACPI tables of my
>>> test machines, so testing it would be appreciated.  However, AFAICS these
>>> machines are affected by the performance issue related to the scale-invariance
>>> when they are running acpi-cpufreq, so what we are doing here is not entirely
>>> sufficient.
>>
>> I have benchmarks running on several Ryzen and EPYC systems with this
>> patch. The full batch of tests won't be done until tomorrow, but in
>> looking at the data so far from an AMD EPYC 7F72 2P server over the past
>> few hours, this patch does provide fairly comparable numbers to
>> Giovanni's patch. There were a few outliers so far but waiting to see
>> with the complete set of results. At the very least it's clear enough
>> already this new patch is at least an improvement over the current 5.11
>> upstream state with schedutil on AMD.
> Thanks for the feedback, much appreciated!
>
> Let me submit the patch properly, then.


Everything continues looking good in running this patch on a variety of 
AMD Zen2/Zen3 systems.

As Giovanni has been focusing on EPYC testing, I have been running 
several Ryzen laptops/desktop for more exposure and there it's looking 
very good. On a Ryzen 9 5900X[1] when looking at this latest patch 
against current 5.11 Git and 5.10, the performance is recovered and in 
some cases better off now than 5.10 with Schedutil. No anomalies there 
and with other Zen 2/3 desktops and Zen 2 notebook the performance 
relative to 5.10 is comparable or in some cases better while all 
indications point to the 5.11 regression being addressed. Some of the 
slower systems still finishing up but no unexpected results yet and 
likely just redundant testing at this point.

Tests on EPYC hardware has also been looking good. With some 44 tests on 
an EPYC 7F72 2P setup[2] when taking the geometric mean of all the data 
finding it rightly in line with the prior patch from Giovanni. EPYC 7702 
and EPYC 7F52 1P performance similarly showing no regression any longer 
with the patched kernel. This patch also seemed to help CPUFreq ondemand 
performance improve as well in some cases.

Will advise if hitting anything unexpected with the remainder of the 
testing but all is looking solid at this point and a definite 
improvement over the current 5.11 Git state.

Tested-by: Michael Larabel <Michael@phoronix.com>

[1] https://openbenchmarking.org/result/2102048-PTS-RYZEN95920 (5.10 
stable vs. 5.11 Git vs. patched.)
[2] https://openbenchmarking.org/result/2102048-HA-AMDEPYC7F37 
(Giovanni's earlier patch against this new patch, compared to unpatched 
Linux 5.11 Git and then Linux 5.11 with CPUfreq performance governor.)

Michael