cpuidle: cpu_idle_miss accounting fixes
Subsystems (2)
Christian Loehle
reviewerv2 cover cpuidle: cpu_idle_miss accounting fixes
These two fixes exclude timer-broadcast fallbacks from governor miss accounting and respect the latency constraint used for selection when looking for a better deeper state.
No functional change outside of sysfs above/below and tracing.
Patch 1: Exclude timer-broadcast fallbacks
When tick_broadcast_enter() fails, the core forces a shallower state. Exclude these entries from governor miss accounting while retaining residency, usage and governor feedback.
On Orion O6, traces with menu and TEO running schbench, tbench and hackbench recorded 6930 cpu_idle_miss events on 8172 broadcast fallbacks before the fix, and none on 7763 fallbacks with patch 1. All matched fallbacks entered WFI; counts cover complete idle episodes within workload markers.
Patch 2: Respect latency constraints for below misses
An enabled deeper state may violate the latency QoS constraint, causing a false below miss. Sample the latency limit before governor selection and share it with the governors and miss accounting. Exclude states exceeding it, without an additional state-table walk. Disable flags continue to be checked at exit.
Changes since v1:
- Add observed broadcast fallback statistics to patch 1. (Rafael)
- Sample QoS once before governor selection and reuse the same limit for miss accounting. (Rafael)
I know the broader refactor in v2 makes it less appealling to backport these to stable, but it massively improves the value of the cpu_idle_miss trace event, when I have to look at some older stable-based Android traces, where latency requirements may be set dynamically at runtime.
Christian Loehle (2):
cpuidle: Skip miss accounting for timer broadcast fallbacks
cpuidle: Respect latency constraints when accounting below misses
Documentation/driver-api/pm/cpuidle.rst | 13 ++++++------- drivers/cpuidle/cpuidle.c | 18 +++++++++++++++++- drivers/cpuidle/cpuidle.h | 1 + drivers/cpuidle/governors/haltpoll.c | 2 +- drivers/cpuidle/governors/ladder.c | 2 +- drivers/cpuidle/governors/menu.c | 2 +- drivers/cpuidle/governors/teo.c | 2 +- include/linux/cpuidle.h | 2 +- 8 files changed, 29 insertions(+), 13 deletions(-)
base-commit: 067c5c24cef23d913b67eaaf71ae67e7169a6620
Signature · 1 line
2.34.1
Christian Loehle
reviewerPATCH v2 1/2 cpuidle: Skip miss accounting for timer broadcast fallbacks
A failed tick_broadcast_enter() can force a shallower state than the governor selected. Exclude successful broadcast fallbacks from above/below miss accounting while retaining their usage, residency and governor feedback.
On Orion O6 with menu and TEO under normal workloads, 9,782 of 33,136 broadcast fallbacks emitted below misses: 90.4% of all below misses and 13.6% of the 72,059 total misses. Excluding these from the same 795,370 complete idle episodes reduces the miss ratio from 9.06% to 7.83%.
--- drivers/cpuidle/cpuidle.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/cpuidle/cpuidle.c +10 -0
@@ -222,6 +222,7 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,struct cpuidle_state *target_state = &drv->states[index];bool broadcast = !!(target_state->flags &CPUIDLE_FLAG_TIMER_STOP ); +bool broadcast_failed = false;ktime_t time_start, time_end;instrumentation_begin ();
@@ -232,6 +233,7 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,* CPU as a broadcast timer, this call may fail if it is not available. */ if (broadcast &&tick_broadcast_enter ()) { + broadcast_failed = true; index =find_deepest_state (drv, dev, target_state->exit_latency_ns,CPUIDLE_FLAG_TIMER_STOP , false);
@@ -306,6 +308,13 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev, dev->states_usage[entered_state].time_ns += diff; dev->states_usage[entered_state].usage++; +/* +* The broadcast constraint forced a shallower state, do not +* account for it as a cpu_idle_miss. +*/ +if (broadcast_failed) + goto out; +if (diff < drv->states[entered_state].target_residency_ns) {for (i = entered_state -1 ; i >=0 ; i--) {if (dev->states_usage[i].disable)
@@ -338,6 +347,7 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev, dev->states_usage[index].rejected++; } +out:instrumentation_end ();return entered_state;
Signature · 1 line
2.34.1
Christian Loehle
reviewerPATCH v2 2/2 cpuidle: Respect latency constraints when accounting below misses
In reply to Christian LoehleOnly count a below miss when an enabled deeper state meets the latency constraint.
Sample the limit once in cpuidle_select() and share it between governors and accounting since a separate read before entry can already differ from the value used for selection.
Continue checking disable flags on exit only, as they're not expected to change as frequently.
--- Documentation/driver-api/pm/cpuidle.rst | 13 ++++++------- drivers/cpuidle/cpuidle.c | 8 +++++++- drivers/cpuidle/cpuidle.h | 1 + drivers/cpuidle/governors/haltpoll.c | 2 +- drivers/cpuidle/governors/ladder.c | 2 +- drivers/cpuidle/governors/menu.c | 2 +- drivers/cpuidle/governors/teo.c | 2 +- include/linux/cpuidle.h | 2 +- 8 files changed, 19 insertions(+), 13 deletions(-)
Documentation/driver-api/pm/cpuidle.rst +6 -7
@@ -146,13 +146,12 @@ callbacks: In addition, ``CPUIdle`` governors are required to take power management quality of service (PM QoS) constraints on the processor wakeup latency into -account when selecting idle states. In order to obtain the current effective -PM QoS wakeup latency constraint for a given CPU, a ``CPUIdle`` governor is -expected to pass the number of the CPU to -:c:func:`cpuidle_governor_latency_req()`. Then, the governor's ``->select()`` -callback must not return the index of an indle state whose -:c:member:`exit_latency` value is greater than the number returned by that -function. +account when selecting idle states. Before invoking the governor's +``->select()`` callback, the core saves the effective PM QoS wakeup latency +constraint in ``dev->latency_req``, in nanoseconds. Governors use this value +to constrain the :c:member:`exit_latency_ns` of the selected idle state. +The core also uses the saved value for below miss accounting, so subsequent +PM QoS updates do not change the constraint used to evaluate that selection. CPU Idle Time Management Drivers
drivers/cpuidle/cpuidle.c +7 -1
@@ -131,12 +131,15 @@ void cpuidle_use_deepest_state(u64 latency_limit_ns)* @dev: cpuidle device for the given CPU. * @latency_limit_ns: Idle state exit latency limit * +* Save the limit for miss accounting on the subsequent idle entry. +* * Return: the index of the deepest available idle state. */ int cpuidle_find_deepest_state (struct cpuidle_driver *drv,struct cpuidle_device *dev,u64 latency_limit_ns) { + dev->latency_req =min_t (u64, latency_limit_ns,S64_MAX );return find_deepest_state (drv, dev, latency_limit_ns,0 , false); }
@@ -327,7 +330,8 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev, } }else if (diff > delay) {for (i = entered_state +1 ; i < drv->state_count; i++) { -if (dev->states_usage[i].disable) +if (dev->states_usage[i].disable || + drv->states[i].exit_latency_ns > dev->latency_req)continue ;/*
@@ -369,6 +373,8 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,int cpuidle_select (struct cpuidle_driver *drv,struct cpuidle_device *dev,bool *stop_tick) { +/* Use the same constraint for state selection and miss accounting. */ + dev->latency_req =cpuidle_governor_latency_req (dev->cpu);return cpuidle_curr_governor->select (drv, dev, stop_tick); }
drivers/cpuidle/cpuidle.h +1 -0
@@ -23,6 +23,7 @@ extern void cpuidle_install_idle_handler(void);extern void cpuidle_uninstall_idle_handler (void );/* governors */ +s64 cpuidle_governor_latency_req (unsigned int cpu);extern struct cpuidle_governor *cpuidle_find_governor (const char *str);extern int cpuidle_switch_governor (struct cpuidle_governor *gov);
drivers/cpuidle/governors/haltpoll.c +1 -1
@@ -50,7 +50,7 @@ static int haltpoll_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, bool *stop_tick) { - if (cpuidle_governor_latency_req(dev->cpu) == 0) { +if (dev->latency_req ==0 ) { *stop_tick = false;return 0 ; }
drivers/cpuidle/governors/ladder.c +1 -1
@@ -71,7 +71,7 @@ static int ladder_select_state(struct cpuidle_driver *drv,struct ladder_device_state *last_state;int last_idx = dev->last_state_idx;int first_idx = drv->states[0 ].flags &CPUIDLE_FLAG_POLLING ?1 :0 ; -s64 latency_req =cpuidle_governor_latency_req (dev->cpu); +s64 latency_req = dev->latency_req;s64 last_residency;/* Special case when user has set very strict latency requirement */
drivers/cpuidle/governors/menu.c +1 -1
@@ -219,7 +219,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, bool *stop_tick) {struct menu_device *data =this_cpu_ptr (&menu_devices); -s64 latency_req =cpuidle_governor_latency_req (dev->cpu); +s64 latency_req = dev->latency_req;u64 predicted_ns;ktime_t delta, delta_tick;int i, idx;
drivers/cpuidle/governors/teo.c +1 -1
@@ -309,7 +309,7 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, bool *stop_tick) {struct teo_cpu *cpu_data =this_cpu_ptr (&teo_cpus); -s64 latency_req =cpuidle_governor_latency_req (dev->cpu); +s64 latency_req = dev->latency_req;ktime_t delta_tick =TICK_NSEC /2 ;unsigned int idx_intercept_sum =0 ;unsigned int intercept_sum =0 ;
include/linux/cpuidle.h +1 -1
@@ -101,6 +101,7 @@ struct cpuidle_device {u64 last_residency_ns;u64 poll_limit_ns;u64 forced_idle_latency_limit_ns; +s64 latency_req;struct cpuidle_state_usage states_usage[CPUIDLE_STATE_MAX ];struct cpuidle_state_kobj *kobjs[CPUIDLE_STATE_MAX ];struct cpuidle_driver_kobj *kobj_driver;
@@ -306,7 +307,6 @@ struct cpuidle_governor { };extern int cpuidle_register_governor (struct cpuidle_governor *gov); -extern s64 cpuidle_governor_latency_req (unsigned int cpu); #define __CPU_PM_CPU_IDLE_ENTER(low_level_idle_enter, \ idx, \
Signature · 1 line
2.34.1