linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Linux PM <linux-pm@vger.kernel.org>, Doug Smythies <dsmythies@telus.net>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>
Subject: [PATCH v1 1/2] cpuidle: teo: Fix alternative idle state lookup
Date: Fri, 30 Jul 2021 16:38:07 +0200	[thread overview]
Message-ID: <1794352.tdWV9SEqCh@kreacher> (raw)
In-Reply-To: <4336554.LvFx2qVVIh@kreacher>

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

There are three mistakes in the loop in teo_select() that is looking
for an alternative candidate idle state.  First, it should walk all
of the idle states shallower than the current candidate one,
including all of the disabled ones, but it terminates after the first
enabled idle state.  Second, it should not terminate its last step
if idle state 0 is disabled (which is related to the first issue).
Finally, it may return the current alternative candidate idle state
prematurely if the time span criterion is not met by the idle state
under consideration at the moment.

To address the issues mentioned above, make the loop in question walk
all of the idle states shallower than the current candidate idle state
all the way down to idle state 0 and rearrange the checks in it.

Fixes: 77577558f25d ("cpuidle: teo: Rework most recent idle duration values treatment")
Reported-by: Doug Smythies <dsmythies@telus.net>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/governors/teo.c |   40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/cpuidle/governors/teo.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/teo.c
+++ linux-pm/drivers/cpuidle/governors/teo.c
@@ -397,32 +397,46 @@ static int teo_select(struct cpuidle_dri
 		intercept_sum = 0;
 		recent_sum = 0;
 
-		for (i = idx - 1; i >= idx0; i--) {
+		for (i = idx - 1; i >= 0; i--) {
 			struct teo_bin *bin = &cpu_data->state_bins[i];
 			s64 span_ns;
 
 			intercept_sum += bin->intercepts;
 			recent_sum += bin->recent;
 
+			span_ns = teo_middle_of_bin(i, drv);
+
+			if ((!alt_recent || 2 * recent_sum > idx_recent_sum) &&
+			    (!alt_intercepts ||
+			     2 * intercept_sum > idx_intercept_sum)) {
+				if (teo_time_ok(span_ns) &&
+				    !dev->states_usage[i].disable) {
+					idx = i;
+					duration_ns = span_ns;
+				} else {
+					/*
+					 * The current state is too shallow or
+					 * disabled, so take the first enabled
+					 * deeper state with suitable time span.
+					 */
+					idx = last_enabled_idx;
+					duration_ns = last_enabled_span_ns;
+				}
+				break;
+			}
+
 			if (dev->states_usage[i].disable)
 				continue;
 
-			span_ns = teo_middle_of_bin(i, drv);
 			if (!teo_time_ok(span_ns)) {
 				/*
-				 * The current state is too shallow, so select
-				 * the first enabled deeper state.
+				 * The current state is too shallow, but if an
+				 * alternative candidate state has been found,
+				 * it may still turn out to be a better choice.
 				 */
-				duration_ns = last_enabled_span_ns;
-				idx = last_enabled_idx;
-				break;
-			}
+				if (last_enabled_idx != idx)
+					continue;
 
-			if ((!alt_recent || 2 * recent_sum > idx_recent_sum) &&
-			    (!alt_intercepts ||
-			     2 * intercept_sum > idx_intercept_sum)) {
-				idx = i;
-				duration_ns = span_ns;
 				break;
 			}
 




  reply	other threads:[~2021-07-30 14:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-30 14:35 [PATCH v1 0/2] cpuidle: teo: Fix selection when idle state 0 is disabled Rafael J. Wysocki
2021-07-30 14:38 ` Rafael J. Wysocki [this message]
2021-07-30 14:38 ` [PATCH v1 2/2] cpuidle: teo: Rename two local variables in teo_select() Rafael J. Wysocki
2021-07-31  4:04 ` [PATCH v1 0/2] cpuidle: teo: Fix selection when idle state 0 is disabled Doug Smythies
2021-08-03 13:18   ` Rafael J. Wysocki

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=1794352.tdWV9SEqCh@kreacher \
    --to=rjw@rjwysocki.net \
    --cc=dsmythies@telus.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rafael@kernel.org \
    --subject='Re: [PATCH v1 1/2] cpuidle: teo: Fix alternative idle state lookup' \
    /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

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox