[PATCH 1/3] x86: mtrr_cleanup update command line

[PATCH 1/3] x86: mtrr_cleanup update command line

[Yinghai Lu]

change enable_mtrr_cleanup to mtrr-cleanup, disable_mtrr_cleanup to nomtrr-cleanup.

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 Documentation/kernel-parameters.txt |    4 ++--
 arch/x86/Kconfig                    |    2 +-
 arch/x86/kernel/cpu/mtrr/main.c     |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -614,8 +614,8 @@ and is between 256 and 4096 characters.
 			See drivers/char/README.epca and
 			Documentation/digiepca.txt.
 
-	disable_mtrr_cleanup [X86]
-	enable_mtrr_cleanup [X86]
+	nomtrr-cleanup [X86]
+	mtrr-cleanup [X86]
 			The kernel tries to adjust MTRR layout from continuous
 			to discrete, to make X server driver able to add WB
 			entry later. This parameter enables/disables that.
Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ linux-2.6/arch/x86/Kconfig
@@ -1250,7 +1250,7 @@ config MTRR_SANITIZER
 	  Convert MTRR layout from continuous to discrete, so X drivers can
 	  add writeback entries.
 
-	  Can be disabled with disable_mtrr_cleanup on the kernel command line.
+	  Can be disabled with nomtrr-cleanup on the kernel command line.
 	  The largest mtrr entry size for a continous block can be set with
 	  mtrr_chunk_size.
 
Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
@@ -826,7 +826,7 @@ static int __init disable_mtrr_cleanup_s
 		enable_mtrr_cleanup = 0;
 	return 0;
 }
-early_param("disable_mtrr_cleanup", disable_mtrr_cleanup_setup);
+early_param("nomtrr-cleanup", disable_mtrr_cleanup_setup);
 
 static int __init enable_mtrr_cleanup_setup(char *str)
 {
@@ -834,7 +834,7 @@ static int __init enable_mtrr_cleanup_se
 		enable_mtrr_cleanup = 1;
 	return 0;
 }
-early_param("enable_mtrr_cleanup", enable_mtrr_cleanup_setup);
+early_param("mtrr-cleanup", enable_mtrr_cleanup_setup);
 
 static int __init mtrr_cleanup_debug_setup(char *str)
 {

[PATCH 2/3] x86: doc mtrr-cleanup-debug

[Yinghai Lu]

doc mtrr-cleanup-debug.

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 Documentation/kernel-parameters.txt |    3 +++
 arch/x86/kernel/cpu/mtrr/main.c     |    2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -620,6 +620,9 @@ and is between 256 and 4096 characters.
 			to discrete, to make X server driver able to add WB
 			entry later. This parameter enables/disables that.
 
+	mtrr-cleanup-debug [X86]
+			print out more debug info for mtrr cleanup.
+
 	mtrr_chunk_size=nn[KMG] [X86]
 			used for mtrr cleanup. It is largest continous chunk
 			that could hold holes aka. UC entries.
Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
@@ -841,7 +841,7 @@ static int __init mtrr_cleanup_debug_set
 	debug_print = 1;
 	return 0;
 }
-early_param("mtrr_cleanup_debug", mtrr_cleanup_debug_setup);
+early_param("mtrr-cleanup-debug", mtrr_cleanup_debug_setup);
 
 struct var_mtrr_state {
 	unsigned long	range_startk;

[PATCH 3/3] x86: change MTRR_SANITIZER to def_bool y

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>

---
 arch/x86/Kconfig |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ linux-2.6/arch/x86/Kconfig
@@ -1243,7 +1243,7 @@ config MTRR
 	  See <file:Documentation/x86/mtrr.txt> for more information.
 
 config MTRR_SANITIZER
-	bool
+	def_bool y
 	prompt "MTRR cleanup support"
 	depends on MTRR
 	help
@@ -1254,7 +1254,7 @@ config MTRR_SANITIZER
 	  The largest mtrr entry size for a continous block can be set with
 	  mtrr_chunk_size.
 
-	  If unsure, say N.
+	  If unsure, say Y.
 
 config MTRR_SANITIZER_ENABLE_DEFAULT
 	int "MTRR cleanup enable value (0-1)"

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[H. Peter Anvin]

Randy Dunlap wrote:
> 
> E.g., the "apic" entries are not grouped together and these mtrr entries should
> not be grouped together unless they all begin with "mtrr", which is an option here:
> they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
> And "disable_mtrr_trim" could be renamed to "mtrr-notrim".
> 

That does collide with the (not always kept) convention of prefixing 
"no" to disable a boolean option, though.

	-hpa

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Yinghai Lu]

On Tue, Sep 30, 2008 at 4:41 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> Randy Dunlap wrote:
>>
>> E.g., the "apic" entries are not grouped together and these mtrr entries
>> should
>> not be grouped together unless they all begin with "mtrr", which is an
>> option here:
>> they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
>> And "disable_mtrr_trim" could be renamed to "mtrr-notrim".
>>
>
> That does collide with the (not always kept) convention of prefixing "no" to
> disable a boolean option, though.
>

it seems should group them and then provide one index section...

YH

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Randy Dunlap]

On Tue, 30 Sep 2008 16:29:38 -0700 Yinghai Lu wrote:

> change enable_mtrr_cleanup to mtrr-cleanup, disable_mtrr_cleanup to nomtrr-cleanup.
> 
> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> ---
>  Documentation/kernel-parameters.txt |    4 ++--
>  arch/x86/Kconfig                    |    2 +-
>  arch/x86/kernel/cpu/mtrr/main.c     |    4 ++--
>  3 files changed, 5 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/kernel-parameters.txt
> +++ linux-2.6/Documentation/kernel-parameters.txt
> @@ -614,8 +614,8 @@ and is between 256 and 4096 characters.
>  			See drivers/char/README.epca and
>  			Documentation/digiepca.txt.
>  
> -	disable_mtrr_cleanup [X86]
> -	enable_mtrr_cleanup [X86]
> +	nomtrr-cleanup [X86]
> +	mtrr-cleanup [X86]
>  			The kernel tries to adjust MTRR layout from continuous
>  			to discrete, to make X server driver able to add WB
>  			entry later. This parameter enables/disables that.

Looks like Documentation/kernel-parameters.txt needs a comment that says that
entries are supposed to be listed in alphabetical order, not grouped by <subject>.
Please don't add them like this.

E.g., the "apic" entries are not grouped together and these mtrr entries should
not be grouped together unless they all begin with "mtrr", which is an option here:
they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
And "disable_mtrr_trim" could be renamed to "mtrr-notrim".


> Index: linux-2.6/arch/x86/Kconfig
> ===================================================================
> --- linux-2.6.orig/arch/x86/Kconfig
> +++ linux-2.6/arch/x86/Kconfig
> @@ -1250,7 +1250,7 @@ config MTRR_SANITIZER
>  	  Convert MTRR layout from continuous to discrete, so X drivers can
>  	  add writeback entries.
>  
> -	  Can be disabled with disable_mtrr_cleanup on the kernel command line.
> +	  Can be disabled with nomtrr-cleanup on the kernel command line.
>  	  The largest mtrr entry size for a continous block can be set with
>  	  mtrr_chunk_size.
>  
> Index: linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/cpu/mtrr/main.c
> +++ linux-2.6/arch/x86/kernel/cpu/mtrr/main.c
> @@ -826,7 +826,7 @@ static int __init disable_mtrr_cleanup_s
>  		enable_mtrr_cleanup = 0;
>  	return 0;
>  }
> -early_param("disable_mtrr_cleanup", disable_mtrr_cleanup_setup);
> +early_param("nomtrr-cleanup", disable_mtrr_cleanup_setup);
>  
>  static int __init enable_mtrr_cleanup_setup(char *str)
>  {
> @@ -834,7 +834,7 @@ static int __init enable_mtrr_cleanup_se
>  		enable_mtrr_cleanup = 1;
>  	return 0;
>  }
> -early_param("enable_mtrr_cleanup", enable_mtrr_cleanup_setup);
> +early_param("mtrr-cleanup", enable_mtrr_cleanup_setup);
>  
>  static int __init mtrr_cleanup_debug_setup(char *str)
>  {


---
~Randy

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Yinghai Lu]

On Tue, Sep 30, 2008 at 4:52 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Sep 30, 2008 at 4:41 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> Randy Dunlap wrote:
>>>
>>> E.g., the "apic" entries are not grouped together and these mtrr entries
>>> should
>>> not be grouped together unless they all begin with "mtrr", which is an
>>> option here:
>>> they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
>>> And "disable_mtrr_trim" could be renamed to "mtrr-notrim".
>>>
>>
>> That does collide with the (not always kept) convention of prefixing "no" to
>> disable a boolean option, though.
>>
>
> it seems should group them and then provide one index section...
>
or
1. put all description in .c files
2. have one scripts to search early_param and __setup and create that
kernel_parameter...

YH

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Randy Dunlap]

On Tue, 30 Sep 2008 16:41:20 -0700 H. Peter Anvin wrote:

> Randy Dunlap wrote:
> > 
> > E.g., the "apic" entries are not grouped together and these mtrr entries should
> > not be grouped together unless they all begin with "mtrr", which is an option here:
> > they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
> > And "disable_mtrr_trim" could be renamed to "mtrr-notrim".
> > 
> 
> That does collide with the (not always kept) convention of prefixing 
> "no" to disable a boolean option, though.

True.  Then they just need to be listed in alpha order, please.

---
~Randy

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Randy Dunlap]

On Tue, 30 Sep 2008 16:59:49 -0700 Yinghai Lu wrote:

> On Tue, Sep 30, 2008 at 4:52 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Tue, Sep 30, 2008 at 4:41 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> >> Randy Dunlap wrote:
> >>>
> >>> E.g., the "apic" entries are not grouped together and these mtrr entries
> >>> should
> >>> not be grouped together unless they all begin with "mtrr", which is an
> >>> option here:
> >>> they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
> >>> And "disable_mtrr_trim" could be renamed to "mtrr-notrim".
> >>>
> >>
> >> That does collide with the (not always kept) convention of prefixing "no" to
> >> disable a boolean option, though.
> >>
> >
> > it seems should group them and then provide one index section...
> >
> or
> 1. put all description in .c files
> 2. have one scripts to search early_param and __setup and create that
> kernel_parameter...

Until that's done, please keep them in alpha order, like they (mostly)
are now.


---
~Randy

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Randy Dunlap]

On Tue, 30 Sep 2008 16:59:49 -0700 Yinghai Lu wrote:

> On Tue, Sep 30, 2008 at 4:52 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Tue, Sep 30, 2008 at 4:41 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> >> Randy Dunlap wrote:
> >>>
> >>> E.g., the "apic" entries are not grouped together and these mtrr entries
> >>> should
> >>> not be grouped together unless they all begin with "mtrr", which is an
> >>> option here:
> >>> they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
> >>> And "disable_mtrr_trim" could be renamed to "mtrr-notrim".
> >>>
> >>
> >> That does collide with the (not always kept) convention of prefixing "no" to
> >> disable a boolean option, though.
> >>
> >
> > it seems should group them and then provide one index section...
> >
> or
> 1. put all description in .c files
> 2. have one scripts to search early_param and __setup and create that
> kernel_parameter...

Here's a script that can be used as a starting point if someone wants to:
  http://www.xenotime.net/linux/scripts/module-params

---
~Randy

Re: [PATCH 3/3] x86: change MTRR_SANITIZER to def_bool y

[Ingo Molnar]

* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> 
> ---
>  arch/x86/Kconfig |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

applied the patch below to tip/x86/mtrr, thanks Yinghai!

(i'll wait for v2 of the mtrr parameter documentation patches.)

	Ingo

---------------------->
>From 2ffb3501f6f356ff80e7149214bc64d3fa9021c4 Mon Sep 17 00:00:00 2001
From: Yinghai Lu <yhlu.kernel@gmail.com>
Date: Tue, 30 Sep 2008 16:29:40 -0700
Subject: [PATCH] x86: change MTRR_SANITIZER to def_bool y

This option has been added in v2.6.26 as a default-disabled
feature and went through several revisions since then.

The feature fixes a wide range of MTRR setup problems that BIOSes
leave us with: slow system, slow Xorg, slow system when adding lots
of RAM, etc., so we want to enable it by default for v2.6.28.

See:

  [Bug 10508] Upgrade to 4GB of RAM messes up MTRRs
  http://bugzilla.kernel.org/show_bug.cgi?id=10508

and the test results in:

   http://lkml.org/lkml/2008/9/29/273

1. hpa
reg00: base=0xc0000000 (3072MB), size=1024MB: uncachable, count=1
reg01: base=0x13c000000 (5056MB), size=  64MB: uncachable, count=1
reg02: base=0x00000000 (   0MB), size=4096MB: write-back, count=1
reg03: base=0x100000000 (4096MB), size=1024MB: write-back, count=1
reg04: base=0xbf700000 (3063MB), size=   1MB: uncachable, count=1
reg05: base=0xbf800000 (3064MB), size=   8MB: uncachable, count=1

will get
Found optimal setting for mtrr clean up
gran_size: 1M   chunk_size: 128M        num_reg: 6      lose RAM: 0M
range0: 0000000000000000 - 00000000c0000000
Setting variable MTRR 0, base: 0MB, range: 2048MB, type WB
Setting variable MTRR 1, base: 2048MB, range: 1024MB, type WB
hole: 00000000bf700000 - 00000000c0000000
Setting variable MTRR 2, base: 3063MB, range: 1MB, type UC
Setting variable MTRR 3, base: 3064MB, range: 8MB, type UC
range0: 0000000100000000 - 0000000140000000
Setting variable MTRR 4, base: 4096MB, range: 1024MB, type WB
hole: 000000013c000000 - 0000000140000000
Setting variable MTRR 5, base: 5056MB, range: 64MB, type UC

2. Dylan Taft
reg00: base=0x00000000 (   0MB), size=4096MB: write-back, count=1
reg01: base=0x100000000 (4096MB), size= 512MB: write-back, count=1
reg02: base=0x120000000 (4608MB), size= 256MB: write-back, count=1
reg03: base=0xd0000000 (3328MB), size= 256MB: uncachable, count=1
reg04: base=0xe0000000 (3584MB), size= 512MB: uncachable, count=1
reg05: base=0xc7e00000 (3198MB), size=   2MB: uncachable, count=1
reg06: base=0xc8000000 (3200MB), size= 128MB: uncachable, count=1

will get
Found optimal setting for mtrr clean up
gran_size: 1M   chunk_size: 4M  num_reg: 6      lose RAM: 0M
range0: 0000000000000000 - 00000000c8000000
Setting variable MTRR 0, base: 0MB, range: 2048MB, type WB
Setting variable MTRR 1, base: 2048MB, range: 1024MB, type WB
Setting variable MTRR 2, base: 3072MB, range: 128MB, type WB
hole: 00000000c7e00000 - 00000000c8000000
Setting variable MTRR 3, base: 3198MB, range: 2MB, type UC
rangeX: 0000000100000000 - 0000000130000000
Setting variable MTRR 4, base: 4096MB, range: 512MB, type WB
Setting variable MTRR 5, base: 4608MB, range: 256MB, type WB

3. Gabriel
reg00: base=0xd0000000 (3328MB), size= 256MB: uncachable, count=1
reg01: base=0xe0000000 (3584MB), size= 512MB: uncachable, count=1
reg02: base=0x00000000 (   0MB), size=4096MB: write-back, count=1
reg03: base=0x100000000 (4096MB), size= 512MB: write-back, count=1
reg04: base=0x120000000 (4608MB), size= 128MB: write-back, count=1
reg05: base=0x128000000 (4736MB), size=  64MB: write-back, count=1
reg06: base=0xcf600000 (3318MB), size=   2MB: uncachable, count=1

will get
Found optimal setting for mtrr clean up
gran_size: 1M   chunk_size: 16M         num_reg: 7      lose RAM: 0M
range0: 0000000000000000 - 00000000d0000000
Setting variable MTRR 0, base: 0MB, range: 2048MB, type WB
Setting variable MTRR 1, base: 2048MB, range: 1024MB, type WB
Setting variable MTRR 2, base: 3072MB, range: 256MB, type WB
hole: 00000000cf600000 - 00000000cf800000
Setting variable MTRR 3, base: 3318MB, range: 2MB, type UC
rangeX: 0000000100000000 - 000000012c000000
Setting variable MTRR 4, base: 4096MB, range: 512MB, type WB
Setting variable MTRR 5, base: 4608MB, range: 128MB, type WB
Setting variable MTRR 6, base: 4736MB, range: 64MB, type WB

4. Mika Fischer
reg00: base=0xc0000000 (3072MB), size=1024MB: uncachable, count=1
reg01: base=0x00000000 ( 0MB), size=4096MB: write-back, count=1
reg02: base=0x100000000 (4096MB), size=1024MB: write-back, count=1
reg03: base=0xbf700000 (3063MB), size= 1MB: uncachable, count=1
reg04: base=0xbf800000 (3064MB), size= 8MB: uncachable, count=1

will get
Found optimal setting for mtrr clean up
gran_size: 1M   chunk_size: 16M         num_reg: 5      lose RAM: 0M
range0: 0000000000000000 - 00000000c0000000
Setting variable MTRR 0, base: 0MB, range: 2048MB, type WB
Setting variable MTRR 1, base: 2048MB, range: 1024MB, type WB
hole: 00000000bf700000 - 00000000c0000000
Setting variable MTRR 2, base: 3063MB, range: 1MB, type UC
Setting variable MTRR 3, base: 3064MB, range: 8MB, type UC
rangeX: 0000000100000000 - 0000000140000000
Setting variable MTRR 4, base: 4096MB, range: 1024MB, type WB

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/Kconfig |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index ed92864..09f6b7f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1120,7 +1120,7 @@ config MTRR
 	  See <file:Documentation/mtrr.txt> for more information.
 
 config MTRR_SANITIZER
-	bool
+	def_bool y
 	prompt "MTRR cleanup support"
 	depends on MTRR
 	help
@@ -1131,7 +1131,7 @@ config MTRR_SANITIZER
 	  The largest mtrr entry size for a continous block can be set with
 	  mtrr_chunk_size.
 
-	  If unsure, say N.
+	  If unsure, say Y.
 
 config MTRR_SANITIZER_ENABLE_DEFAULT
 	int "MTRR cleanup enable value (0-1)"

Re: [PATCH 3/3] x86: change MTRR_SANITIZER to def_bool y

[Yinghai Lu]

On Tue, Sep 30, 2008 at 11:36 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>
>> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
>>
>> ---
>>  arch/x86/Kconfig |    4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> applied the patch below to tip/x86/mtrr, thanks Yinghai!
>
> (i'll wait for v2 of the mtrr parameter documentation patches.)

mtrr-cleanup-debug, mtrr-clearnup, nomtrr-cleanup  - v2 the 1/3 and
2/3 in series...
or
mtrr_cleanup_debug, mtrr_clearnup, nomtrr_cleanup -V?
or
mtrrcleanup_debug, mtrrclearnup, nomtrrcleanup -v1

which one should we use ?

YH

Re: [PATCH 3/3] x86: change MTRR_SANITIZER to def_bool y

[J.A. Magallón]

On Tue, 30 Sep 2008 23:46:54 -0700, "Yinghai Lu" <yinghai@kernel.org> wrote:

> On Tue, Sep 30, 2008 at 11:36 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Yinghai Lu <yhlu.kernel@gmail.com> wrote:
> >
> >> Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> >>
> >> ---
> >>  arch/x86/Kconfig |    4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > applied the patch below to tip/x86/mtrr, thanks Yinghai!
> >
> > (i'll wait for v2 of the mtrr parameter documentation patches.)
> 
> mtrr-cleanup-debug, mtrr-clearnup, nomtrr-cleanup  - v2 the 1/3 and
> 2/3 in series...
> or
> mtrr_cleanup_debug, mtrr_clearnup, nomtrr_cleanup -V?
> or
> mtrrcleanup_debug, mtrrclearnup, nomtrrcleanup -v1
> 
> which one should we use ?
> 

My vote ;)

mtrr-cleanup (mtrr-sanitize ?)
mtrr-nocleanup (probably better mtrr-asis or mtrr-default ?)
mtrr-debug

And you could get all parameters with dashes instead of underlines:

mtrr_chunk_size   -> mtrr-chunk-size
mtrr_gran_size    -> mtrr-gran-size
mtrr_spare_reg_nr -> mtrr-spare-regs (mtrr-spares ?)
disable_mtrr_trim -> mtrr-notrim (mtrr-trim needed for symmetry ?)

All begin with mtrr- and alpha ordering groups them together :).

-- 
J.A. Magallon <jamagallon()ono!com>     \               Software is like sex:
                                         \         It's better when it's free
Mandriva Linux release 2009.0 (Cooker) for i586
Linux 2.6.25-jam18 (gcc 4.3.1 20080626 (GCC) #1 SMP

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Randy Dunlap]

On Tue, 30 Sep 2008 16:57:39 -0700 Randy Dunlap wrote:

> On Tue, 30 Sep 2008 16:29:38 -0700 Yinghai Lu wrote:
> 
> > change enable_mtrr_cleanup to mtrr-cleanup, disable_mtrr_cleanup to nomtrr-cleanup.
> > 
> > Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
> > 
> > ---
> >  Documentation/kernel-parameters.txt |    4 ++--
> >  arch/x86/Kconfig                    |    2 +-
> >  arch/x86/kernel/cpu/mtrr/main.c     |    4 ++--
> >  3 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > Index: linux-2.6/Documentation/kernel-parameters.txt
> > ===================================================================
> > --- linux-2.6.orig/Documentation/kernel-parameters.txt
> > +++ linux-2.6/Documentation/kernel-parameters.txt
> > @@ -614,8 +614,8 @@ and is between 256 and 4096 characters.
> >  			See drivers/char/README.epca and
> >  			Documentation/digiepca.txt.
> >  
> > -	disable_mtrr_cleanup [X86]
> > -	enable_mtrr_cleanup [X86]
> > +	nomtrr-cleanup [X86]
> > +	mtrr-cleanup [X86]
> >  			The kernel tries to adjust MTRR layout from continuous
> >  			to discrete, to make X server driver able to add WB
> >  			entry later. This parameter enables/disables that.
> 
> Looks like Documentation/kernel-parameters.txt needs a comment that says that
> entries are supposed to be listed in alphabetical order, not grouped by <subject>.
> Please don't add them like this.

BTW, this comment is alread in Documentation/kernel-parameters.txt, at the top of the file:

The following is a consolidated list of the kernel parameters as implemented
(mostly) by the __setup() macro and sorted into English Dictionary order
(defined as ignoring all punctuation and sorting digits before letters in a
case insensitive manner), and with descriptions where known.


> E.g., the "apic" entries are not grouped together and these mtrr entries should
> not be grouped together unless they all begin with "mtrr", which is an option here:
> they could be renamed to "mtrr-cleanup" and "mtrr-nocleanup".
> And "disable_mtrr_trim" could be renamed to "mtrr-notrim".

---
~Randy

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Yinghai Lu]

On Wed, Oct 8, 2008 at 5:07 PM, Randy Dunlap <rdunlap@xenotime.net> wrote:
> On Tue, 30 Sep 2008 16:57:39 -0700 Randy Dunlap wrote:
>
> BTW, this comment is alread in Documentation/kernel-parameters.txt, at the top of the file:

oh...

>
> The following is a consolidated list of the kernel parameters as implemented
> (mostly) by the __setup() macro and sorted into English Dictionary order
> (defined as ignoring all punctuation and sorting digits before letters in a
> case insensitive manner), and with descriptions where known.
>
>
how about

like to filter out disable, enable, no, etc...

diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 4b9ee9b..782e2b0 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -620,35 +620,6 @@ and is between 256 and 4096 characters. It is
defined in the file
                        See drivers/char/README.epca and
                        Documentation/digiepca.txt.

-       disable_mtrr_cleanup [X86]
-       enable_mtrr_cleanup [X86]
-                       The kernel tries to adjust MTRR layout from continuous
-                       to discrete, to make X server driver able to add WB
-                       entry later. This parameter enables/disables that.
-
-       mtrr_chunk_size=nn[KMG] [X86]
-                       used for mtrr cleanup. It is largest continous chunk
-                       that could hold holes aka. UC entries.
-
-       mtrr_gran_size=nn[KMG] [X86]
-                       Used for mtrr cleanup. It is granularity of mtrr block.
-                       Default is 1.
-                       Large value could prevent small alignment from
-                       using up MTRRs.
-
-       mtrr_spare_reg_nr=n [X86]
-                       Format: <integer>
-                       Range: 0,7 : spare reg number
-                       Default : 1
-                       Used for mtrr cleanup. It is spare mtrr entries number.
-                       Set to 2 or more if your graphical card needs more.
-
-       disable_mtrr_trim [X86, Intel and AMD only]
-                       By default the kernel will trim any uncacheable
-                       memory out of your available memory pool based on
-                       MTRR settings.  This parameter disables that behavior,
-                       possibly causing your machine to run very slowly.
-
        dmasound=       [HW,OSS] Sound subsystem buffers

        dscc4.setup=    [NET]
@@ -1310,6 +1281,35 @@ and is between 256 and 4096 characters. It is
defined in the file
                        [HW] Make the MicroTouch USB driver use raw coordinates
                        ('y', default) or cooked coordinates ('n')

+       disable_mtrr_cleanup [X86]
+       enable_mtrr_cleanup [X86]
+                       The kernel tries to adjust MTRR layout from continuous
+                       to discrete, to make X server driver able to add WB
+                       entry later. This parameter enables/disables that.
+
+       mtrr_chunk_size=nn[KMG] [X86]
+                       used for mtrr cleanup. It is largest continous chunk
+                       that could hold holes aka. UC entries.
+
+       mtrr_gran_size=nn[KMG] [X86]
+                       Used for mtrr cleanup. It is granularity of mtrr block.
+                       Default is 1.
+                       Large value could prevent small alignment from
+                       using up MTRRs.
+
+       mtrr_spare_reg_nr=n [X86]
+                       Format: <integer>
+                       Range: 0,7 : spare reg number
+                       Default : 1
+                       Used for mtrr cleanup. It is spare mtrr entries number.
+                       Set to 2 or more if your graphical card needs more.
+
+       disable_mtrr_trim [X86, Intel and AMD only]
+                       By default the kernel will trim any uncacheable
+                       memory out of your available memory pool based on
+                       MTRR settings.  This parameter disables that behavior,
+                       possibly causing your machine to run very slowly.
+
        n2=             [NET] SDL Inc. RISCom/N2 synchronous serial card

        NCR_D700=       [HW,SCSI]

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Randy Dunlap]

On Wed, 8 Oct 2008 18:28:39 -0700 Yinghai Lu wrote:

> On Wed, Oct 8, 2008 at 5:07 PM, Randy Dunlap <rdunlap@xenotime.net> wrote:
> > On Tue, 30 Sep 2008 16:57:39 -0700 Randy Dunlap wrote:
> >
> > BTW, this comment is alread in Documentation/kernel-parameters.txt, at the top of the file:
> 
> oh...
> 
> >
> > The following is a consolidated list of the kernel parameters as implemented
> > (mostly) by the __setup() macro and sorted into English Dictionary order
> > (defined as ignoring all punctuation and sorting digits before letters in a
> > case insensitive manner), and with descriptions where known.
> >
> >
> how about
> 
> like to filter out disable, enable, no, etc...

I don't think that mtrr gets any special treatment here.
All "acpi" parameters are not grouped together (unless they begin with
"acpi").  All "apic" parameters are not grouped together.
All "*iommu*" parameters are not grouped together....


> diff --git a/Documentation/kernel-parameters.txt
> b/Documentation/kernel-parameters.txt
> index 4b9ee9b..782e2b0 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -620,35 +620,6 @@ and is between 256 and 4096 characters. It is
> defined in the file
>                         See drivers/char/README.epca and
>                         Documentation/digiepca.txt.
> 
> -       disable_mtrr_cleanup [X86]
> -       enable_mtrr_cleanup [X86]
> -                       The kernel tries to adjust MTRR layout from continuous
> -                       to discrete, to make X server driver able to add WB
> -                       entry later. This parameter enables/disables that.
> -
> -       mtrr_chunk_size=nn[KMG] [X86]
> -                       used for mtrr cleanup. It is largest continous chunk
> -                       that could hold holes aka. UC entries.
> -
> -       mtrr_gran_size=nn[KMG] [X86]
> -                       Used for mtrr cleanup. It is granularity of mtrr block.
> -                       Default is 1.
> -                       Large value could prevent small alignment from
> -                       using up MTRRs.
> -
> -       mtrr_spare_reg_nr=n [X86]
> -                       Format: <integer>
> -                       Range: 0,7 : spare reg number
> -                       Default : 1
> -                       Used for mtrr cleanup. It is spare mtrr entries number.
> -                       Set to 2 or more if your graphical card needs more.
> -
> -       disable_mtrr_trim [X86, Intel and AMD only]
> -                       By default the kernel will trim any uncacheable
> -                       memory out of your available memory pool based on
> -                       MTRR settings.  This parameter disables that behavior,
> -                       possibly causing your machine to run very slowly.
> -
>         dmasound=       [HW,OSS] Sound subsystem buffers
> 
>         dscc4.setup=    [NET]
> @@ -1310,6 +1281,35 @@ and is between 256 and 4096 characters. It is
> defined in the file
>                         [HW] Make the MicroTouch USB driver use raw coordinates
>                         ('y', default) or cooked coordinates ('n')
> 
> +       disable_mtrr_cleanup [X86]
> +       enable_mtrr_cleanup [X86]
> +                       The kernel tries to adjust MTRR layout from continuous
> +                       to discrete, to make X server driver able to add WB
> +                       entry later. This parameter enables/disables that.
> +
> +       mtrr_chunk_size=nn[KMG] [X86]
> +                       used for mtrr cleanup. It is largest continous chunk
> +                       that could hold holes aka. UC entries.
> +
> +       mtrr_gran_size=nn[KMG] [X86]
> +                       Used for mtrr cleanup. It is granularity of mtrr block.
> +                       Default is 1.
> +                       Large value could prevent small alignment from
> +                       using up MTRRs.
> +
> +       mtrr_spare_reg_nr=n [X86]
> +                       Format: <integer>
> +                       Range: 0,7 : spare reg number
> +                       Default : 1
> +                       Used for mtrr cleanup. It is spare mtrr entries number.
> +                       Set to 2 or more if your graphical card needs more.
> +
> +       disable_mtrr_trim [X86, Intel and AMD only]
> +                       By default the kernel will trim any uncacheable
> +                       memory out of your available memory pool based on
> +                       MTRR settings.  This parameter disables that behavior,
> +                       possibly causing your machine to run very slowly.
> +
>         n2=             [NET] SDL Inc. RISCom/N2 synchronous serial card
> 
>         NCR_D700=       [HW,SCSI]
> 


---
~Randy

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[Yinghai Lu]

On Thu, Oct 9, 2008 at 10:20 AM, Randy Dunlap <rdunlap@xenotime.net> wrote:
> On Wed, 8 Oct 2008 18:28:39 -0700 Yinghai Lu wrote:
>
>> On Wed, Oct 8, 2008 at 5:07 PM, Randy Dunlap <rdunlap@xenotime.net> wrote:
>> > On Tue, 30 Sep 2008 16:57:39 -0700 Randy Dunlap wrote:
>> >
>> > BTW, this comment is alread in Documentation/kernel-parameters.txt, at the top of the file:
>>
>> oh...
>>
>> >
>> > The following is a consolidated list of the kernel parameters as implemented
>> > (mostly) by the __setup() macro and sorted into English Dictionary order
>> > (defined as ignoring all punctuation and sorting digits before letters in a
>> > case insensitive manner), and with descriptions where known.
>> >
>> >
>> how about
>>
>> like to filter out disable, enable, no, etc...
>
> I don't think that mtrr gets any special treatment here.
> All "acpi" parameters are not grouped together (unless they begin with
> "acpi").  All "apic" parameters are not grouped together.
> All "*iommu*" parameters are not grouped together....

could adjust acpi iommu to same layout

YH

Re: [PATCH 1/3] x86: mtrr_cleanup update command line

[H. Peter Anvin]

Yinghai Lu wrote:
>>>
>>> like to filter out disable, enable, no, etc...
>> I don't think that mtrr gets any special treatment here.
>> All "acpi" parameters are not grouped together (unless they begin with
>> "acpi").  All "apic" parameters are not grouped together.
>> All "*iommu*" parameters are not grouped together....
> 
> could adjust acpi iommu to same layout
> 

I think the point is that it shouldn't be...

	-hpa