linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Huang Rui <ray.huang@amd.com>
To: Giovanni Gherdovich <ggherdovich@suse.cz>
Cc: "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Borislav Petkov <bp@suse.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"Sharma, Deepak" <Deepak.Sharma@amd.com>,
	"Deucher, Alexander" <Alexander.Deucher@amd.com>,
	"Limonciello, Mario" <Mario.Limonciello@amd.com>,
	Steven Noonan <steven@valvesoftware.com>,
	"Fontenot, Nathan" <Nathan.Fontenot@amd.com>,
	"Su, Jinzhou (Joe)" <Jinzhou.Su@amd.com>,
	"Du, Xiaojian" <Xiaojian.Du@amd.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH v3 00/21] cpufreq: introduce a new AMD CPU frequency control mechanism
Date: Sat, 6 Nov 2021 00:09:59 +0800	[thread overview]
Message-ID: <YYVXV/sCbO0mPVK2@hr-amd> (raw)
In-Reply-To: <a0e932477e9b826c0781dda1d0d2953e57f904cc.camel@suse.cz>

On Fri, Nov 05, 2021 at 12:40:18AM +0800, Giovanni Gherdovich wrote:
> On Fri, 2021-10-29 at 21:02 +0800, Huang Rui wrote:
> > Hi all,
> > 
> > We would like to introduce a new AMD CPU frequency control mechanism as the
> > "amd-pstate" driver for modern AMD Zen based CPU series in Linux Kernel.
> > 
> > ..snip..
> 
> Hello,
> 
> I've tested this driver and it seems the results are a little underwhelming.
> The test machine is a two sockets server with two AMD EPYC 7713,
> family:model:stepping 25:1:1, 128 cores/256 threads, 256G of memory and SSD
> storage. On this system, the amd-pstate driver works only in "shared memory
> support", not in "full MSR support", meaning that frequency switches are
> triggered from a workqueue instead of scheduler context (!fast_switch).
> 

Hi Giovanni,

I am really appreciated for the detailed tests and analysis! Thank you!

The initial driver was developed on a mobile CPU (Cezanne) with 8 cores/16
threads which supports the "full MSR" solution. And we spent a lot of time
to debug with BIOS, SMU firmware, and hardware guys to bring up this driver
on this CPU. The test results we provided were based on those series of
processors.

For the processors with "shared memory solution", we bring it up in a short
time recently to hope more AMD processors to also support new driver. :-)
Although our CPUs comply with the ACPI standard theoretically, different
processors have different SBIOS and SMU firmware (I assumed you know this
in previous mail). In real case, we need to verify it one by one, because
there are some differences in SBIOS ACPI _CPC table and firmware
implementation.

Of course, right now, we can start to optimize other processors and "shared
memory solution" in parallel.

Would you mind that we add a module param or filter the known good
processors (mobile parts) to load amd-pstate. And others can use the param
to switch between amd-pstate and acpi-cpufreq manually? After we address the
performance gap, then we can switch it back.

> Dbench sees some ludicrous improvements in both performance and performance
> per watt; likewise netperf sees some modest improvements, but that's about
> the only good news. Schedutil/ondemand on tbench and hackbench do worse
> with amd-pstate than acpi-cpufreq. I don't have data for
> ondemand/amd-pstate on kernbench and gitsource, but schedutil regresses on
> both.
> 
> Here the tables, then some questions & discussion points.
> 
> Tilde (~) means the result is the same as baseline (which is, the ratio is close to 1).
> "Sugov" means "schedutil governor", "perfgov" means "performance governor".
> 
>              :        acpi-cpufreq          :        amd-pstate          :
>  - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
>              :  ondemand  sugov  perfgov    :  ondemand  sugov  perfgov  :  better if
>  - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
>                                        PERFORMANCE RATIOS
> dbench       :  1.00      ~      0.33       :  0.37      0.35   0.36     :  lower
> netperf      :  1.00      0.97   ~          :  1.03      1.04   ~        :  higher
> tbench       :  1.00      1.04   1.06       :  0.83      0.40   1.05     :  higher
> hackbench    :  1.00      ~      1.03       :  1.09      1.42   1.03     :  lower
> kernbench    :  1.00      0.96   0.97       :  N/A       1.08   ~        :  lower
> gitsource    :  1.00      0.67   0.69       :  N/A       0.79   0.67     :  lower
>  - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
>                                   PERFORMANCE-PER-WATT RATIOS
> dbench4      :  1.00      ~      3.37       :  2.68      3.12   3.03     :  higher
> netperf      :  1.00      0.96   ~          :  1.09      1.06   ~        :  higher
> tbench4      :  1.00      1.03   1.06       :  0.76      0.34   1.04     :  higher
> hackbench    :  1.00      ~      0.95       :  0.88      0.65   0.96     :  higher
> kernbench    :  1.00      1.06   1.05       :  N/A       0.93   1.05     :  higher
> gitsource    :  1.00      1.53   1.50       :  N/A       1.33   1.55     :  higher
> 
> 
> How to read the table: all numbers are ratios of the results of some
> governor/driver combination and ondemand/acpi-cpufreq, which is the
> baseline (first column). When the "better if" column says "higher", a ratio
> larger than 1 indicates an improvement; otherwise it's a regression.
> Example: hackbench with sugov/amd-pstate is 42% slower than with
> ondemand/acpi-cpufreq (top table). At the same time, it's also 35% less
> efficient (bottom table).

It seems the issue mainly from the processors with big number of cores and
threads. Let's find the similiar family threadripper or EYPC processors to
duplicate the test results. Will contact at you for details. :-)

> 
> Now, some questions / possible troubleshooting directions:
> 
> - ACPI-CPUFREQ DRIVER: REQUESTS ARE HINTS OR MANDATES?
>   When using acpi-cpufreq, and the OS requests some frequency (one of the
>   three allowed P-States), does the hardware underneath stick to it? Or
>   does it do some ulterior adjustment based on the load?
>   This would tell if a machine using acpi-cpufreq is less dumb than it
>   seems, and can in principle do fine-grain adjustments all the same.
> 

The acpi-cpufreq driver should request the frequency level to go, however,
the firmware has a policy to adjust clock as well according to the hardware
condition such as voltage, electricity, and temperature. Legacy ACPI
P-state doesn't have any transaction to firmware side. But on amd-pstate,
the firmware can detects the performance goals as the hints that driver
provides.

> - PROCESSING CPPC DOORBELL REQUESTS: HOW FAST IS THAT?
>   How long does it take the hardware to process the CPPC doorbell
>   request to change frequency? What happens to outstanding requests, if
>   they're not processed in a timely manner? Is there any queue of requests,
>   and if so, how long is it? Could it be that if requests come in too quickly
>   the CPU ends up playing catch-up on freq switches that are obsoletes or
>   redundant?

That's a good question. We need to consult with firmware and hardware guys.
Or any method, we can caculate it from software side.

> 
> - LIKE-FOR-LIKE: TRY BENCHMARKING WITH AMD-PSTATE LIMITED TO 3 P-STATES?
>   Could it be that to study the performance of the "shared memory support"
>   system against acpi-cpufreq a more like-to-like comparison would be to limit
>   amd-pstate to only the 3 P-States available to acpi-cpufreq? That would be
>   for experimental/benchmarking purposes only. Eg: on my machines acpi-cpufreq
>   sees 1.5GHz, 1.7GHz and 2GHz. Given that max boost is 3.72GHz, and the CPPC
>   range is the abstract interval 0..255, I could limit amd-pstate to only set
>   performance level of 68, 102 and 137, and see what it gives against the old
>   driver. What do you think?

That's good idea. We can give some experiments like this.

> 
> - PROCESSING CPPC DOORBELL REQS IS SLOW. BUT /MAKING/ A REQUEST, SLOW TOO?
>   Looks to me that with the "shared memory support" the frequency update
>   process is doubly asynchronous: first we have the ->target() callback
>   deferred to a workqueue, then when it's eventually executed, it calls
>   cppc_update_perf() which again just asks the firmware to do work at a
>   later time. Are we sure that cppc_update_perf() is actually so slow to
>   warrant !fast_switch?

That's a good question! I think your platform with "shared memory support"
is actually to read/write the memory in Platform Communication Channel
(PCC) to update the performance goals. However, acpi-cpufreq driver is
using the MSR registers with cpu_freq_write_amd()/cpu_freq_read_amd().

Is that possible that MSR register access faster than the memory doorbell
in PCC?

> 
> - HOW MANY P-STATES ARE TOO MANY?
>   I've always believed the contrary, but what if having too many P-States is
>   harmful for both performance and efficiency? Maybe the governor is
>   requesting many updates in small increments where less (and larger) updates
>   would be more appropriate?

I am thinking that, maybe, we can dig out better policy to control the
perf range.


Thanks again for questions / possible troubleshooting directions. They are
very helpful. Next step, let us find out what is the root cause of the
performance gap between acpi-cpufreq and amd-pstate driver.

Thanks,
Ray

  reply	other threads:[~2021-11-05 16:10 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-29 13:02 [PATCH v3 00/21] cpufreq: introduce a new AMD CPU frequency control mechanism Huang Rui
2021-10-29 13:02 ` [PATCH v3 01/21] x86/cpufreatures: add AMD Collaborative Processor Performance Control feature flag Huang Rui
2021-10-29 14:39   ` Borislav Petkov
2021-11-06 10:28   ` Borislav Petkov
2021-11-09  3:08     ` Huang Rui
2021-10-29 13:02 ` [PATCH v3 02/21] x86/msr: add AMD CPPC MSR definitions Huang Rui
2021-10-29 13:02 ` [PATCH v3 03/21] ACPI: CPPC: implement support for SystemIO registers Huang Rui
2021-10-29 13:02 ` [PATCH v3 04/21] ACPI: CPPC: Check present CPUs for determining _CPC is valid Huang Rui
2021-10-29 13:02 ` [PATCH v3 05/21] ACPI: CPPC: add cppc enable register function Huang Rui
2021-10-29 14:15   ` Limonciello, Mario
2021-11-01  9:20     ` Huang Rui
2021-10-29 13:02 ` [PATCH v3 06/21] cpufreq: amd: introduce a new amd pstate driver to support future processors Huang Rui
2021-11-02 18:52   ` Limonciello, Mario
2021-11-02 19:38   ` Nathan Fontenot
2021-11-03  7:01     ` Huang Rui
2021-11-04 15:10       ` Nathan Fontenot
2021-11-05  4:20         ` Huang Rui
2021-10-29 13:02 ` [PATCH v3 07/21] cpufreq: amd: add fast switch function for amd-pstate Huang Rui
2021-10-29 14:16   ` Limonciello, Mario
2021-11-02 19:56   ` Nathan Fontenot
2021-10-29 13:02 ` [PATCH v3 08/21] cpufreq: amd: add acpi cppc function as the backend for legacy processors Huang Rui
2021-10-29 14:20   ` Limonciello, Mario
2021-11-01  9:02     ` Huang Rui
2021-11-02 18:46   ` Nathan Fontenot
2021-11-03 12:00     ` Huang Rui
2021-10-29 13:02 ` [PATCH v3 09/21] cpufreq: amd: add trace for amd-pstate module Huang Rui
2021-10-29 13:02 ` [PATCH v3 10/21] cpufreq: amd: add boost mode support for amd-pstate Huang Rui
2021-10-29 13:02 ` [PATCH v3 11/21] cpufreq: amd: add amd-pstate frequencies attributes Huang Rui
2021-11-05 18:59   ` Nathan Fontenot
2021-11-10 12:28     ` Huang Rui
2021-10-29 13:02 ` [PATCH v3 12/21] cpufreq: amd: add amd-pstate performance attributes Huang Rui
2021-11-05 18:50   ` Nathan Fontenot
2021-10-29 13:02 ` [PATCH v3 13/21] cpupower: add AMD P-state capability flag Huang Rui
2021-10-29 13:02 ` [PATCH v3 14/21] cpupower: add the function to check amd-pstate enabled Huang Rui
2021-10-29 13:02 ` [PATCH v3 15/21] cpupower: initial AMD P-state capability Huang Rui
2021-10-29 13:02 ` [PATCH v3 16/21] cpupower: add the function to get the sysfs value from specific table Huang Rui
2021-10-29 13:02 ` [PATCH v3 17/21] cpupower: add amd-pstate sysfs definition and access helper Huang Rui
2021-10-29 14:10   ` Limonciello, Mario
2021-11-01  9:14     ` Huang Rui
2021-10-29 13:02 ` [PATCH v3 18/21] cpupower: enable boost state support for amd-pstate module Huang Rui
2021-11-02 20:11   ` Nathan Fontenot
2021-11-03  7:04     ` Huang Rui
2021-10-29 13:02 ` [PATCH v3 19/21] cpupower: move print_speed function into misc helper Huang Rui
2021-10-29 13:02 ` [PATCH v3 20/21] cpupower: print amd-pstate information on cpupower Huang Rui
2021-10-29 13:02 ` [PATCH v3 21/21] Documentation: amd-pstate: add amd-pstate driver introduction Huang Rui
2021-11-04 16:40 ` [PATCH v3 00/21] cpufreq: introduce a new AMD CPU frequency control mechanism Giovanni Gherdovich
2021-11-05 16:09   ` Huang Rui [this message]
2021-11-06  8:58     ` Matt McDonald
2021-11-08  9:20       ` Huang Rui
2021-11-12 11:21         ` Du, Xiaojian

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YYVXV/sCbO0mPVK2@hr-amd \
    --to=ray.huang@amd.com \
    --cc=Alexander.Deucher@amd.com \
    --cc=Deepak.Sharma@amd.com \
    --cc=Jinzhou.Su@amd.com \
    --cc=Mario.Limonciello@amd.com \
    --cc=Nathan.Fontenot@amd.com \
    --cc=Xiaojian.Du@amd.com \
    --cc=bp@suse.de \
    --cc=ggherdovich@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=skhan@linuxfoundation.org \
    --cc=steven@valvesoftware.com \
    --cc=viresh.kumar@linaro.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).