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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 31 May 2013 19:33:06 +0300
From:	Stratos Karafotis <stratosk@...aphore.gr>
To:	Viresh Kumar <viresh.kumar@...aro.org>,
	"Rafael J. Wysocki" <rjw@...k.pl>
CC:	linux-kernel@...r.kernel.org, cpufreq@...r.kernel.org,
	linux-pm@...r.kernel.org
Subject: Re: [PATCH] cpufreq: ondemand: Change the calculation of target frequency

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

View attachment "trans_table.txt" of type "text/plain" (2942 bytes)