All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bernhard Walle <bwalle@suse.de>
To: Oleg Verych <olecom@flower.upol.cz>
Cc: linux-arch@vger.kernel.org, akpm@linux-foundation.org,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 1/7] Extended crashkernel command line
Date: Wed, 26 Sep 2007 10:34:53 +0200	[thread overview]
Message-ID: <20070926083453.GE11385@suse.de> (raw)
In-Reply-To: <E1IaHPU-00089b-Bc@flower>

* 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.
  */

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

WARNING: multiple messages have this Message-ID (diff)
From: Bernhard Walle <bwalle@suse.de>
To: Oleg Verych <olecom@flower.upol.cz>
Cc: linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
	kexec@lists.infradead.org, akpm@linux-foundation.org
Subject: Re: [patch 1/7] Extended crashkernel command line
Date: Wed, 26 Sep 2007 10:34:53 +0200	[thread overview]
Message-ID: <20070926083453.GE11385@suse.de> (raw)
In-Reply-To: <E1IaHPU-00089b-Bc@flower>

* 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.
  */

  reply	other threads:[~2007-09-26  8:35 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-25 18:22 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
2007-09-25 18:22 ` Bernhard Walle
2007-09-25 18:22 ` [patch 1/7] Extended crashkernel command line Bernhard Walle
2007-09-25 18:22   ` Bernhard Walle
2007-09-25 20:53   ` Oleg Verych
2007-09-25 20:53     ` Oleg Verych
2007-09-26  8:34     ` Bernhard Walle [this message]
2007-09-26  8:34       ` Bernhard Walle
2007-09-26 16:16     ` Bernhard Walle
2007-09-26 16:16       ` Bernhard Walle
2007-09-26 18:18       ` Oleg Verych
2007-09-26 18:18         ` Oleg Verych
2007-09-26 18:18         ` Bernhard Walle
2007-09-26 18:18           ` Bernhard Walle
2007-09-26 21:05         ` Bernhard Walle
2007-09-26 21:05           ` Bernhard Walle
2007-09-26 21:32           ` reviewed (Re: [patch 1/7] Extended crashkernel command line) Oleg Verych
2007-09-26 21:32             ` Oleg Verych
2007-09-25 18:22 ` [patch 2/7] Use extended crashkernel command line on i386 Bernhard Walle
2007-09-25 18:22   ` Bernhard Walle
2007-09-25 18:23 ` [patch 3/7] Use extended crashkernel command line on x86_64 Bernhard Walle
2007-09-25 18:23   ` Bernhard Walle
2007-09-25 18:23 ` [patch 4/7] Use extended crashkernel command line on ia64 Bernhard Walle
2007-09-25 18:23   ` Bernhard Walle
2007-09-25 18:23   ` Bernhard Walle
2007-09-25 18:23 ` [patch 5/7] Use extended crashkernel command line on ppc64 Bernhard Walle
2007-09-25 18:23   ` Bernhard Walle
2007-09-25 18:23   ` Bernhard Walle
2007-09-25 19:45   ` Andrew Morton
2007-09-25 19:45     ` Andrew Morton
2007-09-25 19:45     ` Andrew Morton
2007-09-26  8:15     ` Bernhard Walle
2007-09-26  8:15       ` Bernhard Walle
2007-09-26  8:15       ` Bernhard Walle
2007-09-25 18:23 ` [patch 6/7] Use extended crashkernel command line on sh Bernhard Walle
2007-09-25 18:23   ` Bernhard Walle
2007-09-25 18:23 ` [patch 7/7] Add documentation for extended crashkernel syntax Bernhard Walle
2007-09-25 18:23   ` Bernhard Walle
  -- strict thread matches above, loose matches on Subject: below --
2007-09-20 17:18 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
2007-09-20 17:18 ` [patch 1/7] Extended crashkernel command line Bernhard Walle
2007-09-20 17:18   ` Bernhard Walle
2007-09-22 23:14   ` Oleg Verych
2007-09-22 23:14     ` Oleg Verych
2007-09-23 20:19     ` Bernhard Walle
2007-09-23 20:19       ` Bernhard Walle
2007-09-23 21:15       ` Oleg Verych
2007-09-23 21:15         ` Oleg Verych
2007-09-23 21:04         ` Bernhard Walle
2007-09-23 21:04           ` Bernhard Walle
2007-09-13 16:14 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
2007-09-13 16:14 ` [patch 1/7] Extended crashkernel command line Bernhard Walle
2007-09-13 16:14   ` Bernhard Walle
2007-09-19 22:32   ` Andrew Morton
2007-09-19 22:32     ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070926083453.GE11385@suse.de \
    --to=bwalle@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=kexec@lists.infradead.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=olecom@flower.upol.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.