linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@techsingularity.net>
To: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: Takashi Iwai <tiwai@suse.de>, linux-kernel@vger.kernel.org
Subject: Re: ACPI _CST introduced performance regresions on Haswll
Date: Tue, 6 Oct 2020 22:18:20 +0100	[thread overview]
Message-ID: <20201006211820.GN3227@techsingularity.net> (raw)
In-Reply-To: <25f31d3e-7a67-935f-93ba-32216a5084e2@intel.com>

On Tue, Oct 06, 2020 at 09:29:24PM +0200, Rafael J. Wysocki wrote:
> > After the commit, the default_status file does not appear in /sys
> > 
> Something is amiss, then, because the commit doesn't affect the presence of
> this file.
> 

This was cleared up in another mail.

> The only thing it does is to set the use_acpi flag for several processor
> models in intel_idle.c.
> 
> It can be effectively reversed by removing all of the ".use_acpi = true,"
> lines from intel_idle.c.
> 
> In particular, please check if changing the value of use_acpi in struct
> idle_cpu_hsx from 'true' to 'false' alone (without reverting the commit)
> makes the issue go away in 5.9-rc8 (the default_status file should be
> present regardless).
> 

Thanks.  I applied the following

diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index 9a810e4a7946..6478347669a9 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -1044,7 +1044,7 @@ static const struct idle_cpu idle_cpu_hsw __initconst = {
 static const struct idle_cpu idle_cpu_hsx __initconst = {
 	.state_table = hsw_cstates,
 	.disable_promotion_to_c1e = true,
-	.use_acpi = true,
+	.use_acpi = false,
 };

netperf UDP_STREAM
                                      pre                 enable                 enable                5.9-rc8                5.9-rc8
                                      cst                    cst        cst-no-hsx-acpi                vanilla            no-hsx-acpi
Hmean     send-64       203.96 (   0.00%)      179.23 * -12.13%*      201.04 (  -1.44%)      203.24 (  -0.36%)      233.43 *  14.45%*
Hmean     send-128      403.66 (   0.00%)      355.99 * -11.81%*      402.28 (  -0.34%)      387.65 *  -3.97%*      461.47 *  14.32%*
Hmean     send-256      784.39 (   0.00%)      697.78 * -11.04%*      782.15 (  -0.29%)      758.49 *  -3.30%*      895.31 *  14.14%*
Hmean     recv-64       203.96 (   0.00%)      179.23 * -12.13%*      201.04 (  -1.44%)      203.24 (  -0.36%)      233.43 *  14.45%*
Hmean     recv-128      403.66 (   0.00%)      355.99 * -11.81%*      402.28 (  -0.34%)      387.65 *  -3.97%*      461.47 *  14.32%*
Hmean     recv-256      784.39 (   0.00%)      697.78 * -11.04%*      782.15 (  -0.29%)      758.49 *  -3.30%*      895.28 *  14.14%*

This is a more limited run to save time but is enough to illustrate
the point.

pre-cst is just before your patch
enable-cst is your patch that was bisected
enable-cst-no-hsx-acpi is your patch with use_acpi disabled
5.9-rc8-vanilla is what it sounds like
5.9-rc8-no-hsx-acpi disables use_acpi

The enable-cst-no-hsx-acpi result indicates that use_acpi was the issue for
Haswell (at least these machines). Looking just at 5.9-rc8-vanillaa might
have been misleading because its performance is not far off the baseline
due to unrelated changes that mostly offset the performance penalty.

The key question is -- how appropriate would it be to disable acpi for
Haswell? Would that be generally safe or could it hide other surprises?

-- 
Mel Gorman
SUSE Labs

  reply	other threads:[~2020-10-06 21:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-06  8:36 ACPI _CST introduced performance regresions on Haswll Mel Gorman
2020-10-06 16:00 ` Rafael J. Wysocki
2020-10-06 19:03   ` Mel Gorman
2020-10-06 19:29     ` Rafael J. Wysocki
2020-10-06 21:18       ` Mel Gorman [this message]
2020-10-07 15:45         ` Rafael J. Wysocki
2020-10-08  9:09           ` Mel Gorman
2020-10-08 17:15             ` Rafael J. Wysocki
2020-10-08 17:34               ` Mel Gorman
2020-10-13 18:55                 ` Rafael J. Wysocki
2020-10-14 22:37                   ` Mel Gorman
2020-10-15 18:34                     ` Mel Gorman
2020-10-16 13:41                       ` Rafael J. Wysocki
2020-10-16 14:09                         ` Mel Gorman
2020-10-16 15:29                           ` Rafael J. Wysocki
2020-10-06 19:47     ` Mel Gorman
2020-10-07 15:40       ` Rafael J. Wysocki
2020-10-07 19:23         ` Mel Gorman

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=20201006211820.GN3227@techsingularity.net \
    --to=mgorman@techsingularity.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=tiwai@suse.de \
    /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).