From: Haren Myneni <hidden> Date: 2012-09-09 11:37:58
enable_ppr kernel parameter is used to enable PPR save and restore.
Supported on Power7 and later processors.
By default, CPU_FTR_HAS_PPR is set for POWER7. If this parameter is not
passed, disable CPU_FTR_HAS_PPR.
Signed-off-by: Haren Myneni <haren@us.ibm.com>
---
Documentation/kernel-parameters.txt | 4 ++++
arch/powerpc/include/asm/cputable.h | 6 ++++--
arch/powerpc/kernel/setup_64.c | 14 ++++++++++++++
3 files changed, 22 insertions(+), 2 deletions(-)
@@ -809,6 +809,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted. to discrete, to make X server driver able to add WB entry later. This parameter enables that.+ enable_ppr [PPC/PSERIES]+ Saves user defined PPR when process enters to kernel + and restores PPR at exit. But it impacts performance.+ enable_timer_pin_1 [X86] Enable PIN 1 of APIC timer Can be useful to work around chipset bugs
@@ -98,6 +98,8 @@ int dcache_bsize;inticache_bsize;intucache_bsize;+staticu32enable_ppr=0;+#ifdef CONFIG_SMPstaticchar*smt_enabled_cmdline;
@@ -357,6 +359,9 @@ void __init setup_system(void){DBG(" -> setup_system()\n");+if(cpu_has_feature(CPU_FTR_HAS_PPR)&&!enable_ppr)+cur_cpu_spec->cpu_features&=~CPU_FTR_HAS_PPR;+/* Apply the CPUs-specific and firmware specific fixups to kernel*text(nopoutsectionsnotrelevanttothisCPUorthisfirmware)*/
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-09-10 00:12:10
On Sun, 2012-09-09 at 04:37 -0700, Haren Myneni wrote:
enable_ppr kernel parameter is used to enable PPR save and restore.
Supported on Power7 and later processors.
By default, CPU_FTR_HAS_PPR is set for POWER7. If this parameter is not
passed, disable CPU_FTR_HAS_PPR.
What is the point ? Obscure / magic kernel command line options to turn
on a feature are pointless. Nobody knows about them, nobody enables
them.
What you are doing is guarantee that nobody's ever going to enable your
code, so your whole patch series is thus irrelevant :-)
If there's a good reason to *avoid* your option, then maybe consider
adding an option to *disable* the PPR save/restore, though of course
that would bring the argument that if it needs to be disabled maybe we
shouldn't do it in the first place, or you might want to think of a
reasonable way to intuit what the option should be.
Ben.
@@ -809,6 +809,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted. to discrete, to make X server driver able to add WB entry later. This parameter enables that.+ enable_ppr [PPC/PSERIES]+ Saves user defined PPR when process enters to kernel + and restores PPR at exit. But it impacts performance.+ enable_timer_pin_1 [X86] Enable PIN 1 of APIC timer Can be useful to work around chipset bugs
@@ -98,6 +98,8 @@ int dcache_bsize;inticache_bsize;intucache_bsize;+staticu32enable_ppr=0;+#ifdef CONFIG_SMPstaticchar*smt_enabled_cmdline;
@@ -357,6 +359,9 @@ void __init setup_system(void){DBG(" -> setup_system()\n");+if(cpu_has_feature(CPU_FTR_HAS_PPR)&&!enable_ppr)+cur_cpu_spec->cpu_features&=~CPU_FTR_HAS_PPR;+/* Apply the CPUs-specific and firmware specific fixups to kernel*text(nopoutsectionsnotrelevanttothisCPUorthisfirmware)*/
From: Michael Neuling <hidden> Date: 2012-09-10 00:22:38
Benjamin Herrenschmidt [off-list ref] wrote:
On Sun, 2012-09-09 at 04:37 -0700, Haren Myneni wrote:
quoted
enable_ppr kernel parameter is used to enable PPR save and restore.
Supported on Power7 and later processors.
By default, CPU_FTR_HAS_PPR is set for POWER7. If this parameter is not
passed, disable CPU_FTR_HAS_PPR.
What is the point ? Obscure / magic kernel command line options to turn
on a feature are pointless. Nobody knows about them, nobody enables
them.
What you are doing is guarantee that nobody's ever going to enable your
code, so your whole patch series is thus irrelevant :-)
If there's a good reason to *avoid* your option, then maybe consider
adding an option to *disable* the PPR save/restore, though of course
that would bring the argument that if it needs to be disabled maybe we
shouldn't do it in the first place, or you might want to think of a
reasonable way to intuit what the option should be.
IIRC, Haren was saying there's a 6% hit on null syscall for this. So we
suggested having a cmdline option to disable it for distros.
Haren, is my recollection correct? If so, can you add this info the
change log and change the sex of the option.
Mikey
@@ -809,6 +809,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted. to discrete, to make X server driver able to add WB entry later. This parameter enables that.+ enable_ppr [PPC/PSERIES]+ Saves user defined PPR when process enters to kernel + and restores PPR at exit. But it impacts performance.+ enable_timer_pin_1 [X86] Enable PIN 1 of APIC timer Can be useful to work around chipset bugs
@@ -98,6 +98,8 @@ int dcache_bsize;inticache_bsize;intucache_bsize;+staticu32enable_ppr=0;+#ifdef CONFIG_SMPstaticchar*smt_enabled_cmdline;
@@ -357,6 +359,9 @@ void __init setup_system(void){DBG(" -> setup_system()\n");+if(cpu_has_feature(CPU_FTR_HAS_PPR)&&!enable_ppr)+cur_cpu_spec->cpu_features&=~CPU_FTR_HAS_PPR;+/* Apply the CPUs-specific and firmware specific fixups to kernel*text(nopoutsectionsnotrelevanttothisCPUorthisfirmware)*/
From: Haren Myneni <hidden> Date: 2012-09-11 05:42:20
On 09/09/2012 05:22 PM, Michael Neuling wrote:
Benjamin Herrenschmidt [off-list ref] wrote:
quoted
On Sun, 2012-09-09 at 04:37 -0700, Haren Myneni wrote:
quoted
enable_ppr kernel parameter is used to enable PPR save and restore.
Supported on Power7 and later processors.
By default, CPU_FTR_HAS_PPR is set for POWER7. If this parameter is not
passed, disable CPU_FTR_HAS_PPR.
What is the point ? Obscure / magic kernel command line options to turn
on a feature are pointless. Nobody knows about them, nobody enables
them.
What you are doing is guarantee that nobody's ever going to enable your
code, so your whole patch series is thus irrelevant :-)
If there's a good reason to *avoid* your option, then maybe consider
adding an option to *disable* the PPR save/restore, though of course
that would bring the argument that if it needs to be disabled maybe we
shouldn't do it in the first place, or you might want to think of a
reasonable way to intuit what the option should be.
IIRC, Haren was saying there's a 6% hit on null syscall for this. So we
suggested having a cmdline option to disable it for distros.
Haren, is my recollection correct? If so, can you add this info the
change log and change the sex of the option.
Thanks Michael. Yes, we noticed 6% overhead with null syscall test.
Hence added cmdline option as suggested. I will add this comment in the
changelog.
Regarding the option name, I thought about various ones such as
retain_process_ppr, retain_smt_priority, save_ppr and etc. Finally added
'enable_ppr' since it enables CPU_FTR (CPU_FTR_HAS_PPR) which allows to
save/restore PPR value. Sure, I will change this option.
Thanks
Haren
@@ -809,6 +809,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted. to discrete, to make X server driver able to add WB entry later. This parameter enables that.+ enable_ppr [PPC/PSERIES]+ Saves user defined PPR when process enters to kernel + and restores PPR at exit. But it impacts performance.+ enable_timer_pin_1 [X86] Enable PIN 1 of APIC timer Can be useful to work around chipset bugs
@@ -98,6 +98,8 @@ int dcache_bsize;inticache_bsize;intucache_bsize;+staticu32enable_ppr=0;+#ifdef CONFIG_SMPstaticchar*smt_enabled_cmdline;
@@ -357,6 +359,9 @@ void __init setup_system(void){DBG(" -> setup_system()\n");+if(cpu_has_feature(CPU_FTR_HAS_PPR)&&!enable_ppr)+cur_cpu_spec->cpu_features&=~CPU_FTR_HAS_PPR;+/* Apply the CPUs-specific and firmware specific fixups to kernel*text(nopoutsectionsnotrelevanttothisCPUorthisfirmware)*/
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2012-09-11 05:55:21
On Mon, 2012-09-10 at 22:42 -0700, Haren Myneni wrote:
Thanks Michael. Yes, we noticed 6% overhead with null syscall test.
Hence added cmdline option as suggested. I will add this comment in
the
changelog.
Regarding the option name, I thought about various ones such as
retain_process_ppr, retain_smt_priority, save_ppr and etc. Finally
added
'enable_ppr' since it enables CPU_FTR (CPU_FTR_HAS_PPR) which allows
to
save/restore PPR value. Sure, I will change this option.
No, that isn't a problem with the name. It's a problem with the polarity
of the option.
If you need a command line argument to enable the option, then nobody
will enable it, it's pointless.
Cheers,
Ben.
From: Ryan Arnold <hidden> Date: 2012-09-28 22:11:56
On Tue, 2012-09-11 at 15:55 +1000, Benjamin Herrenschmidt wrote:
On Mon, 2012-09-10 at 22:42 -0700, Haren Myneni wrote:
quoted
Thanks Michael. Yes, we noticed 6% overhead with null syscall test.
Hence added cmdline option as suggested. I will add this comment in
the
changelog.
Regarding the option name, I thought about various ones such as
retain_process_ppr, retain_smt_priority, save_ppr and etc. Finally
added
'enable_ppr' since it enables CPU_FTR (CPU_FTR_HAS_PPR) which allows
to
save/restore PPR value. Sure, I will change this option.
No, that isn't a problem with the name. It's a problem with the polarity
of the option.
If you need a command line argument to enable the option, then nobody
will enable it, it's pointless.
In GLIBC (ppc.h) we'll be providing a user space API to change the
thread priority in user state. We're also interested in using this in
some of the locking constructs if performance tests indicate it's
beneficial.
I have concerns with being able to enable/disable this option at boot
time. Usually, in GLIBC we'll just do a kernel version check and enable
certain facilities if we're building against a particular kernel that
supports them.
In this case, with a configurable option, GLIBC is going to need the
kernel to export a hwcap bit that tells us whether we need to do the
save/restore ourselves. Having to check the hwcap, and do the
save/restore in user space will, of course, increase the overhead on our
side.
If no hwcap bit is provided and this is disabled at kernel boot time, no
check is done and the user process assumes it's running under a certain
priority when it is, in-fact, not. I don't care for this option. We'll
be hitting code paths that are ineffective and unnecessary.
Ryan S. Arnold
Linux Technology Center