| Message ID | 2237950.TTgtiEjdez@kreacher (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | cpuidle: teo: Fix issues related to disabled idle states | expand |
On 2019.10.10 14:36 Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > The TEO governor uses idle duration "bins" defined in accordance with > the CPU idle states table provided by the driver, so that each "bin" > covers the idle duration range between the target residency of the > idle state corresponding to it and the target residency of the closest > deeper idle state. The governor collects statistics for each bin > regardless of whether or not the idle state corresponding to it is > currently enabled. > > In particular, the "hits" and "misses" metrics measure the likelihood > of a situation in which both the time till the next timer (sleep > length) and the idle duration measured after wakeup fall into the > given bin. Namely, if the "hits" value is greater than the "misses" > one, that situation is more likely than the one in which the sleep > length falls into the given bin, but the idle duration measured after > wakeup falls into a bin corresponding to one of the shallower idle > states. > > If the idle state corresponding to the given bin is disabled, it > cannot be selected and if it turns out to be the one that should be > selected, a shallower idle state needs to be used instead of it. > Nevertheless, the metrics collected for the bin corresponding to it > are still valid and need to be taken into account as though that > state had not been disabled. > > For this reason, make teo_select() always use the "hits" and "misses" > values of the idle duration range that the sleep length falls into > even if the specific idle state corresponding to it is disabled and > if the "hits" values is greater than the "misses" one, select the > closest enabled shallower idle state in that case. > > Fixes: b26bf6ab716f ("cpuidle: New timer events oriented governor for tickless systems") > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Great thanks. Acked-by: Doug Smythies <dsmythies@telus.net>
Index: linux-pm/drivers/cpuidle/governors/teo.c =================================================================== --- linux-pm.orig/drivers/cpuidle/governors/teo.c +++ linux-pm/drivers/cpuidle/governors/teo.c @@ -233,7 +233,7 @@ static int teo_select(struct cpuidle_dri { struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu); int latency_req = cpuidle_governor_latency_req(dev->cpu); - unsigned int duration_us, early_hits; + unsigned int duration_us, hits, misses, early_hits; int max_early_idx, constraint_idx, idx, i; ktime_t delta_tick; @@ -247,6 +247,8 @@ static int teo_select(struct cpuidle_dri cpu_data->sleep_length_ns = tick_nohz_get_sleep_length(&delta_tick); duration_us = ktime_to_us(cpu_data->sleep_length_ns); + hits = 0; + misses = 0; early_hits = 0; max_early_idx = -1; constraint_idx = drv->state_count; @@ -265,6 +267,17 @@ static int teo_select(struct cpuidle_dri continue; /* + * This state is disabled, so the range of idle duration + * values corresponding to it is covered by the current + * candidate state, but still the "hits" and "misses" + * metrics of the disabled state need to be used to + * decide whether or not the state covering the range in + * question is good enough. + */ + hits = cpu_data->states[i].hits; + misses = cpu_data->states[i].misses; + + /* * If the "early hits" metric of a disabled state is * greater than the current maximum, it should be taken * into account, because it would be a mistake to select @@ -280,8 +293,11 @@ static int teo_select(struct cpuidle_dri continue; } - if (idx < 0) + if (idx < 0) { idx = i; /* first enabled state */ + hits = cpu_data->states[i].hits; + misses = cpu_data->states[i].misses; + } if (s->target_residency > duration_us) break; @@ -290,6 +306,8 @@ static int teo_select(struct cpuidle_dri constraint_idx = i; idx = i; + hits = cpu_data->states[i].hits; + misses = cpu_data->states[i].misses; if (early_hits < cpu_data->states[i].early_hits && !(tick_nohz_tick_stopped() && @@ -307,8 +325,7 @@ static int teo_select(struct cpuidle_dri * "early hits" metric, but if that cannot be determined, just use the * state selected so far. */ - if (cpu_data->states[idx].hits <= cpu_data->states[idx].misses && - max_early_idx >= 0) { + if (hits <= misses && max_early_idx >= 0) { idx = max_early_idx; duration_us = drv->states[idx].target_residency; }