Re: [patch 1/7] Extended crashkernel command line

[Bernhard Walle <bwalle@suse.de>]

* Oleg Verych <olecom@flower.upol.cz> [2007-09-25 22:53]:
> * Tue, 25 Sep 2007 20:22:58 +0200
> >
> > +++ b/include/linux/kexec.h
> > @@ -187,6 +187,8 @@ extern u32 vmcoreinfo_note[VMCOREINFO_NO
> >  extern size_t vmcoreinfo_size;
> >  extern size_t vmcoreinfo_max_size;
> >  
> > +int __init parse_crashkernel(char *cmdline, unsigned long long system_ram,
> > +		unsigned long long *crash_size, unsigned long long *crash_base);
> 
> I still want to point out my consernes about `unsigned long long long'
> bloat.

What "concerns" (it's unsigned long long and not unsigned long long
long)? Is is common coding style in the Linux kernel to *not* use
unsigned long long? This type *is* used e.g. in
arch/i386/kernel/e820.c also for pysical memory values.

> #v+
> olecom@flower:/tmp$ grep '@offset' <extended-crashkernel-cmdline.mbox
>     crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
> -       /* crashkernel=size@offset specifies the size to reserve for a crash
> +While the "crashkernel=size[@offset]" syntax is sufficient for most
> +    crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
> +       crashkernel=range1:size1[,range2:size2,...][@offset]
> olecom@flower:/tmp$ grep '@base' <extended-crashkernel-cmdline.mbox
> + *     crashkernel=size[@base]
> olecom@flower:/tmp$ grep '@<base' <extended-crashkernel-cmdline.mbox
> + *   crashkernel=<ramsize-range>:<size>[,...][@<base>]
> olecom@flower:/tmp$ grep '@<offset' <extended-crashkernel-cmdline.mbox
> olecom@flower:/tmp$
> #v-

The patch below fixes this.

> > + *
> > + * The function returns 0 on success and -EINVAL on failure.
> > + */
> > +static int __init parse_crashkernel_mem(char 			*cmdline,
> > +					unsigned long long	system_ram,
> > +					unsigned long long	*crash_size,
> > +					unsigned long long	*crash_base)
> > +{
> > +	char *cur = cmdline;
> > +
> > +	/* for each entry of the comma-separated list */
> > +	do {
> > +		unsigned long long start = 0, end = ULLONG_MAX;
> > +		unsigned long long size = -1;
> 
> memparse(), as a wrapper for somple_strtoll(), always have a return value
> (zero by default).
> <http://article.gmane.org/gmane.linux.kernel/583426>

Next reply (because of a different patch).


---

Only use 'offset' and not 'base' for the address where the reserved area
starts.


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

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

--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1157,7 +1157,7 @@ module_init(crash_notes_memory_init)
 /*
  * This function parses command lines in the format
  *
- *   crashkernel=<ramsize-range>:<size>[,...][@<base>]
+ *   crashkernel=ramsize-range:size[,...][@offset]
  *
  * The function returns 0 on success and -EINVAL on failure.
  */
@@ -1222,7 +1222,7 @@ static int __init parse_crashkernel_mem(
 /*
  * That function parses "simple" (old) crashkernel command lines like
  *
- * 	crashkernel=size[@base]
+ * 	crashkernel=size[@offset]
  *
  * It returns 0 on success and -EINVAL on failure.
  */