All of lore.kernel.org
 help / color / mirror / Atom feed
From: venkatesh.pallipadi@intel.com
To: len.brown@intel.com, andi@firstfloor.org
Cc: linux-pm@lists.osdl.org
Subject: [patch 2/3] cpuidle: Menu governor fix wrong usage of measured_us
Date: Wed, 30 Jul 2008 19:21:43 -0700	[thread overview]
Message-ID: <20080731022215.894712000@intel.com> (raw)
In-Reply-To: 20080731022141.088307000@intel.com

[-- Attachment #1: cpuidle_menu_fix.patch --]
[-- Type: text/plain, Size: 2836 bytes --]

There is a bug in menu governor where we have
		if (data->elapsed_us < data->elapsed_us + measured_us)

with measured_us already having elapsed_us added in tickless case here
	unsigned int measured_us =
		cpuidle_get_last_residency(dev) + data->elapsed_us;


Also, it should be last_residency, not measured_us, that need to be used to
do comparing and distinguish between expected & non-expected events.

Refactor menu_reflect() to fix these two problems.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Wei Gang <gang.wei@intel.com>

---
 drivers/cpuidle/governors/menu.c |   31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

Index: linux-2.6/drivers/cpuidle/governors/menu.c
===================================================================
--- linux-2.6.orig/drivers/cpuidle/governors/menu.c	2008-07-28 16:25:58.000000000 -0700
+++ linux-2.6/drivers/cpuidle/governors/menu.c	2008-07-28 16:29:18.000000000 -0700
@@ -74,9 +74,9 @@ static void menu_reflect(struct cpuidle_
 {
 	struct menu_device *data = &__get_cpu_var(menu_devices);
 	int last_idx = data->last_state_idx;
-	unsigned int measured_us =
-		cpuidle_get_last_residency(dev) + data->elapsed_us;
+	unsigned int last_idle_us = cpuidle_get_last_residency(dev);
 	struct cpuidle_state *target = &dev->states[last_idx];
+	unsigned int measured_us;
 
 	/*
 	 * Ugh, this idle state doesn't support residency measurements, so we
@@ -84,20 +84,27 @@ static void menu_reflect(struct cpuidle_
 	 * for one full standard timer tick.  However, be aware that this
 	 * could potentially result in a suboptimal state transition.
 	 */
-	if (!(target->flags & CPUIDLE_FLAG_TIME_VALID))
-		measured_us = USEC_PER_SEC / HZ;
+	if (unlikely(!(target->flags & CPUIDLE_FLAG_TIME_VALID)))
+		last_idle_us = USEC_PER_SEC / HZ;
 
-	/* Predict time remaining until next break event */
-	if (measured_us + BREAK_FUZZ < data->expected_us - target->exit_latency) {
-		data->predicted_us = max(measured_us, data->last_measured_us);
+	/*
+	 * measured_us and elapsed_us are the cumulative idle time, since the
+	 * last time we were woken out of idle by an interrupt.
+	 */
+	if (data->elapsed_us <= data->elapsed_us + last_idle_us)
+		measured_us = data->elapsed_us + last_idle_us;
+	else
+		measured_us = -1;
+
+	/* Predict time until next break event */
+	data->predicted_us = max(measured_us, data->last_measured_us);
+
+	if (last_idle_us + BREAK_FUZZ <
+	    data->expected_us - target->exit_latency) {
 		data->last_measured_us = measured_us;
 		data->elapsed_us = 0;
 	} else {
-		if (data->elapsed_us < data->elapsed_us + measured_us)
-			data->elapsed_us = measured_us;
-		else
-			data->elapsed_us = -1;
-		data->predicted_us = max(measured_us, data->last_measured_us);
+		data->elapsed_us = measured_us;
 	}
 }
 

-- 

  parent reply	other threads:[~2008-07-31  2:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-31  2:21 [patch 0/3] cpuilde: Misc bug fixes venkatesh.pallipadi
2008-07-31  2:21 ` [patch 1/3] cpuidle: Do not use poll_idle unless user asks for it venkatesh.pallipadi
2008-07-31  2:21 ` venkatesh.pallipadi [this message]
2008-07-31  2:21 ` [patch 3/3] cpuidle: Make ladder governor honor latency requirements fully venkatesh.pallipadi

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=20080731022215.894712000@intel.com \
    --to=venkatesh.pallipadi@intel.com \
    --cc=andi@firstfloor.org \
    --cc=len.brown@intel.com \
    --cc=linux-pm@lists.osdl.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.