| Message ID | 5406371.vuaACy5i7F@aspire.rjw.lan (mailing list archive) |
|---|---|
| State | Mainlined |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | cpufreq: conservative: Take limits changes into account properly | expand |
On Mon, 15 Oct 2018 at 23:24, Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > If the policy limits change between invocations of cs_dbs_update(), > the requested frequency value stored in dbs_info may not be updated > and the function may use a stale value of it next time. Moreover, if > idle periods are takem into account by cs_dbs_update(), the requested > frequency value stored in dbs_info may be below the min policy limit, > which is incorrect. > > To fix these problems, always update the requested frequency value > in dbs_info along with the local copy of it when the previous > requested frequency is beyond the policy limits and avoid decreasing > the requested frequency below the min policy limit when taking > idle periods into account. > > Reported-by: Waldemar Rymarkiewicz <waldemarx.rymarkiewicz@intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/cpufreq/cpufreq_conservative.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c > =================================================================== > --- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c > +++ linux-pm/drivers/cpufreq/cpufreq_conservative.c > @@ -80,8 +80,10 @@ static unsigned int cs_dbs_update(struct > * changed in the meantime, so fall back to current frequency in that > * case. > */ > - if (requested_freq > policy->max || requested_freq < policy->min) > + if (requested_freq > policy->max || requested_freq < policy->min) { > requested_freq = policy->cur; > + dbs_info->requested_freq = requested_freq; > + } > > freq_step = get_freq_step(cs_tuners, policy); > > @@ -92,7 +94,7 @@ static unsigned int cs_dbs_update(struct > if (policy_dbs->idle_periods < UINT_MAX) { > unsigned int freq_steps = policy_dbs->idle_periods * freq_step; > > - if (requested_freq > freq_steps) > + if (requested_freq > policy->min + freq_steps) > requested_freq -= freq_steps; > else > requested_freq = policy->min; Looks good. Acked-by: Waldemar Rymarkiewicz <waldemarx.rymarkiewicz@intel.com>
On 15-10-18, 23:21, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > If the policy limits change between invocations of cs_dbs_update(), > the requested frequency value stored in dbs_info may not be updated > and the function may use a stale value of it next time. Moreover, if > idle periods are takem into account by cs_dbs_update(), the requested > frequency value stored in dbs_info may be below the min policy limit, > which is incorrect. > > To fix these problems, always update the requested frequency value > in dbs_info along with the local copy of it when the previous > requested frequency is beyond the policy limits and avoid decreasing > the requested frequency below the min policy limit when taking > idle periods into account. > > Reported-by: Waldemar Rymarkiewicz <waldemarx.rymarkiewicz@intel.com> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/cpufreq/cpufreq_conservative.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Index: linux-pm/drivers/cpufreq/cpufreq_conservative.c =================================================================== --- linux-pm.orig/drivers/cpufreq/cpufreq_conservative.c +++ linux-pm/drivers/cpufreq/cpufreq_conservative.c @@ -80,8 +80,10 @@ static unsigned int cs_dbs_update(struct * changed in the meantime, so fall back to current frequency in that * case. */ - if (requested_freq > policy->max || requested_freq < policy->min) + if (requested_freq > policy->max || requested_freq < policy->min) { requested_freq = policy->cur; + dbs_info->requested_freq = requested_freq; + } freq_step = get_freq_step(cs_tuners, policy); @@ -92,7 +94,7 @@ static unsigned int cs_dbs_update(struct if (policy_dbs->idle_periods < UINT_MAX) { unsigned int freq_steps = policy_dbs->idle_periods * freq_step; - if (requested_freq > freq_steps) + if (requested_freq > policy->min + freq_steps) requested_freq -= freq_steps; else requested_freq = policy->min;