Re: [PATCH] cpufreq: ondemand: Change the calculation of target frequency

[Stratos Karafotis <stratosk@semaphore.gr>]

[-- Attachment #1: Type: text/plain, Size: 2740 bytes --]

On 05/31/2013 11:51 AM, Viresh Kumar wrote:
>> ---
>>   arch/x86/include/asm/processor.h   | 29 ----------------------
>>   drivers/cpufreq/Makefile           |  2 +-
>>   drivers/cpufreq/acpi-cpufreq.c     |  5 ----
>>   drivers/cpufreq/cpufreq.c          | 21 ----------------
>>   drivers/cpufreq/cpufreq_governor.c | 10 +-------
>>   drivers/cpufreq/cpufreq_governor.h |  1 -
>>   drivers/cpufreq/cpufreq_ondemand.c | 39 ++++++-----------------------
>>   drivers/cpufreq/mperf.c            | 51 --------------------------------------
>>   drivers/cpufreq/mperf.h            |  9 -------
>>   include/linux/cpufreq.h            |  6 -----
>>   10 files changed, 9 insertions(+), 164 deletions(-)
>>   delete mode 100644 drivers/cpufreq/mperf.c
>>   delete mode 100644 drivers/cpufreq/mperf.h
> 
> I believe you should have removed other users of getavg() in a separate
> patch and also cc'd relevant people so that you can some review comments
> from  them.

I will split the patch in two. If it's OK, I will keep the removal of 
__cpufreq_driver_getavg in the original patch and move the clean up of
APERF/MPERF support in a second patch. I will also cc relevant people.


>>          /* Check for frequency increase */
>> -       if (load_freq > od_tuners->up_threshold * policy->cur) {
>> +       if (load > od_tuners->up_threshold) {
> 
> Chances of this getting hit are minimal now.. I don't know if keeping
> this will change anything now :)

Actually, no. This getting hit pretty often.
Please find attached the cpufreq statistics - trans_table during build
of 3.4 kernel. With default up_threshold (95), the transition to max
happened many times because of load was greater than up_threshold.
I also thought to keep this code to leave up_threshold functionality unaffected.
 
On 05/31/2013 03:42 PM, Rafael J. Wysocki wrote:
> On Friday, May 31, 2013 02:24:59 PM Viresh Kumar wrote:
>>> +       } else {
>>> +               /* Calculate the next frequency proportional to load */
>>>                  unsigned int freq_next;
>>> -               freq_next = load_freq / od_tuners->adj_up_threshold;
>>> +               freq_next = load * policy->max / 100;
>>
>> Rafael asked why you believe this is the right formula and I really couldn't
>> find an appropriate answer to that, sorry :(
> 
> Right, it would be good to explain that.
> 
> "Proportional to load" means C * load, so why is "policy->max / 100" *the* right C?
> 

I think, finally(?) I see your point. The right C should be "policy->cpuinfo.max_freq / 100".
This way the target frequency will be proportional to load and the calculation will
"map" the load to CPU freq table.

I will update the patch according to your observations and suggestions.

Thanks,
Stratos

[-- Attachment #2: trans_table.txt --]
[-- Type: text/plain, Size: 2941 bytes --]

   From  :    To
         :   3401000   3400000   3300000   3100000   3000000   2900000   2800000   2600000   2500000   2400000   2200000   2100000   2000000   1900000   1700000   1600000 
  3401000:         0         0         4         2         4         2         3         0         2         1         1         1         1         4         0        29 
  3400000:         0         0         0         0         0         0         0         0         0         0         0         0         0         0         0         0 
  3300000:         4         0         0         0         1         0         0         0         0         0         0         1         0         0         0         7 
  3100000:         2         0         0         0         1         0         0         0         0         0         0         0         0         0         0         0 
  3000000:         4         0         0         0         0         0         0         0         0         1         0         0         0         0         0         4 
  2900000:         1         0         0         0         1         0         0         0         0         0         0         0         0         0         0         7 
  2800000:         3         0         0         0         0         1         0         0         0         1         0         0         0         0         0         3 
  2600000:         0         0         0         0         0         1         0         0         0         1         0         0         0         0         0         4 
  2500000:         0         0         0         0         0         0         0         0         0         0         0         0         0         0         0         4 
  2400000:         3         0         0         0         0         0         1         0         0         0         0         0         0         0         0         7 
  2200000:         1         0         0         0         0         0         0         0         0         1         0         0         0         0         0         3 
  2100000:         1         0         0         0         0         0         0         1         0         0         0         0         0         1         0         4 
  2000000:         1         0         0         0         0         0         0         0         0         0         0         0         0         0         0         1 
  1900000:         0         0         2         0         0         0         0         0         0         1         0         0         0         0         0         8 
  1700000:         0         0         0         0         0         0         0         0         0         0         0         0         0         0         0         2 
  1600000:        33         0         7         1         2         5         4         5         2         5         4         5         1         6         2         0