[PATCH] Make extended crashkernel= syntax less confusing

[PATCH] Make extended crashkernel= syntax less confusing

[Michael Ellerman]

The extended crashkernel syntax is a little confusing in the
way it handles ranges. eg:

 crashkernel=512M-2G:64M,2G-:128M

Means if the machine has between 512M and 2G of memory the
crash region should be 64M, and if the machine has 2G of
memory the region should be 64M. Only if the machine has
more than 2G memory will 128M be allocated.

Although that semantic is correct, it is somewhat baffling.
Instead I propose that the end of the range means the first
address past the end of the range, ie: 512M up to but not
including 2G.

Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
Acked-by: Bernhard Walle <bwalle@suse.de>
---

 kernel/kexec.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 6782dce..0ab5e33 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1217,7 +1217,7 @@ static int __init parse_crashkernel_mem(char 			*cmdline,
 		}
 
 		/* match ? */
-		if (system_ram >= start && system_ram <= end) {
+		if (system_ram >= start && system_ram < end) {
 			*crash_size = size;
 			break;
 		}
-- 
1.5.5


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Make extended crashkernel= syntax less confusing

[Andrew Morton]

On Wed, 30 Apr 2008 11:36:27 +1000 (EST)
Michael Ellerman <michael@ellerman.id.au> wrote:

> The extended crashkernel syntax is a little confusing in the
> way it handles ranges. eg:
> 
>  crashkernel=512M-2G:64M,2G-:128M
> 
> Means if the machine has between 512M and 2G of memory the
> crash region should be 64M, and if the machine has 2G of
> memory the region should be 64M. Only if the machine has
> more than 2G memory will 128M be allocated.
> 
> Although that semantic is correct, it is somewhat baffling.
> Instead I propose that the end of the range means the first
> address past the end of the range, ie: 512M up to but not
> including 2G.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> Acked-by: Bernhard Walle <bwalle@suse.de>
> ---
> 
>  kernel/kexec.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 6782dce..0ab5e33 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -1217,7 +1217,7 @@ static int __init parse_crashkernel_mem(char 			*cmdline,
>  		}
>  
>  		/* match ? */
> -		if (system_ram >= start && system_ram <= end) {
> +		if (system_ram >= start && system_ram < end) {
>  			*crash_size = size;
>  			break;
>  		}

I'm a bit surprised to see the code being updated but not the
documentation.  Were they out of sync before or are they out of sync after
this patch or were they always in sync or what?


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Make extended crashkernel= syntax less confusing

[Bernhard Walle]

* Andrew Morton <akpm@linux-foundation.org> [2008-04-30 11:29]:
>
> I'm a bit surprised to see the code being updated but not the
> documentation.  Were they out of sync before or are they out of sync after
> this patch or were they always in sync or what?

It was simply not specified. I propose the following patch to make it
clear:

----

[PATCH] Clarify inclusive/exclusive in crashkernel commandline in documentation


Reflect the changes from Michael Ellerman in crashkernel syntax that make the
end of the memory exclusive in the documentation. It was simply not specified
before.


Signed-off-by: Bernhard Walle <bwalle@suse.de>

---
 Documentation/kdump/kdump.txt |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

--- a/Documentation/kdump/kdump.txt
+++ b/Documentation/kdump/kdump.txt
@@ -245,6 +245,8 @@ The syntax is:
     crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
     range=start-[end]
 
+    'start' is inclusive and 'end' is exclusive.
+
 For example:
 
     crashkernel=512M-2G:64M,2G-:128M
@@ -253,10 +255,11 @@ This would mean:
 
     1) if the RAM is smaller than 512M, then don't reserve anything
        (this is the "rescue" case)
-    2) if the RAM size is between 512M and 2G, then reserve 64M
+    2) if the RAM size is between 512M and 2G (exclusive), then reserve 64M
     3) if the RAM size is larger than 2G, then reserve 128M
 
 
+
 Boot into System Kernel
 =======================
 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Make extended crashkernel= syntax less confusing

[Simon Horman]

On Wed, Apr 30, 2008 at 11:57:26PM +0200, Bernhard Walle wrote:
> * Andrew Morton <akpm@linux-foundation.org> [2008-04-30 11:29]:
> >
> > I'm a bit surprised to see the code being updated but not the
> > documentation.  Were they out of sync before or are they out of sync after
> > this patch or were they always in sync or what?
> 
> It was simply not specified. I propose the following patch to make it
> clear:

This is fine by me, acked.

> ----
> 
> [PATCH] Clarify inclusive/exclusive in crashkernel commandline in documentation
> 
> 
> Reflect the changes from Michael Ellerman in crashkernel syntax that make the
> end of the memory exclusive in the documentation. It was simply not specified
> before.
> 
> 
> Signed-off-by: Bernhard Walle <bwalle@suse.de>
> 
> ---
>  Documentation/kdump/kdump.txt |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> --- a/Documentation/kdump/kdump.txt
> +++ b/Documentation/kdump/kdump.txt
> @@ -245,6 +245,8 @@ The syntax is:
>      crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
>      range=start-[end]
>  
> +    'start' is inclusive and 'end' is exclusive.
> +
>  For example:
>  
>      crashkernel=512M-2G:64M,2G-:128M
> @@ -253,10 +255,11 @@ This would mean:
>  
>      1) if the RAM is smaller than 512M, then don't reserve anything
>         (this is the "rescue" case)
> -    2) if the RAM size is between 512M and 2G, then reserve 64M
> +    2) if the RAM size is between 512M and 2G (exclusive), then reserve 64M
>      3) if the RAM size is larger than 2G, then reserve 128M
>  
>  
> +
>  Boot into System Kernel
>  =======================
>  
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

-- 
Horms


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] Make extended crashkernel= syntax less confusing

[Michael Ellerman]

[-- Attachment #1.1: Type: text/plain, Size: 2055 bytes --]

On Wed, 2008-04-30 at 11:29 -0700, Andrew Morton wrote:
> On Wed, 30 Apr 2008 11:36:27 +1000 (EST)
> Michael Ellerman <michael@ellerman.id.au> wrote:
> 
> > The extended crashkernel syntax is a little confusing in the
> > way it handles ranges. eg:
> > 
> >  crashkernel=512M-2G:64M,2G-:128M
> > 
> > Means if the machine has between 512M and 2G of memory the
> > crash region should be 64M, and if the machine has 2G of
> > memory the region should be 64M. Only if the machine has
> > more than 2G memory will 128M be allocated.
> > 
> > Although that semantic is correct, it is somewhat baffling.
> > Instead I propose that the end of the range means the first
> > address past the end of the range, ie: 512M up to but not
> > including 2G.
> > 
> > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > Acked-by: Bernhard Walle <bwalle@suse.de>
> > ---
> > 
> >  kernel/kexec.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/kernel/kexec.c b/kernel/kexec.c
> > index 6782dce..0ab5e33 100644
> > --- a/kernel/kexec.c
> > +++ b/kernel/kexec.c
> > @@ -1217,7 +1217,7 @@ static int __init parse_crashkernel_mem(char 			*cmdline,
> >  		}
> >  
> >  		/* match ? */
> > -		if (system_ram >= start && system_ram <= end) {
> > +		if (system_ram >= start && system_ram < end) {
> >  			*crash_size = size;
> >  			break;
> >  		}
> 
> I'm a bit surprised to see the code being updated but not the
> documentation.  Were they out of sync before or are they out of sync after
> this patch or were they always in sync or what?

Well I think my patch makes the code match the documentation, but it
depends on how you interpret "between xM and yM".

Bernhard's patch to make it explicit is a good idea though.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec