Messages in this thread | | | Date | Wed, 28 Oct 2020 13:51:48 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH] cpufreq: schedutil: set sg_policy->next_freq to the final cpufreq |
| |
On 27-10-20, 19:54, zhuguangqing83@gmail.com wrote: > From: zhuguangqing <zhuguangqing@xiaomi.com> > > 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.
Understood until here, but not sure I followed everything after that.
> 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.
FWIW, whenever policy->min/max gets changed, sg_policy->limits_changed gets set to true by sugov_limits() and the next time schedutil callback gets called from the scheduler, we will fix the frequency.
And so there shouldn't be any issue here, unless I am missing something.
-- viresh
| |