Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1416784

Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in ascending order

Show all headers | View raw

On Tuesday, June 07, 2016 09:58:07 AM Viresh Kumar wrote:
> On 06-06-16, 23:56, Rafael J. Wysocki wrote:
> > Since you are adding new code, you can write it so it doesn't do
> > unnecessary checks from the start.
> 
> Hmm, I will do all that in this series only now.
> 
> > While at it, the "if ((freq < policy->min) || (freq > policy->max))"
> > checks in cpufreq_find_index_l() and cpufreq_find_index_h() don't look
> > good to me, because they very well may cause those function to return
> > -EINVAL even when there's a valid table and that may cause
> > acpi_cpufreq_fast_switch() to do bad things.
> 
> Hmm. So, the checks are for sure required here, otherwise we may end up
> returning a frequency which we aren't allowed to. Also note that 'freq' here
> isn't the target-freq, but the entry in the freq-table.
> 
> This routine should be returning a valid freq within the ranges specified by
> policy->min/max.

Which in principle may not be possible if the range doesn't include any
frequency in the table, eg. min == max and between the table entries.

However, the CPU has to run at *some* frequency, even if there's none in the
min/max range.

And if we are sure that there is at least one valid frequency between min
and max, please note that target_freq has already been clamped between them,
so clamping again is rather unuseful.  And of course it is racy in general,
which makes it even more unuseful.

> Also note that these routines shall *never* return -EINVAL, otherwise it is
> mostly a bug we are hitting.

So make them explicitly return a valid frequency every time.

> We have enough checks in place to make sure that there is at least one valid
> entry in the freq-table which is >= policy->min and <= policy->max.

That assuming that the driver will always do the right thing in its ->verify
callback.

> I will take care of rest of the comments though. Thanks.

Thanks!

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in ascending order Viresh Kumar <viresh.kumar@linaro.org> - 2016-06-03 15:40 +0200
  Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order Steve Muckle <steve.muckle@linaro.org> - 2016-06-04 01:50 +0200
    Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order Viresh Kumar <viresh.kumar@linaro.org> - 2016-06-06 06:00 +0200
      Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in ascending order "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-06-06 14:10 +0200
        Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order Viresh Kumar <viresh.kumar@linaro.org> - 2016-06-06 14:40 +0200
          Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-06 15:00 +0200
            Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order Viresh Kumar <viresh.kumar@linaro.org> - 2016-06-06 18:30 +0200
              Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order "Rafael J. Wysocki" <rafael@kernel.org> - 2016-06-07 00:00 +0200
                Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order Viresh Kumar <viresh.kumar@linaro.org> - 2016-06-07 06:30 +0200
                Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in ascending order "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-06-08 02:40 +0200
                Re: [PATCH V3 8/9] cpufreq: Keep policy->freq_table sorted in  ascending order Viresh Kumar <viresh.kumar@linaro.org> - 2016-06-08 05:50 +0200

csiph-web