Re: [PATCH] cpufreq: ondemand: Change the calculation of target frequency
From: Stratos Karafotis
Date: Sat Jun 01 2013 - 08:50:15 EST
On 06/01/2013 03:27 PM, Rafael J. Wysocki wrote:
> On Friday, May 31, 2013 07:33:06 PM Stratos Karafotis wrote:
>> 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.
>
> That seems to mean "take the percentage of policy->cpuinfo.max_freq proportional
> to the current load and use the available frequency closest to that". Is that
> correct?
>
> Rafael
>
>
In my opinion, yes. I thought, yesterday, after your question, to normalize load
to policy->min - policy->max. But I think it's a more correct approach to take
the percentage of cpuinfo.max, as you said.
Actually, I did my tests on the percentage of policy->max that was equal to
cpuinfo.max.
Unless, I miss something here. :)
Thanks,
Stratos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/