diff options
author | venkatesh.pallipadi@intel.com <venkatesh.pallipadi@intel.com> | 2008-07-30 22:21:43 -0400 |
---|---|---|
committer | Andi Kleen <ak@linux.intel.com> | 2008-08-15 15:25:25 -0400 |
commit | 320eee776357db52d6fcfb11cff985b1976a4595 (patch) | |
tree | 29859b2e6209a35fc6a735fa4c195e7daeaa1b31 /drivers/cpuidle | |
parent | a2bd92023357e47f22a34d4cb1635453546662bc (diff) |
cpuidle: Menu governor fix wrong usage of measured_us
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>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Diffstat (limited to 'drivers/cpuidle')
-rw-r--r-- | drivers/cpuidle/governors/menu.c | 31 |
1 files changed, 19 insertions, 12 deletions
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index b8f3e21530bd..8d7cf3f31450 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c | |||
@@ -74,9 +74,9 @@ static void menu_reflect(struct cpuidle_device *dev) | |||
74 | { | 74 | { |
75 | struct menu_device *data = &__get_cpu_var(menu_devices); | 75 | struct menu_device *data = &__get_cpu_var(menu_devices); |
76 | int last_idx = data->last_state_idx; | 76 | int last_idx = data->last_state_idx; |
77 | unsigned int measured_us = | 77 | unsigned int last_idle_us = cpuidle_get_last_residency(dev); |
78 | cpuidle_get_last_residency(dev) + data->elapsed_us; | ||
79 | struct cpuidle_state *target = &dev->states[last_idx]; | 78 | struct cpuidle_state *target = &dev->states[last_idx]; |
79 | unsigned int measured_us; | ||
80 | 80 | ||
81 | /* | 81 | /* |
82 | * Ugh, this idle state doesn't support residency measurements, so we | 82 | * Ugh, this idle state doesn't support residency measurements, so we |
@@ -84,20 +84,27 @@ static void menu_reflect(struct cpuidle_device *dev) | |||
84 | * for one full standard timer tick. However, be aware that this | 84 | * for one full standard timer tick. However, be aware that this |
85 | * could potentially result in a suboptimal state transition. | 85 | * could potentially result in a suboptimal state transition. |
86 | */ | 86 | */ |
87 | if (!(target->flags & CPUIDLE_FLAG_TIME_VALID)) | 87 | if (unlikely(!(target->flags & CPUIDLE_FLAG_TIME_VALID))) |
88 | measured_us = USEC_PER_SEC / HZ; | 88 | last_idle_us = USEC_PER_SEC / HZ; |
89 | 89 | ||
90 | /* Predict time remaining until next break event */ | 90 | /* |
91 | if (measured_us + BREAK_FUZZ < data->expected_us - target->exit_latency) { | 91 | * measured_us and elapsed_us are the cumulative idle time, since the |
92 | data->predicted_us = max(measured_us, data->last_measured_us); | 92 | * last time we were woken out of idle by an interrupt. |
93 | */ | ||
94 | if (data->elapsed_us <= data->elapsed_us + last_idle_us) | ||
95 | measured_us = data->elapsed_us + last_idle_us; | ||
96 | else | ||
97 | measured_us = -1; | ||
98 | |||
99 | /* Predict time until next break event */ | ||
100 | data->predicted_us = max(measured_us, data->last_measured_us); | ||
101 | |||
102 | if (last_idle_us + BREAK_FUZZ < | ||
103 | data->expected_us - target->exit_latency) { | ||
93 | data->last_measured_us = measured_us; | 104 | data->last_measured_us = measured_us; |
94 | data->elapsed_us = 0; | 105 | data->elapsed_us = 0; |
95 | } else { | 106 | } else { |
96 | if (data->elapsed_us < data->elapsed_us + measured_us) | 107 | data->elapsed_us = measured_us; |
97 | data->elapsed_us = measured_us; | ||
98 | else | ||
99 | data->elapsed_us = -1; | ||
100 | data->predicted_us = max(measured_us, data->last_measured_us); | ||
101 | } | 108 | } |
102 | } | 109 | } |
103 | 110 | ||