On 2019.08.02 02:12 Rafael J. Wysocki wrote:
On Fri, Aug 2, 2019 at 7:44 AM Viresh Kumar viresh.kumar@linaro.org wrote:
To avoid reducing the frequency of a CPU prematurely, we skip reducing the frequency if the CPU had been busy recently.
This should not be done when the limits of the policy are changed, for example due to thermal throttling. We should always get the frequency within the new limits as soon as possible.
Trying to fix this by using only one flag, i.e. need_freq_update, can lead to a race condition where the flag gets cleared without forcing us to change the frequency at least once. And so this patch introduces another flag to avoid that race condition.
Fixes: ecd288429126 ("cpufreq: schedutil: Don't set next_freq to UINT_MAX") Cc: v4.18+ stable@vger.kernel.org # v4.18+ Reported-by: Doug Smythies doug.smythies@gmail.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
V2->V3:
- Updated commit log.
V1->V2:
- Fixed the race condition using a different flag.
@Doug: I haven't changed the code since you last tested these. Your Tested-by tag can be useful while applying the patches. Thanks.
Tested-by: Doug Smythies dsmythies@telus.net For acpi-cpufreq/schedutil only (which we already know).
I tested including Rafael's suggested change. I.E.
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index 592ff72..ae3ec77 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -441,7 +441,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time, struct sugov_policy *sg_policy = sg_cpu->sg_policy; unsigned long util, max; unsigned int next_f; - bool busy = false; + bool busy;
sugov_iowait_boost(sg_cpu, time, flags); sg_cpu->last_update = time; @@ -452,8 +452,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time, return;
/* Limits may have changed, don't skip frequency update */ - if (!sg_policy->need_freq_update) - busy = sugov_cpu_is_busy(sg_cpu); + busy = !sg_policy->need_freq_update && sugov_cpu_is_busy(sg_cpu);
util = sugov_get_util(sg_cpu); max = sg_cpu->max;
... Doug