Skip to content

Commit 406cbca

Browse files
Juri Lellipundiramit
authored andcommitted
UPSTREAM: cpufreq: schedutil: use now as reference when aggregating shared policy requests
Currently, sugov_next_freq_shared() uses last_freq_update_time as a reference to decide when to start considering CPU contributions as stale. However, since last_freq_update_time is set by the last CPU that issued a frequency transition, this might cause problems in certain cases. In practice, the detection of stale utilization values fails whenever the CPU with such values was the last to update the policy. For example (and please note again that the SCHED_CPUFREQ_RT flag is not the problem here, but only the detection of after how much time that flag has to be considered stale), suppose a policy with 2 CPUs: CPU0 | CPU1 | | RT task scheduled | SCHED_CPUFREQ_RT is set | CPU1->last_update = now | freq transition to max | last_freq_update_time = now | more than TICK_NSEC nsecs | a small CFS wakes up | CPU0->last_update = now1 | delta_ns(CPU0) < TICK_NSEC* | CPU0's util is considered | delta_ns(CPU1) = | last_freq_update_time - | CPU1->last_update = 0 | < TICK_NSEC | CPU1 is still considered | CPU1->SCHED_CPUFREQ_RT is set | we stay at max (until CPU1 | exits from idle) | * delta_ns is actually negative as now1 > last_freq_update_time While last_freq_update_time is a sensible reference for rate limiting, it doesn't seem to be useful for working around stale CPU states. Fix the problem by always considering now (time) as the reference for deciding when CPUs have stale contributions. Signed-off-by: Juri Lelli <juri.lelli@arm.com> Acked-by: Vincent Guittot <vincent.guittot@linaro.org> Acked-by: Viresh Kumar <viresh.kumar@linaro.org> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> (cherry picked from commit d86ab9cff8b936aadde444d0e263a8db5ff0349b)
1 parent fac6ab8 commit 406cbca

1 file changed

Lines changed: 3 additions & 4 deletions

File tree

kernel/sched/cpufreq_schedutil.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,11 +322,10 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
322322
sugov_update_commit(sg_policy, time, next_f);
323323
}
324324

325-
static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu)
325+
static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
326326
{
327327
struct sugov_policy *sg_policy = sg_cpu->sg_policy;
328328
struct cpufreq_policy *policy = sg_policy->policy;
329-
u64 last_freq_update_time = sg_policy->last_freq_update_time;
330329
unsigned long util = 0, max = 1;
331330
unsigned int j;
332331

@@ -342,7 +341,7 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu)
342341
* enough, don't take the CPU into account as it probably is
343342
* idle now (and clear iowait_boost for it).
344343
*/
345-
delta_ns = last_freq_update_time - j_sg_cpu->last_update;
344+
delta_ns = time - j_sg_cpu->last_update;
346345
if (delta_ns > TICK_NSEC) {
347346
j_sg_cpu->iowait_boost = 0;
348347
j_sg_cpu->iowait_boost_pending = false;
@@ -387,7 +386,7 @@ static void sugov_update_shared(struct update_util_data *hook, u64 time,
387386
if (flags & SCHED_CPUFREQ_DL)
388387
next_f = sg_policy->policy->cpuinfo.max_freq;
389388
else
390-
next_f = sugov_next_freq_shared(sg_cpu);
389+
next_f = sugov_next_freq_shared(sg_cpu, time);
391390

392391
sugov_update_commit(sg_policy, time, next_f);
393392
}

0 commit comments

Comments
 (0)