public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [patch 0/3] cpuilde: Misc bug fixes
@ 2008-07-31  2:21 venkatesh.pallipadi
  2008-07-31  2:21 ` [patch 1/3] cpuidle: Do not use poll_idle unless user asks for it venkatesh.pallipadi
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: venkatesh.pallipadi @ 2008-07-31  2:21 UTC (permalink / raw)
  To: len.brown, andi; +Cc: linux-pm



-- 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [patch 1/3] cpuidle: Do not use poll_idle unless user asks for it
  2008-07-31  2:21 [patch 0/3] cpuilde: Misc bug fixes venkatesh.pallipadi
@ 2008-07-31  2:21 ` venkatesh.pallipadi
  2008-07-31  2:21 ` [patch 2/3] cpuidle: Menu governor fix wrong usage of measured_us venkatesh.pallipadi
  2008-07-31  2:21 ` [patch 3/3] cpuidle: Make ladder governor honor latency requirements fully venkatesh.pallipadi
  2 siblings, 0 replies; 4+ messages in thread
From: venkatesh.pallipadi @ 2008-07-31  2:21 UTC (permalink / raw)
  To: len.brown, andi; +Cc: linux-pm

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

poll_idle was added to CPUIDLE, just as a low latency idle handler, to be
used in cases when user desires CPUs not to enter any idle state at all. It
was supposed to be a run time idle=poll option to the user. But, it was indeed
getting used during normal menu and ladder governor default case, with no
special user setting (Reported by Linus Torvalds).

Change below ensures that poll_idle will not be used unless user explicitly
asks pm_qos infrastructure for zero latency requirement.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

---
 drivers/cpuidle/governors/ladder.c |   14 ++++++++++----
 drivers/cpuidle/governors/menu.c   |   11 +++++++++--
 2 files changed, 19 insertions(+), 6 deletions(-)

Index: linux-2.6/drivers/cpuidle/governors/menu.c
===================================================================
--- linux-2.6.orig/drivers/cpuidle/governors/menu.c	2008-07-28 16:52:04.000000000 -0700
+++ linux-2.6/drivers/cpuidle/governors/menu.c	2008-07-30 19:00:48.000000000 -0700
@@ -34,21 +34,28 @@ static DEFINE_PER_CPU(struct menu_device
 static int menu_select(struct cpuidle_device *dev)
 {
 	struct menu_device *data = &__get_cpu_var(menu_devices);
+	int latency_req = pm_qos_requirement(PM_QOS_CPU_DMA_LATENCY);
 	int i;
 
+	/* Special case when user has set very strict latency requirement */
+	if (unlikely(latency_req == 0)) {
+		data->last_state_idx = 0;
+		return 0;
+	}
+
 	/* determine the expected residency time */
 	data->expected_us =
 		(u32) ktime_to_ns(tick_nohz_get_sleep_length()) / 1000;
 
 	/* find the deepest idle state that satisfies our constraints */
-	for (i = 1; i < dev->state_count; i++) {
+	for (i = CPUIDLE_DRIVER_STATE_START + 1; i < dev->state_count; i++) {
 		struct cpuidle_state *s = &dev->states[i];
 
 		if (s->target_residency > data->expected_us)
 			break;
 		if (s->target_residency > data->predicted_us)
 			break;
-		if (s->exit_latency > pm_qos_requirement(PM_QOS_CPU_DMA_LATENCY))
+		if (s->exit_latency > latency_req)
 			break;
 	}
 
Index: linux-2.6/drivers/cpuidle/governors/ladder.c
===================================================================
--- linux-2.6.orig/drivers/cpuidle/governors/ladder.c	2008-07-28 16:52:04.000000000 -0700
+++ linux-2.6/drivers/cpuidle/governors/ladder.c	2008-07-30 19:01:28.000000000 -0700
@@ -67,10 +67,17 @@ static int ladder_select_state(struct cp
 	struct ladder_device *ldev = &__get_cpu_var(ladder_devices);
 	struct ladder_device_state *last_state;
 	int last_residency, last_idx = ldev->last_state_idx;
+	int latency_req = pm_qos_requirement(PM_QOS_CPU_DMA_LATENCY);
 
 	if (unlikely(!ldev))
 		return 0;
 
+	/* Special case when user has set very strict latency requirement */
+	if (unlikely(latency_req == 0)) {
+		ladder_do_selection(ldev, last_idx, 0);
+		return 0;
+	}
+
 	last_state = &ldev->states[last_idx];
 
 	if (dev->states[last_idx].flags & CPUIDLE_FLAG_TIME_VALID)
@@ -81,8 +88,7 @@ static int ladder_select_state(struct cp
 	/* consider promotion */
 	if (last_idx < dev->state_count - 1 &&
 	    last_residency > last_state->threshold.promotion_time &&
-	    dev->states[last_idx + 1].exit_latency <=
-			pm_qos_requirement(PM_QOS_CPU_DMA_LATENCY)) {
+	    dev->states[last_idx + 1].exit_latency <= latency_req) {
 		last_state->stats.promotion_count++;
 		last_state->stats.demotion_count = 0;
 		if (last_state->stats.promotion_count >= last_state->threshold.promotion_count) {
@@ -92,7 +98,7 @@ static int ladder_select_state(struct cp
 	}
 
 	/* consider demotion */
-	if (last_idx > 0 &&
+	if (last_idx > CPUIDLE_DRIVER_STATE_START &&
 	    last_residency < last_state->threshold.demotion_time) {
 		last_state->stats.demotion_count++;
 		last_state->stats.promotion_count = 0;
@@ -117,7 +123,7 @@ static int ladder_enable_device(struct c
 	struct ladder_device_state *lstate;
 	struct cpuidle_state *state;
 
-	ldev->last_state_idx = 0;
+	ldev->last_state_idx = CPUIDLE_DRIVER_STATE_START;
 
 	for (i = 0; i < dev->state_count; i++) {
 		state = &dev->states[i];

-- 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [patch 2/3] cpuidle: Menu governor fix wrong usage of measured_us
  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
  2008-07-31  2:21 ` [patch 3/3] cpuidle: Make ladder governor honor latency requirements fully venkatesh.pallipadi
  2 siblings, 0 replies; 4+ messages in thread
From: venkatesh.pallipadi @ 2008-07-31  2:21 UTC (permalink / raw)
  To: len.brown, andi; +Cc: linux-pm

[-- 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;
 	}
 }
 

-- 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [patch 3/3] cpuidle: Make ladder governor honor latency requirements fully
  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 ` [patch 2/3] cpuidle: Menu governor fix wrong usage of measured_us venkatesh.pallipadi
@ 2008-07-31  2:21 ` venkatesh.pallipadi
  2 siblings, 0 replies; 4+ messages in thread
From: venkatesh.pallipadi @ 2008-07-31  2:21 UTC (permalink / raw)
  To: len.brown, andi; +Cc: linux-pm

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

ladder governor only honored latency requirement when promoting C-states.
Instead. it should check for latency requirement on each idle call,
and demote to appropriate C-state when there is a latency requirement change.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>

---
 drivers/cpuidle/governors/ladder.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

Index: linux-2.6/drivers/cpuidle/governors/ladder.c
===================================================================
--- linux-2.6.orig/drivers/cpuidle/governors/ladder.c	2008-07-28 15:49:10.000000000 -0700
+++ linux-2.6/drivers/cpuidle/governors/ladder.c	2008-07-28 15:49:21.000000000 -0700
@@ -99,6 +99,18 @@ static int ladder_select_state(struct cp
 
 	/* consider demotion */
 	if (last_idx > CPUIDLE_DRIVER_STATE_START &&
+	    dev->states[last_idx].exit_latency > latency_req) {
+		int i;
+
+		for (i = last_idx - 1; i > CPUIDLE_DRIVER_STATE_START; i--) {
+			if (dev->states[i].exit_latency <= latency_req)
+				break;
+		}
+		ladder_do_selection(ldev, last_idx, i);
+		return i;
+	}
+
+	if (last_idx > CPUIDLE_DRIVER_STATE_START &&
 	    last_residency < last_state->threshold.demotion_time) {
 		last_state->stats.demotion_count++;
 		last_state->stats.promotion_count = 0;

-- 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-07-31  2:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [patch 2/3] cpuidle: Menu governor fix wrong usage of measured_us venkatesh.pallipadi
2008-07-31  2:21 ` [patch 3/3] cpuidle: Make ladder governor honor latency requirements fully venkatesh.pallipadi

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