New: Intel Pstate driver - "kick" code is not needed
Subsystems (2)
https://bugzilla.kernel.org/show_bug.cgi?id=64271
Bug ID: 64271
Summary: Intel Pstate driver - "kick" code is not needed
Product: Power Management
Version: 2.5
Kernel Version: 3.12rc7
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: cpufreq
Assignee: [email protected]
Reporter: [email protected]
Regression: NoThere is code in the routine intel_pstate_timer_func to "kick" the CPU out of the minimum pstate.
The code was added back when several things were not working properly with the driver. It was added due to a "ping-pong" issue uncovered by the Phoronix test suite ffpmeg test. I have tried extensively to recreate the issue with the "kick" code removed and have not been able to. I am unable to detect any difference with the ffmpeg test between the normal code and code with the "kick removed" The assertion is that the "kick" code is not required and should be deleted.
This bug has been extracted from bug 59481 which ended up discussing a few tangent issues. The main issue was solved and the bug closed. Now I am following up on some of the tangent issues.
I am saying the kick portion of this code can be removed because it serves no purpose and makes the code harder to read and understand:
static void intel_pstate_timer_func(unsigned long __data)
{
struct cpudata *cpu = (struct cpudata *) __data;
intel_pstate_sample(cpu);
intel_pstate_adjust_busy_pstate(cpu);
if (cpu->pstate.current_pstate == cpu->pstate.min_pstate) {
cpu->min_pstate_count++;
if (!(cpu->min_pstate_count % 5)) {
intel_pstate_set_pstate(cpu, cpu->pstate.max_pstate);
}
} else
cpu->min_pstate_count = 0;
intel_pstate_set_sample_time(cpu);
}Where this was left on 2013.07.18 with Dirk Brandewie was that he wanted to go back and verify in his test environment. Which, of course, should be done.
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.
[Bug 64271] Intel Pstate driver - "kick" code is not needed
https://bugzilla.kernel.org/show_bug.cgi?id=64271
--- Comment #1 from Doug Smythies <[email protected]> --- Adding some phoronix ffmpeg test results:
Kernel: 3.12RC7 with No Kick modification.
Linux s15 3.12.0-rc7-nokick #49 SMP Sat Nov 2 21:40:01 PDT 2013 x86_64 x86_64 x86_64 GNU/Linux
Mode: Powersave with default settings.
Sample size 250 tests. (the limit seems to be 256)
Test run 1 is rejected as it includes extra time to load the test file. For subsequent tests, the file is already cached.
ave 13.475713 seconds max 13.70577598 seconds min 13.29449391 seconds
stddev 0.073586378 seconds
Kernel: 3.12RC7
Linux s15 3.12.0-rc7+ #48 SMP Fri Nov 1 21:09:51 PDT 2013 x86_64 x86_64 x86_64 GNU/Linux
Mode: Performance with default settings.
Sample size 250 tests.
Test run 1 is NOT rejected as, due an error on my part, I restarted the test and the so the test file was already cached.
ave 13.4565717 seconds max 13.66979098 seconds min 13.2799778 seconds
stddev 0.070516137 seconds
Kernel: 3.12RC7
Linux s15 3.12.0-rc7+ #48 SMP Fri Nov 1 21:09:51 PDT 2013 x86_64 x86_64 x86_64 GNU/Linux
Mode: Powersave with default settings.
Sample size 250 tests.
Test run 1 is NOT rejected as the test had been just run under different conditions so the test file was already cached.
ave 13.45689662 seconds min 13.64681292 seconds max 13.30776 seconds
stddev 0.067261141 seconds
System Information
Hardware:
Processor: Intel Core i7-2600K @ 3.80GHz (8 Cores), Motherboard: ASUS P8Z68-M PRO, Chipset: Intel 2nd Generation Core Family DRAM, Memory: 16384MB, Disk:
1000GB Seagate ST1000DM003-9YN1 + 1000GB Seagate ST1000DL002-9TT1 + 120GB INTEL SSDSA2CW12, Graphics: Intel 2nd Generation Core Family IGP, Audio: Realtek ALC892, Network: Intel 82574L Gigabit Connection
Software:
OS: Ubuntu 12.04, Kernel: 3.12.0-rc7+ (x86_64), Compiler: GCC 4.6, File-System:
ext4, Screen Resolution: 1680x1050
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.
[Bug 64271] Intel Pstate driver - "kick" code is not needed
In reply to [email protected]https://bugzilla.kernel.org/show_bug.cgi?id=64271
Dirk Brandewie <[email protected]> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |[email protected]
--- Comment #2 from Dirk Brandewie <[email protected]> --- Agreed. Do you want to submit the patch or should I?
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.
[Bug 64271] Intel Pstate driver - "kick" code is not needed
In reply to [email protected]https://bugzilla.kernel.org/show_bug.cgi?id=64271
--- Comment #3 from Doug Smythies <[email protected]> ---
Dirk: I do not know how to submit a patch. Which doesn't mean I shouldn't learn how, but I have also been cheating when I make my "no kick" test code by just deleting the 3 lines that decide to and do the kick. The real solution would be a bit more. I'll do it of you want (it would be a good way to get rid of me for awhile (just joking)).
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.
[Bug 64271] Intel Pstate driver - "kick" code is not needed
In reply to [email protected]https://bugzilla.kernel.org/show_bug.cgi?id=64271
--- Comment #4 from Dirk Brandewie <[email protected]> --- Here is the patch I will send upstream it may not make it until v3.14-rc1 since the merge window is upon us :-)
commit 7d9aba45dbd356686eca097b31028eccec0e11d6
Author: Dirk Brandewie <[email protected]> Date: Mon Oct 14 07:54:45 2013 -0700
cpufreq/intel_pstate: Remove periodic P state boost
Remove the periodic P state boost. This code required for some corner case benchmark tests. The calculation of the required P state was incorrect/inaccurate and would not allow P state increase.
This was fixed by a combination of commits:
2134ed4 cpufreq / intel_pstate: Change to scale off of max P-state d253d2a intel_pstate: Improve accuracy by not truncating until final
result
Fixes Bug:
https://bugzilla.kernel.org/show_bug.cgi?id=64271
--- drivers/cpufreq/intel_pstate.c | 11 ----------- 1 file changed, 11 deletions(-)
drivers/cpufreq/intel_pstate.c +0 -11
@@ -83,8 +83,6 @@ struct cpudata {struct pstate_data pstate;struct _pid pid; -int min_pstate_count; -u64 prev_aperf;u64 prev_mperf;int sample_ptr;
@@ -567,15 +565,6 @@ static void intel_pstate_timer_func(unsigned long __data)intel_pstate_sample (cpu);intel_pstate_adjust_busy_pstate (cpu); - -if (cpu->pstate.current_pstate == cpu->pstate.min_pstate) { - cpu->min_pstate_count++; -if (!(cpu->min_pstate_count %5 )) { -intel_pstate_set_pstate (cpu, cpu->pstate.max_pstate); - } - }else - cpu->min_pstate_count =0 ; -intel_pstate_set_sample_time (cpu); }
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.
[Bug 64271] Intel Pstate driver - "kick" code is not needed
In reply to [email protected]https://bugzilla.kernel.org/show_bug.cgi?id=64271
--- Comment #5 from Doug Smythies <[email protected]> ---
Dirk: Thanks.
Note that I have tested this to death already, and now I only run with a nokick version of intel_pstate.
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.
[Bug 64271] Intel Pstate driver - "kick" code is not needed
In reply to [email protected]https://bugzilla.kernel.org/show_bug.cgi?id=64271
Aaron Lu <[email protected]> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
CC| |[email protected]
Resolution|--- |CODE_FIX--- Comment #6 from Aaron Lu <[email protected]> --- commit 91a4cd4f3d8169d7398f9123683f64575927c682
Author: Dirk Brandewie <[email protected]> Date: Tue Dec 17 09:42:07 2013 -0800
intel_pstate: Remove periodic P state boost
entered Linus' tree as v3.14 material.
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.
[Bug 64271] Intel Pstate driver - "kick" code is not needed
In reply to [email protected]https://bugzilla.kernel.org/show_bug.cgi?id=64271
Aaron Lu <[email protected]> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |CLOSED
Signature · 2 lines
You are receiving this mail because: You are the assignee for the bug.