| Message ID | 20160316121400.680a6a46@annuminas.surriel.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
On Wednesday, March 16, 2016 12:14:00 PM Rik van Riel wrote: > The menu governor uses five different factors to pick the > idle state: > - the user configured latency_req > - the time until the next timer (next_timer_us) > - the typical sleep interval, as measured recently > - an estimate of sleep time by dividing next_timer_us by an observed factor > - a load corrected version of the above, divided again by load > > Only the first three items are known with enough confidence that > we can use them to consider polling, instead of an actual CPU > idle state, because the cost of being wrong about polling can be > excessive power use. > > The latter two are used in the menu governor's main selection > loop, and can result in choosing a shallower idle state when > the system is expected to be busy again soon. > > This pushes a busy system in the "performance" direction of > the performance<>power tradeoff, when choosing between idle > states, but stays more strictly on the "power" state when > deciding between polling and C1. > > Signed-off-by: Rik van Riel <riel@redhat.com> Applied, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Sorry for the delay in my reply / test. The patch e-mail went to my junk folder for some reason. On 2106.03.17 17:46 Rafael J. Wysocki wrote: > On Wednesday, March 16, 2016 12:14:00 PM Rik van Riel wrote: >> The menu governor uses five different factors to pick the >> idle state: >> - the user configured latency_req >> - the time until the next timer (next_timer_us) >> - the typical sleep interval, as measured recently >> - an estimate of sleep time by dividing next_timer_us by an observed factor >> - a load corrected version of the above, divided again by load >> >> Only the first three items are known with enough confidence that >> we can use them to consider polling, instead of an actual CPU >> idle state, because the cost of being wrong about polling can be >> excessive power use. >> >> The latter two are used in the menu governor's main selection >> loop, and can result in choosing a shallower idle state when >> the system is expected to be busy again soon. >> >> This pushes a busy system in the "performance" direction of >> the performance<>power tradeoff, when choosing between idle >> states, but stays more strictly on the "power" state when >> deciding between polling and C1. >> >> Signed-off-by: Rik van Riel <riel@redhat.com> For my part of it, this patch seems to be not O.K. (reference rvr5 = this patch) Aggregate idle for the 2000 second test. All in minutes. (old tests re-stated) State k45rc7-rjw10 k45rc7-rjw10-reverted k45rc7-rjw10-rvr5 0.00 18.07 0.92 18.67 1.00 12.35 19.51 12.82 2.00 3.96 4.28 3.97 3.00 1.55 1.53 1.58 4.00 138.96 141.99 143.80 total 174.90 168.24 180.84 Energy: >> Kernel 4.5-rc7-rjw10: 61983 Joules >> Kernel 4.5-rc7-rjw10-reverted: 48409 Joules (test 2 was 55040 Joules) Kernel 4.5-rc7-rjw10-rvr5: 62243 Joules I did acquire trace data with this test, but haven't post processed it yet. ... Doug -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 18, 2016 at 7:32 AM, Doug Smythies <dsmythies@telus.net> wrote: > Sorry for the delay in my reply / test. The patch e-mail went > to my junk folder for some reason. > > On 2106.03.17 17:46 Rafael J. Wysocki wrote: >> On Wednesday, March 16, 2016 12:14:00 PM Rik van Riel wrote: >>> The menu governor uses five different factors to pick the >>> idle state: >>> - the user configured latency_req >>> - the time until the next timer (next_timer_us) >>> - the typical sleep interval, as measured recently >>> - an estimate of sleep time by dividing next_timer_us by an observed factor >>> - a load corrected version of the above, divided again by load >>> >>> Only the first three items are known with enough confidence that >>> we can use them to consider polling, instead of an actual CPU >>> idle state, because the cost of being wrong about polling can be >>> excessive power use. >>> >>> The latter two are used in the menu governor's main selection >>> loop, and can result in choosing a shallower idle state when >>> the system is expected to be busy again soon. >>> >>> This pushes a busy system in the "performance" direction of >>> the performance<>power tradeoff, when choosing between idle >>> states, but stays more strictly on the "power" state when >>> deciding between polling and C1. >>> >>> Signed-off-by: Rik van Riel <riel@redhat.com> > > For my part of it, this patch seems to be not O.K. > (reference rvr5 = this patch) > > Aggregate idle for the 2000 second test. All in minutes. > (old tests re-stated) > > State k45rc7-rjw10 k45rc7-rjw10-reverted k45rc7-rjw10-rvr5 > 0.00 18.07 0.92 18.67 > 1.00 12.35 19.51 12.82 > 2.00 3.96 4.28 3.97 > 3.00 1.55 1.53 1.58 > 4.00 138.96 141.99 143.80 > > total 174.90 168.24 180.84 > > Energy: >>> Kernel 4.5-rc7-rjw10: 61983 Joules >>> Kernel 4.5-rc7-rjw10-reverted: 48409 Joules (test 2 was 55040 Joules) > Kernel 4.5-rc7-rjw10-rvr5: 62243 Joules > > I did acquire trace data with this test, but haven't post processed it yet. I'm wondering what happens if you replace the expected_interval in the "expected_interval > drv->states[CPUIDLE_DRIVER_STATE_START].target_residency" test with data->next_timer_us (with the Rik's patch applied, of course). Can you please try doing that? -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2016.03.18 06:12 Rafael J. Wysocki wrote: > On Fri, Mar 18, 2016 at 7:32 AM, Doug Smythies <dsmythies@telus.net> wrote: > >> For my part of it, this patch seems to be not O.K. >> (reference rvr5 = this patch) >> >> Aggregate idle for the 2000 second test. All in minutes. >> (old tests re-stated) >> >> State k45rc7-rjw10 k45rc7-rjw10-reverted k45rc7-rjw10-rvr5 >> 0.00 18.07 0.92 18.67 >> 1.00 12.35 19.51 12.82 >> 2.00 3.96 4.28 3.97 >> 3.00 1.55 1.53 1.58 >> 4.00 138.96 141.99 143.80 >> >> total 174.90 168.24 180.84 >> >> Energy: >>>> Kernel 4.5-rc7-rjw10: 61983 Joules >>>> Kernel 4.5-rc7-rjw10-reverted: 48409 Joules (test 2 was 55040 Joules) >> Kernel 4.5-rc7-rjw10-rvr5: 62243 Joules >> >> I did acquire trace data with this test, but haven't post processed it yet. > > I'm wondering what happens if you replace the expected_interval in the > "expected_interval > > drv->states[CPUIDLE_DRIVER_STATE_START].target_residency" test with > data->next_timer_us (with the Rik's patch applied, of course). Can > you please try doing that? O.K. my reference: rvr6 is the above modification to rvr5 It works as well as "reverted"/ State k45rc7-rjw10-rvr6 (mins) 0.00 0.87 1.00 24.20 2.00 4.05 3.00 1.72 4.00 147.50 total 178.34 Energy: Kernel 4.5-rc7-rjw10-rvr6: 55864 Joules Trace data (very crude summary): Kernel 4.5-rc7-rjw10-rvr5: ~3049 long durations at high CPU load (idle state 0) Kernel 4.5-rc7-rjw10-rvr5: ~183 long durations at high, but less, CPU load (not all idle state 0) ... Doug -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 0742b3296673..fba867d917f7 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -196,7 +196,7 @@ static void menu_update(struct cpuidle_driver *drv, struct cpuidle_device *dev); * of points is below a threshold. If it is... then use the * average of these 8 points as the estimated value. */ -static void get_typical_interval(struct menu_device *data) +static unsigned int get_typical_interval(struct menu_device *data) { int i, divisor; unsigned int max, thresh; @@ -254,9 +254,7 @@ static void get_typical_interval(struct menu_device *data) stddev = int_sqrt(stddev); if (((avg > stddev * 6) && (divisor * 4 >= INTERVALS * 3)) || stddev <= 20) { - if (data->next_timer_us > avg) - data->predicted_us = avg; - return; + return avg; } } @@ -270,7 +268,7 @@ static void get_typical_interval(struct menu_device *data) * with sporadic activity with a bunch of short pauses. */ if ((divisor * 4) <= INTERVALS * 3) - return; + return UINT_MAX; thresh = max - 1; goto again; @@ -287,6 +285,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) int latency_req = pm_qos_request(PM_QOS_CPU_DMA_LATENCY); int i; unsigned int interactivity_req; + unsigned int expected_interval; unsigned long nr_iowaiters, cpu_load; if (data->needs_update) { @@ -313,32 +312,39 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev) data->correction_factor[data->bucket], RESOLUTION * DECAY); - get_typical_interval(data); - - /* - * Performance multiplier defines a minimum predicted idle - * duration / latency ratio. Adjust the latency limit if - * necessary. - */ - interactivity_req = data->predicted_us / performance_multiplier(nr_iowaiters, cpu_load); - if (latency_req > interactivity_req) - latency_req = interactivity_req; + expected_interval = get_typical_interval(data); + expected_interval = min(expected_interval, data->next_timer_us); if (CPUIDLE_DRIVER_STATE_START > 0) { data->last_state_idx = CPUIDLE_DRIVER_STATE_START - 1; /* * We want to default to C1 (hlt), not to busy polling - * unless the timer is happening really really soon. + * unless the timer is happening really really soon, or + * C1's exit latency exceeds the user configured limit. */ - if (interactivity_req > 20 && + if (expected_interval > drv->states[CPUIDLE_DRIVER_STATE_START].target_residency && + latency_req > drv->states[CPUIDLE_DRIVER_STATE_START].exit_latency && !drv->states[CPUIDLE_DRIVER_STATE_START].disabled && - dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable == 0) + !dev->states_usage[CPUIDLE_DRIVER_STATE_START].disable) data->last_state_idx = CPUIDLE_DRIVER_STATE_START; } else { data->last_state_idx = CPUIDLE_DRIVER_STATE_START; } /* + * Use the lowest expected idle interval to pick the idle state. + */ + data->predicted_us = min(data->predicted_us, expected_interval); + + /* + * Use the performance multiplier and the user-configurable + * latency_req to determine the maximum exit latency. + */ + interactivity_req = data->predicted_us / performance_multiplier(nr_iowaiters, cpu_load); + if (latency_req > interactivity_req) + latency_req = interactivity_req; + + /* * Find the idle state with the lowest power while satisfying * our constraints. */
The menu governor uses five different factors to pick the idle state: - the user configured latency_req - the time until the next timer (next_timer_us) - the typical sleep interval, as measured recently - an estimate of sleep time by dividing next_timer_us by an observed factor - a load corrected version of the above, divided again by load Only the first three items are known with enough confidence that we can use them to consider polling, instead of an actual CPU idle state, because the cost of being wrong about polling can be excessive power use. The latter two are used in the menu governor's main selection loop, and can result in choosing a shallower idle state when the system is expected to be busy again soon. This pushes a busy system in the "performance" direction of the performance<>power tradeoff, when choosing between idle states, but stays more strictly on the "power" state when deciding between polling and C1. Signed-off-by: Rik van Riel <riel@redhat.com> --- drivers/cpuidle/governors/menu.c | 42 +++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 18 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html