Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

Commit 0a2998f

Browse files
cloehlerafaeljw
authored andcommitted
Revert: "cpuidle: teo: Introduce util-awareness"
This reverts commit 9ce0f7c. Util-awareness was reported to be too aggressive in selecting shallower states. Additionally a single threshold was found to not be suitable for reasoning about sleep length as, for all practical purposes, almost arbitrary sleep lengths are still possible for any load value. Fixes: 9ce0f7c ("cpuidle: teo: Introduce util-awareness") Link: https://patch.msgid.link/20240628095955.34096-2-christian.loehle@arm.com Reported-by: Qais Yousef <qyousef@layalina.io> Reported-by: Vincent Guittot <vincent.guittot@linaro.org> Reviewed-by: Qais Yousef <qyousef@layalina.io> Tested-by: Vincent Guittot <vincent.guittot@linaro.org> Signed-off-by: Christian Loehle <christian.loehle@arm.com> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
1 parent 63e6b02 commit 0a2998f

File tree

1 file changed

+0
-105
lines changed
  • drivers/cpuidle/governors

1 file changed

+0
-105
lines changed

drivers/cpuidle/governors/teo.c

Lines changed: 0 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,8 @@
22
/*
33
* Timer events oriented CPU idle governor
44
*
5-
* TEO governor:
65
* Copyright (C) 2018 - 2021 Intel Corporation
76
* Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
8-
*
9-
* Util-awareness mechanism:
10-
* Copyright (C) 2022 Arm Ltd.
11-
* Author: Kajetan Puchalski <kajetan.puchalski@arm.com>
127
*/
138

149
/**
@@ -104,56 +99,16 @@
10499
* select the given idle state instead of the candidate one.
105100
*
106101
* 3. By default, select the candidate state.
107-
*
108-
* Util-awareness mechanism:
109-
*
110-
* The idea behind the util-awareness extension is that there are two distinct
111-
* scenarios for the CPU which should result in two different approaches to idle
112-
* state selection - utilized and not utilized.
113-
*
114-
* In this case, 'utilized' means that the average runqueue util of the CPU is
115-
* above a certain threshold.
116-
*
117-
* When the CPU is utilized while going into idle, more likely than not it will
118-
* be woken up to do more work soon and so a shallower idle state should be
119-
* selected to minimise latency and maximise performance. When the CPU is not
120-
* being utilized, the usual metrics-based approach to selecting the deepest
121-
* available idle state should be preferred to take advantage of the power
122-
* saving.
123-
*
124-
* In order to achieve this, the governor uses a utilization threshold.
125-
* The threshold is computed per-CPU as a percentage of the CPU's capacity
126-
* by bit shifting the capacity value. Based on testing, the shift of 6 (~1.56%)
127-
* seems to be getting the best results.
128-
*
129-
* Before selecting the next idle state, the governor compares the current CPU
130-
* util to the precomputed util threshold. If it's below, it defaults to the
131-
* TEO metrics mechanism. If it's above, the closest shallower idle state will
132-
* be selected instead, as long as is not a polling state.
133102
*/
134103

135104
#include <linux/cpuidle.h>
136105
#include <linux/jiffies.h>
137106
#include <linux/kernel.h>
138-
#include <linux/sched.h>
139107
#include <linux/sched/clock.h>
140-
#include <linux/sched/topology.h>
141108
#include <linux/tick.h>
142109

143110
#include "gov.h"
144111

145-
/*
146-
* The number of bits to shift the CPU's capacity by in order to determine
147-
* the utilized threshold.
148-
*
149-
* 6 was chosen based on testing as the number that achieved the best balance
150-
* of power and performance on average.
151-
*
152-
* The resulting threshold is high enough to not be triggered by background
153-
* noise and low enough to react quickly when activity starts to ramp up.
154-
*/
155-
#define UTIL_THRESHOLD_SHIFT 6
156-
157112
/*
158113
* The PULSE value is added to metrics when they grow and the DECAY_SHIFT value
159114
* is used for decreasing metrics on a regular basis.
@@ -188,7 +143,6 @@ struct teo_bin {
188143
* @next_recent_idx: Index of the next @recent_idx entry to update.
189144
* @recent_idx: Indices of bins corresponding to recent "intercepts".
190145
* @tick_hits: Number of "hits" after TICK_NSEC.
191-
* @util_threshold: Threshold above which the CPU is considered utilized
192146
*/
193147
struct teo_cpu {
194148
s64 time_span_ns;
@@ -198,28 +152,10 @@ struct teo_cpu {
198152
int next_recent_idx;
199153
int recent_idx[NR_RECENT];
200154
unsigned int tick_hits;
201-
unsigned long util_threshold;
202155
};
203156

204157
static DEFINE_PER_CPU(struct teo_cpu, teo_cpus);
205158

206-
/**
207-
* teo_cpu_is_utilized - Check if the CPU's util is above the threshold
208-
* @cpu: Target CPU
209-
* @cpu_data: Governor CPU data for the target CPU
210-
*/
211-
#ifdef CONFIG_SMP
212-
static bool teo_cpu_is_utilized(int cpu, struct teo_cpu *cpu_data)
213-
{
214-
return sched_cpu_util(cpu) > cpu_data->util_threshold;
215-
}
216-
#else
217-
static bool teo_cpu_is_utilized(int cpu, struct teo_cpu *cpu_data)
218-
{
219-
return false;
220-
}
221-
#endif
222-
223159
/**
224160
* teo_update - Update CPU metrics after wakeup.
225161
* @drv: cpuidle driver containing state data.
@@ -386,7 +322,6 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
386322
int constraint_idx = 0;
387323
int idx0 = 0, idx = -1;
388324
bool alt_intercepts, alt_recent;
389-
bool cpu_utilized;
390325
s64 duration_ns;
391326
int i;
392327

@@ -411,32 +346,6 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
411346
if (!dev->states_usage[0].disable)
412347
idx = 0;
413348

414-
cpu_utilized = teo_cpu_is_utilized(dev->cpu, cpu_data);
415-
/*
416-
* If the CPU is being utilized over the threshold and there are only 2
417-
* states to choose from, the metrics need not be considered, so choose
418-
* the shallowest non-polling state and exit.
419-
*/
420-
if (drv->state_count < 3 && cpu_utilized) {
421-
/*
422-
* If state 0 is enabled and it is not a polling one, select it
423-
* right away unless the scheduler tick has been stopped, in
424-
* which case care needs to be taken to leave the CPU in a deep
425-
* enough state in case it is not woken up any time soon after
426-
* all. If state 1 is disabled, though, state 0 must be used
427-
* anyway.
428-
*/
429-
if ((!idx && !(drv->states[0].flags & CPUIDLE_FLAG_POLLING) &&
430-
teo_state_ok(0, drv)) || dev->states_usage[1].disable) {
431-
idx = 0;
432-
goto out_tick;
433-
}
434-
/* Assume that state 1 is not a polling one and use it. */
435-
idx = 1;
436-
duration_ns = drv->states[1].target_residency_ns;
437-
goto end;
438-
}
439-
440349
/* Compute the sums of metrics for early wakeup pattern detection. */
441350
for (i = 1; i < drv->state_count; i++) {
442351
struct teo_bin *prev_bin = &cpu_data->state_bins[i-1];
@@ -560,18 +469,6 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
560469
if (idx > constraint_idx)
561470
idx = constraint_idx;
562471

563-
/*
564-
* If the CPU is being utilized over the threshold, choose a shallower
565-
* non-polling state to improve latency, unless the scheduler tick has
566-
* been stopped already and the shallower state's target residency is
567-
* not sufficiently large.
568-
*/
569-
if (cpu_utilized) {
570-
i = teo_find_shallower_state(drv, dev, idx, KTIME_MAX, true);
571-
if (teo_state_ok(i, drv))
572-
idx = i;
573-
}
574-
575472
/*
576473
* Skip the timers check if state 0 is the current candidate one,
577474
* because an immediate non-timer wakeup is expected in that case.
@@ -667,11 +564,9 @@ static int teo_enable_device(struct cpuidle_driver *drv,
667564
struct cpuidle_device *dev)
668565
{
669566
struct teo_cpu *cpu_data = per_cpu_ptr(&teo_cpus, dev->cpu);
670-
unsigned long max_capacity = arch_scale_cpu_capacity(dev->cpu);
671567
int i;
672568

673569
memset(cpu_data, 0, sizeof(*cpu_data));
674-
cpu_data->util_threshold = max_capacity >> UTIL_THRESHOLD_SHIFT;
675570

676571
for (i = 0; i < NR_RECENT; i++)
677572
cpu_data->recent_idx[i] = -1;

0 commit comments

Comments
 (0)