Re: [PATCH] cpufreq: ondemand: Change the calculation of target frequency
From: Stratos Karafotis
Date: Fri May 31 2013 - 12:33:30 EST
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
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