From: zhuguangqing <[email protected]>
In the following code path, next_freq is clamped between policy->min
and policy->max twice in functions cpufreq_driver_resolve_freq() and
cpufreq_driver_fast_switch(). For there is no update_lock in the code
path, policy->min and policy->max may be modified (one or more times),
so sg_policy->next_freq updated in function sugov_update_next_freq()
may be not the final cpufreq. Next time when we use
"if (sg_policy->next_freq == next_freq)" to judge whether to update
next_freq, we may get a wrong result.
-> sugov_update_single()
-> get_next_freq()
-> cpufreq_driver_resolve_freq()
-> sugov_fast_switch()
-> sugov_update_next_freq()
-> cpufreq_driver_fast_switch()
For example, at first sg_policy->next_freq is 1 GHz, but the final
cpufreq is 1.2 GHz because policy->min is modified to 1.2 GHz when
we reached cpufreq_driver_fast_switch(). Then next time, policy->min
is changed before we reached cpufreq_driver_resolve_freq() and (assume)
next_freq is 1 GHz, we find "if (sg_policy->next_freq == next_freq)" is
satisfied so we don't change the cpufreq. Actually we should change
the cpufreq to 1.0 GHz this time.
Signed-off-by: zhuguangqing <[email protected]>
---
drivers/cpufreq/cpufreq.c | 6 +++---
include/linux/cpufreq.h | 2 +-
kernel/sched/cpufreq_schedutil.c | 21 ++++++++++-----------
3 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index f4b60663efe6..7e8e03c7506b 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2057,13 +2057,13 @@ EXPORT_SYMBOL(cpufreq_unregister_notifier);
* error condition, the hardware configuration must be preserved.
*/
unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
- unsigned int target_freq)
+ unsigned int *target_freq)
{
unsigned int freq;
int cpu;
- target_freq = clamp_val(target_freq, policy->min, policy->max);
- freq = cpufreq_driver->fast_switch(policy, target_freq);
+ *target_freq = clamp_val(*target_freq, policy->min, policy->max);
+ freq = cpufreq_driver->fast_switch(policy, *target_freq);
if (!freq)
return 0;
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index fa37b1c66443..790df38d48de 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -569,7 +569,7 @@ struct cpufreq_governor {
/* Pass a target to the cpufreq driver */
unsigned int cpufreq_driver_fast_switch(struct cpufreq_policy *policy,
- unsigned int target_freq);
+ unsigned int *target_freq);
int cpufreq_driver_target(struct cpufreq_policy *policy,
unsigned int target_freq,
unsigned int relation);
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index e254745a82cb..38d2dc55dd95 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -99,31 +99,30 @@ static bool sugov_should_update_freq(struct sugov_policy
*sg_policy, u64 time)
return delta_ns >= sg_policy->freq_update_delay_ns;
}
-static bool sugov_update_next_freq(struct sugov_policy *sg_policy, u64 time,
+static inline bool sugov_update_next_freq(struct sugov_policy *sg_policy,
unsigned int next_freq)
{
- if (sg_policy->next_freq == next_freq)
- return false;
-
- sg_policy->next_freq = next_freq;
- sg_policy->last_freq_update_time = time;
-
- return true;
+ return sg_policy->next_freq == next_freq ? false : true;
}
static void sugov_fast_switch(struct sugov_policy *sg_policy, u64 time,
unsigned int next_freq)
{
- if (sugov_update_next_freq(sg_policy, time, next_freq))
- cpufreq_driver_fast_switch(sg_policy->policy, next_freq);
+ if (sugov_update_next_freq(sg_policy, next_freq)) {
+ cpufreq_driver_fast_switch(sg_policy->policy, &next_freq);
+ sg_policy->next_freq = next_freq;
+ sg_policy->last_freq_update_time = time;
+ }
}
static void sugov_deferred_update(struct sugov_policy *sg_policy, u64 time,
unsigned int next_freq)
{
- if (!sugov_update_next_freq(sg_policy, time, next_freq))
+ if (!sugov_update_next_freq(sg_policy, next_freq))
return;
+ sg_policy->next_freq = next_freq;
+ sg_policy->last_freq_update_time = time;
if (!sg_policy->work_in_progress) {
sg_policy->work_in_progress = true;
irq_work_queue(&sg_policy->irq_work);
--
2.17.1
