* [patch 1/7] Extended crashkernel command line
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 20:53 ` Oleg Verych
2007-09-25 18:22 ` [patch 2/7] Use extended crashkernel command line on i386 Bernhard Walle
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Bernhard Walle @ 2007-09-25 18:22 UTC (permalink / raw)
To: kexec, akpm; +Cc: linux-kernel, linux-arch
[-- Attachment #1: crashkernel-generic --]
[-- Type: text/plain, Size: 4719 bytes --]
This is the generic part of the patch. It adds a parse_crashkernel() function
in kernel/kexec.c that is called by the architecture specific code that
actually reserves the memory. That function takes the whole command line and
looks itself for "crashkernel=" in it.
If there are multiple occurrences, then the last one is taken. The advantage
is that if you have a bootloader like lilo or elilo which allows you to append
a command line parameter but not to remove one (like in GRUB), then you can add
another crashkernel value for testing at the boot command line and this one
overwrites the command line in the configuration then.
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
include/linux/kexec.h | 2
kernel/kexec.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 144 insertions(+)
--- a/include/linux/kexec.h
+++ 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);
#else /* !CONFIG_KEXEC */
struct pt_regs;
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1146,6 +1146,148 @@ static int __init crash_notes_memory_ini
}
module_init(crash_notes_memory_init)
+
+/*
+ * parsing the "crashkernel" commandline
+ *
+ * this code is intended to be called from architecture specific code
+ */
+
+
+/*
+ * This function parses command lines in the format
+ *
+ * crashkernel=<ramsize-range>:<size>[,...][@<base>]
+ *
+ * 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;
+
+ /* get the start of the range */
+ start = memparse(cur, &cur);
+ if (*cur != '-') {
+ printk(KERN_WARNING "crashkernel: '-' expected\n");
+ return -EINVAL;
+ }
+ cur++;
+
+ /* if no ':' is here, than we read the end */
+ if (*cur != ':') {
+ end = memparse(cur, &cur);
+ if (end <= start) {
+ printk(KERN_WARNING "crashkernel: end <= start\n");
+ return -EINVAL;
+ }
+ }
+
+ if (*cur != ':') {
+ printk(KERN_WARNING "crashkernel: ':' expected\n");
+ return -EINVAL;
+ }
+ cur++;
+
+ size = memparse(cur, &cur);
+ if (size < 0) {
+ printk(KERN_WARNING "crashkernel: invalid size\n");
+ return -EINVAL;
+ }
+
+ /* match ? */
+ if (system_ram >= start && system_ram <= end) {
+ *crash_size = size;
+ break;
+ }
+ } while (*cur++ == ',');
+
+ if (*crash_size > 0) {
+ while (*cur != ' ' && *cur != '@')
+ cur++;
+ if (*cur == '@')
+ *crash_base = memparse(cur+1, &cur);
+ }
+
+ return 0;
+}
+
+/*
+ * That function parses "simple" (old) crashkernel command lines like
+ *
+ * crashkernel=size[@base]
+ *
+ * It returns 0 on success and -EINVAL on failure.
+ */
+static int __init parse_crashkernel_simple(char *cmdline,
+ unsigned long long *crash_size,
+ unsigned long long *crash_base)
+{
+ char *cur = cmdline;
+
+ *crash_size = memparse(cmdline, &cur);
+ if (cmdline == cur)
+ return -EINVAL;
+
+ if (*cur == '@')
+ *crash_base = memparse(cur+1, &cur);
+
+ return 0;
+}
+
+/*
+ * That function is the entry point for command line parsing and should be
+ * called from the arch-specific code.
+ */
+int __init parse_crashkernel(char *cmdline,
+ unsigned long long system_ram,
+ unsigned long long *crash_size,
+ unsigned long long *crash_base)
+{
+ char *p = cmdline, *ck_cmdline = NULL;
+ char *first_colon, *first_space;
+
+ BUG_ON(!crash_size || !crash_base);
+ *crash_size = 0;
+ *crash_base = 0;
+
+ /* find crashkernel and use the last one if there are more */
+ p = strstr(p, "crashkernel=");
+ while (p) {
+ ck_cmdline = p;
+ p = strstr(p+1, "crashkernel=");
+ }
+
+ if (!ck_cmdline)
+ return -EINVAL;
+
+ ck_cmdline += 12; /* strlen("crashkernel=") */
+
+ /*
+ * if the commandline contains a ':', then that's the extended
+ * syntax -- if not, it must be the classic syntax
+ */
+ first_colon = strchr(ck_cmdline, ':');
+ first_space = strchr(ck_cmdline, ' ');
+ if (first_colon && (!first_space || first_colon < first_space))
+ return parse_crashkernel_mem(ck_cmdline, system_ram,
+ crash_size, crash_base);
+ else
+ return parse_crashkernel_simple(ck_cmdline, crash_size,
+ crash_base);
+
+ return 0;
+}
+
+
+
void crash_save_vmcoreinfo(void)
{
u32 *buf;
--
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 1/7] Extended crashkernel command line
2007-09-25 18:22 ` [patch 1/7] Extended crashkernel command line Bernhard Walle
@ 2007-09-25 20:53 ` Oleg Verych
2007-09-26 8:34 ` Bernhard Walle
2007-09-26 16:16 ` Bernhard Walle
0 siblings, 2 replies; 21+ messages in thread
From: Oleg Verych @ 2007-09-25 20:53 UTC (permalink / raw)
To: Bernhard Walle; +Cc: linux-kernel, linux-arch, kexec, akpm
* Tue, 25 Sep 2007 20:22:58 +0200
>
> This is the generic part of the patch. It adds a parse_crashkernel() function
> in kernel/kexec.c that is called by the architecture specific code that
> actually reserves the memory. That function takes the whole command line and
> looks itself for "crashkernel=" in it.
[]
> +++ 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.
> #else /* !CONFIG_KEXEC */
> struct pt_regs;
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
[]
> +/*
> + * This function parses command lines in the format
> + *
> + * crashkernel=<ramsize-range>:<size>[,...][@<base>]
#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 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>
Consider coded error path now, please.
And why not to make overall result reliable? This is kernel after all.
I.e. if there's valid `crashkernel=' option, but some parsing errors, why
not to apply some heuristics with warning in syslog, if user have some
conf, bootloader (random) errors, but feature still works?
> +
> + /* get the start of the range */
> + start = memparse(cur, &cur);
> + if (*cur != '-') {
> + printk(KERN_WARNING "crashkernel: '-' expected\n");
> + return -EINVAL;
> + }
> + cur++;
> +
> + /* if no ':' is here, than we read the end */
> + if (*cur != ':') {
> + end = memparse(cur, &cur);
> + if (end <= start) {
> + printk(KERN_WARNING "crashkernel: end <= start\n");
> + return -EINVAL;
> + }
> + }
> +
> + if (*cur != ':') {
> + printk(KERN_WARNING "crashkernel: ':' expected\n");
> + return -EINVAL;
> + }
> + cur++;
> +
> + size = memparse(cur, &cur);
> + if (size < 0) {
> + printk(KERN_WARNING "crashkernel: invalid size\n");
> + return -EINVAL;
> + }
> +
> + /* match ? */
> + if (system_ram >= start && system_ram <= end) {
> + *crash_size = size;
> + break;
> + }
> + } while (*cur++ == ',');
> +
> + if (*crash_size > 0) {
> + while (*cur != ' ' && *cur != '@')
> + cur++;
> + if (*cur == '@')
> + *crash_base = memparse(cur+1, &cur);
> + }
> +
> + return 0;
> +}
--
-o--=O`C
#oo'L O
<___=E M
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 1/7] Extended crashkernel command line
2007-09-25 20:53 ` Oleg Verych
@ 2007-09-26 8:34 ` Bernhard Walle
2007-09-26 16:16 ` Bernhard Walle
1 sibling, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-26 8:34 UTC (permalink / raw)
To: Oleg Verych; +Cc: linux-kernel, linux-arch, kexec, akpm
* 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.
*/
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 1/7] Extended crashkernel command line
2007-09-25 20:53 ` Oleg Verych
2007-09-26 8:34 ` Bernhard Walle
@ 2007-09-26 16:16 ` Bernhard Walle
2007-09-26 18:18 ` Oleg Verych
1 sibling, 1 reply; 21+ messages in thread
From: Bernhard Walle @ 2007-09-26 16:16 UTC (permalink / raw)
To: Oleg Verych; +Cc: linux-kernel, linux-arch, kexec, akpm
* Oleg Verych <olecom@flower.upol.cz> [2007-09-25 22:53]:
>
> > + *
> > + * 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>
Ok, that's fixed now, see patch below.
> And why not to make overall result reliable? This is kernel after all.
>
> I.e. if there's valid `crashkernel=' option, but some parsing errors, why
> not to apply some heuristics with warning in syslog, if user have some
> conf, bootloader (random) errors, but feature still works?
I'm against guessing. The user has to specify a parameter which is
right according to syntax.
However, I plan to make a patch that the kernel can detect a sensible
offset automatically for i386 and x86_64 as it's done in ia64. Since
both architectures have a relocatable kernel now, that makes perfectly
sense. But that's another patch.
---
This patches improves error handling in parse_crashkernel_mem() by comparing
the return pointer of memparse() with the input pointer and also replaces
all printk(KERN_WARNING msg) with pr_warning(msg).
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
kernel/kexec.c | 31 ++++++++++++++++++++++++-------
1 file changed, 24 insertions(+), 7 deletions(-)
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1172,33 +1172,50 @@ static int __init parse_crashkernel_mem(
do {
unsigned long long start = 0, end = ULLONG_MAX;
unsigned long long size = -1;
+ char *tmp;
/* get the start of the range */
- start = memparse(cur, &cur);
+ start = memparse(cur, &tmp);
+ if (cur == tmp) {
+ pr_warning("crashkernel: Memory value expected\n");
+ return -EINVAL;
+ }
+ cur = tmp;
if (*cur != '-') {
- printk(KERN_WARNING "crashkernel: '-' expected\n");
+ pr_warning("crashkernel: '-' expected\n");
return -EINVAL;
}
cur++;
/* if no ':' is here, than we read the end */
if (*cur != ':') {
- end = memparse(cur, &cur);
+ end = memparse(cur, &tmp);
+ if (cur == tmp) {
+ pr_warning("crashkernel: Memory "
+ "value expected\n");
+ return -EINVAL;
+ }
+ cur = tmp;
if (end <= start) {
- printk(KERN_WARNING "crashkernel: end <= start\n");
+ pr_warning("crashkernel: end <= start\n");
return -EINVAL;
}
}
if (*cur != ':') {
- printk(KERN_WARNING "crashkernel: ':' expected\n");
+ pr_warning("crashkernel: ':' expected\n");
return -EINVAL;
}
cur++;
- size = memparse(cur, &cur);
+ size = memparse(cur, &tmp);
+ if (cur == tmp) {
+ pr_warning("Memory value expected\n");
+ return -EINVAL;
+ }
+ cur = tmp;
if (size < 0) {
- printk(KERN_WARNING "crashkernel: invalid size\n");
+ pr_warning("crashkernel: invalid size\n");
return -EINVAL;
}
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 1/7] Extended crashkernel command line
2007-09-26 16:16 ` Bernhard Walle
@ 2007-09-26 18:18 ` Oleg Verych
2007-09-26 18:18 ` Bernhard Walle
2007-09-26 21:05 ` Bernhard Walle
0 siblings, 2 replies; 21+ messages in thread
From: Oleg Verych @ 2007-09-26 18:18 UTC (permalink / raw)
To: Bernhard Walle (in kexec list), akpm; +Cc: linux-kernel, linux-arch
Wed, Sep 26, 2007 at 06:16:02PM +0200, Bernhard Walle (part two, see bottom):
> > memparse(), as a wrapper for somple_strtoll(), always have a return value
> > (zero by default).
> > <http://article.gmane.org/gmane.linux.kernel/583426>
Sorry for my typos, i should write `simple_strtoull()'. This function
(ULL from str) have always return value grater or equal to zero.
Thus,
>
> Signed-off-by: Bernhard Walle <bwalle@suse.de>
>
> ---
> kernel/kexec.c | 31 ++++++++++++++++++++++++-------
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -1172,33 +1172,50 @@ static int __init parse_crashkernel_mem(
> do {
> unsigned long long start = 0, end = ULLONG_MAX;
> unsigned long long size = -1;
no need in assigning values here, unless you plan to use them in case
of `return -EINVAL', but i can not see that,
> + char *tmp;
>
> /* get the start of the range */
> - start = memparse(cur, &cur);
> + start = memparse(cur, &tmp);
> + if (cur == tmp) {
> + pr_warning("crashkernel: Memory value expected\n");
> + return -EINVAL;
> + }
> + cur = tmp;
> if (*cur != '-') {
> - printk(KERN_WARNING "crashkernel: '-' expected\n");
> + pr_warning("crashkernel: '-' expected\n");
> return -EINVAL;
> }
> cur++;
>
> /* if no ':' is here, than we read the end */
> if (*cur != ':') {
> - end = memparse(cur, &cur);
> + end = memparse(cur, &tmp);
> + if (cur == tmp) {
> + pr_warning("crashkernel: Memory "
> + "value expected\n");
> + return -EINVAL;
> + }
> + cur = tmp;
> if (end <= start) {
> - printk(KERN_WARNING "crashkernel: end <= start\n");
> + pr_warning("crashkernel: end <= start\n");
> return -EINVAL;
> }
> }
>
> if (*cur != ':') {
> - printk(KERN_WARNING "crashkernel: ':' expected\n");
> + pr_warning("crashkernel: ':' expected\n");
> return -EINVAL;
> }
> cur++;
>
> - size = memparse(cur, &cur);
> + size = memparse(cur, &tmp);
> + if (cur == tmp) {
> + pr_warning("Memory value expected\n");
> + return -EINVAL;
> + }
> + cur = tmp;
> if (size < 0) {
`size' cannot be less that zero here (wonder, if it matters to have
userspace model of this parser and actually feed it with garbage input).
> - printk(KERN_WARNING "crashkernel: invalid size\n");
> + pr_warning("crashkernel: invalid size\n");
> return -EINVAL;
> }
>
Wed, Sep 26, 2007 at 06:16:02PM +0200, Bernhard Walle (part one):
> Ok, that's fixed now, see patch below.
>
> > And why not to make overall result reliable? This is kernel after all.
> >
> > I.e. if there's valid `crashkernel=' option, but some parsing errors, why
> > not to apply some heuristics with warning in syslog, if user have some
> > conf, bootloader (random) errors, but feature still works?
>
> I'm against guessing. The user has to specify a parameter which is
> right according to syntax.
>
> However, I plan to make a patch that the kernel can detect a sensible
> offset automatically for i386 and x86_64 as it's done in ia64. Since
> both architectures have a relocatable kernel now, that makes perfectly
> sense. But that's another patch.
I was thinking about errors in YaST or typos in bootloader config, that
may appear sometimes. And kernel must tolerate this kind of userspace
input to be more reliable. But you know better, i just am waving hands.
____
("Mail-Followup-To:" respected)
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 1/7] Extended crashkernel command line
2007-09-26 18:18 ` Oleg Verych
@ 2007-09-26 18:18 ` Bernhard Walle
2007-09-26 21:05 ` Bernhard Walle
1 sibling, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-26 18:18 UTC (permalink / raw)
To: Oleg Verych
Cc: Bernhard Walle (in kexec list), akpm, linux-kernel, linux-arch
* Oleg Verych <olecom@flower.upol.cz> [2007-09-26 20:18]:
>
> I was thinking about errors in YaST or typos in bootloader config, that
> may appear sometimes. And kernel must tolerate this kind of userspace
> input to be more reliable. But you know better, i just am waving hands.
Of course the kernel must be able to handle them -- outputting an
error message that can be read by the user and not crashing. But I
don't expect that the kernel then reservate crash memory by guessing
of values.
Thanks,
Bernhard
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [patch 1/7] Extended crashkernel command line
2007-09-26 18:18 ` Oleg Verych
2007-09-26 18:18 ` 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
1 sibling, 1 reply; 21+ messages in thread
From: Bernhard Walle @ 2007-09-26 21:05 UTC (permalink / raw)
To: Oleg Verych
Cc: Bernhard Walle (in kexec list), akpm, linux-kernel, linux-arch
* Oleg Verych <olecom@flower.upol.cz> [2007-09-26 20:18]:
> >
> > --- a/kernel/kexec.c
> > +++ b/kernel/kexec.c
> > @@ -1172,33 +1172,50 @@ static int __init parse_crashkernel_mem(
> > do {
> > unsigned long long start = 0, end = ULLONG_MAX;
> > unsigned long long size = -1;
>
> no need in assigning values here, unless you plan to use them in case
> of `return -EINVAL', but i can not see that,
What about this (and yes, I tested with some wrong strings with Qemu):
----
This patch improves error handling in parse_crashkernel_mem() by comparing
the return pointer of memparse() with the input pointer and also replaces
all printk(KERN_WARNING msg) with pr_warning(msg).
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
kernel/kexec.c | 54 +++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 15 deletions(-)
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1167,44 +1167,59 @@ static int __init parse_crashkernel_mem(
unsigned long long *crash_size,
unsigned long long *crash_base)
{
- char *cur = cmdline;
+ char *cur = cmdline, *tmp;
/* for each entry of the comma-separated list */
do {
- unsigned long long start = 0, end = ULLONG_MAX;
- unsigned long long size = -1;
+ unsigned long long start, end = ULLONG_MAX, size;
/* get the start of the range */
- start = memparse(cur, &cur);
+ start = memparse(cur, &tmp);
+ if (cur == tmp) {
+ pr_warning("crashkernel: Memory value expected\n");
+ return -EINVAL;
+ }
+ cur = tmp;
if (*cur != '-') {
- printk(KERN_WARNING "crashkernel: '-' expected\n");
+ pr_warning("crashkernel: '-' expected\n");
return -EINVAL;
}
cur++;
/* if no ':' is here, than we read the end */
if (*cur != ':') {
- end = memparse(cur, &cur);
+ end = memparse(cur, &tmp);
+ if (cur == tmp) {
+ pr_warning("crashkernel: Memory "
+ "value expected\n");
+ return -EINVAL;
+ }
+ cur = tmp;
if (end <= start) {
- printk(KERN_WARNING "crashkernel: end <= start\n");
+ pr_warning("crashkernel: end <= start\n");
return -EINVAL;
}
}
if (*cur != ':') {
- printk(KERN_WARNING "crashkernel: ':' expected\n");
+ pr_warning("crashkernel: ':' expected\n");
return -EINVAL;
}
cur++;
- size = memparse(cur, &cur);
- if (size < 0) {
- printk(KERN_WARNING "crashkernel: invalid size\n");
+ size = memparse(cur, &tmp);
+ if (cur == tmp) {
+ pr_warning("Memory value expected\n");
+ return -EINVAL;
+ }
+ cur = tmp;
+ if (size >= system_ram) {
+ pr_warning("crashkernel: invalid size\n");
return -EINVAL;
}
/* match ? */
- if (system_ram >= start && system_ram <= end) {
+ if (system_ram >= start && system_ram <= end) {
*crash_size = size;
break;
}
@@ -1213,8 +1228,15 @@ static int __init parse_crashkernel_mem(
if (*crash_size > 0) {
while (*cur != ' ' && *cur != '@')
cur++;
- if (*cur == '@')
- *crash_base = memparse(cur+1, &cur);
+ if (*cur == '@') {
+ cur++;
+ *crash_base = memparse(cur, &tmp);
+ if (cur == tmp) {
+ pr_warning("Memory value expected "
+ "after '@'\n");
+ return -EINVAL;
+ }
+ }
}
return 0;
@@ -1234,8 +1256,10 @@ static int __init parse_crashkernel_simp
char *cur = cmdline;
*crash_size = memparse(cmdline, &cur);
- if (cmdline == cur)
+ if (cmdline == cur) {
+ pr_warning("crashkernel: memory value expected\n");
return -EINVAL;
+ }
if (*cur == '@')
*crash_base = memparse(cur+1, &cur);
^ permalink raw reply [flat|nested] 21+ messages in thread* reviewed (Re: [patch 1/7] Extended crashkernel command line)
2007-09-26 21:05 ` Bernhard Walle
@ 2007-09-26 21:32 ` Oleg Verych
0 siblings, 0 replies; 21+ messages in thread
From: Oleg Verych @ 2007-09-26 21:32 UTC (permalink / raw)
To: Bernhard Walle (in kexec list), akpm, linux-kernel, linux-arch
Wed, Sep 26, 2007 at 11:05:33PM +0200, Bernhard Walle:
> * Oleg Verych <olecom@flower.upol.cz> [2007-09-26 20:18]:
> > >
> > > --- a/kernel/kexec.c
> > > +++ b/kernel/kexec.c
> > > @@ -1172,33 +1172,50 @@ static int __init parse_crashkernel_mem(
> > > do {
> > > unsigned long long start = 0, end = ULLONG_MAX;
> > > unsigned long long size = -1;
> >
> > no need in assigning values here, unless you plan to use them in case
> > of `return -EINVAL', but i can not see that,
>
> What about this (and yes, I tested with some wrong strings with Qemu):
Reviewed-by: Oleg Verych <olecom@flower.upol.cz>
Thanks :D
> ----
>
> This patch improves error handling in parse_crashkernel_mem() by comparing
> the return pointer of memparse() with the input pointer and also replaces
> all printk(KERN_WARNING msg) with pr_warning(msg).
>
>
> Signed-off-by: Bernhard Walle <bwalle@suse.de>
_____
^ permalink raw reply [flat|nested] 21+ messages in thread
* [patch 2/7] Use extended crashkernel command line on i386
2007-09-25 18:22 [patch 0/7] Add extended crashkernel command line syntax 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 18:23 ` [patch 3/7] Use extended crashkernel command line on x86_64 Bernhard Walle
` (4 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-25 18:22 UTC (permalink / raw)
To: kexec, akpm; +Cc: linux-kernel, linux-arch
[-- Attachment #1: crashkernel-i386 --]
[-- Type: text/plain, Size: 3605 bytes --]
This patch removes the crashkernel parsing from
arch/i386/kernel/machine_kexec.c and calls the generic function, introduced in
the last patch, in setup_bootmem_allocator().
This is necessary because the amount of System RAM must be known in this
function now because of the new syntax.
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
arch/i386/kernel/e820.c | 3 +-
arch/i386/kernel/machine_kexec.c | 22 -----------------
arch/i386/kernel/setup.c | 49 +++++++++++++++++++++++++++++++++++----
3 files changed, 46 insertions(+), 28 deletions(-)
Files a/arch/i386/kernel/.setup.c.swp and b/arch/i386/kernel/.setup.c.swp differ
--- a/arch/i386/kernel/e820.c
+++ b/arch/i386/kernel/e820.c
@@ -288,7 +288,8 @@ legacy_init_iomem_resources(struct resou
request_resource(res, code_resource);
request_resource(res, data_resource);
#ifdef CONFIG_KEXEC
- request_resource(res, &crashk_res);
+ if (crashk_res.start != crashk_res.end)
+ request_resource(res, &crashk_res);
#endif
}
}
--- a/arch/i386/kernel/machine_kexec.c
+++ b/arch/i386/kernel/machine_kexec.c
@@ -149,28 +149,6 @@ NORET_TYPE void machine_kexec(struct kim
image->start, cpu_has_pae);
}
-/* crashkernel=size@addr specifies the location to reserve for
- * a crash kernel. By reserving this memory we guarantee
- * that linux never sets it up as a DMA target.
- * Useful for holding code to do something appropriate
- * after a kernel panic.
- */
-static int __init parse_crashkernel(char *arg)
-{
- unsigned long size, base;
- size = memparse(arg, &arg);
- if (*arg == '@') {
- base = memparse(arg+1, &arg);
- /* FIXME: Do I want a sanity check
- * to validate the memory range?
- */
- crashk_res.start = base;
- crashk_res.end = base + size - 1;
- }
- return 0;
-}
-early_param("crashkernel", parse_crashkernel);
-
void arch_crash_save_vmcoreinfo(void)
{
#ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
--- a/arch/i386/kernel/setup.c
+++ b/arch/i386/kernel/setup.c
@@ -381,6 +381,49 @@ extern unsigned long __init setup_memory
extern void zone_sizes_init(void);
#endif /* !CONFIG_NEED_MULTIPLE_NODES */
+static inline unsigned long long get_total_mem(void)
+{
+ unsigned long long total;
+
+ total = max_low_pfn - min_low_pfn;
+#ifdef CONFIG_HIGHMEM
+ total += highend_pfn - highstart_pfn;
+#endif
+
+ return total << PAGE_SHIFT;
+}
+
+#ifdef CONFIG_KEXEC
+static void __init reserve_crashkernel(void)
+{
+ unsigned long long total_mem;
+ unsigned long long crash_size, crash_base;
+ int ret;
+
+ total_mem = get_total_mem();
+
+ ret = parse_crashkernel(boot_command_line, total_mem,
+ &crash_size, &crash_base);
+ if (ret == 0 && crash_size > 0) {
+ if (crash_base > 0) {
+ printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
+ "for crashkernel (System RAM: %ldMB)\n",
+ (unsigned long)(crash_size >> 20),
+ (unsigned long)(crash_base >> 20),
+ (unsigned long)(total_mem >> 20));
+ crashk_res.start = crash_base;
+ crashk_res.end = crash_base + crash_size - 1;
+ reserve_bootmem(crash_base, crash_size);
+ } else
+ printk(KERN_INFO "crashkernel reservation failed - "
+ "you have to specify a base address\n");
+ }
+}
+#else
+static inline void __init reserve_crashkernel(void)
+{}
+#endif
+
void __init setup_bootmem_allocator(void)
{
unsigned long bootmap_size;
@@ -456,11 +499,7 @@ void __init setup_bootmem_allocator(void
}
}
#endif
-#ifdef CONFIG_KEXEC
- if (crashk_res.start != crashk_res.end)
- reserve_bootmem(crashk_res.start,
- crashk_res.end - crashk_res.start + 1);
-#endif
+ reserve_crashkernel();
}
/*
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 3/7] Use extended crashkernel command line on x86_64
2007-09-25 18:22 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
2007-09-25 18:22 ` [patch 1/7] Extended crashkernel command line Bernhard Walle
2007-09-25 18:22 ` [patch 2/7] Use extended crashkernel command line on i386 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
` (3 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-25 18:23 UTC (permalink / raw)
To: kexec, akpm; +Cc: linux-kernel, linux-arch, discuss
[-- Attachment #1: crashkernel-x86_64 --]
[-- Type: text/plain, Size: 3424 bytes --]
This patch removes the crashkernel parsing from
arch/x86_64/kernel/machine_kexec.c and calls the generic function, introduced in
the last patch, in setup_bootmem_allocator().
This is necessary because the amount of System RAM must be known in this
function now because of the new syntax.
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
arch/x86_64/kernel/e820.c | 3 +-
arch/x86_64/kernel/machine_kexec.c | 27 -------------------------
arch/x86_64/kernel/setup.c | 39 ++++++++++++++++++++++++++++++-------
3 files changed, 34 insertions(+), 35 deletions(-)
--- a/arch/x86_64/kernel/e820.c
+++ b/arch/x86_64/kernel/e820.c
@@ -226,7 +226,8 @@ void __init e820_reserve_resources(void)
request_resource(res, &code_resource);
request_resource(res, &data_resource);
#ifdef CONFIG_KEXEC
- request_resource(res, &crashk_res);
+ if (crashk_res.start != crashk_res.end)
+ request_resource(res, &crashk_res);
#endif
}
}
--- a/arch/x86_64/kernel/machine_kexec.c
+++ b/arch/x86_64/kernel/machine_kexec.c
@@ -231,33 +231,6 @@ NORET_TYPE void machine_kexec(struct kim
image->start);
}
-/* crashkernel=size@addr specifies the location to reserve for
- * a crash kernel. By reserving this memory we guarantee
- * that linux never set's it up as a DMA target.
- * Useful for holding code to do something appropriate
- * after a kernel panic.
- */
-static int __init setup_crashkernel(char *arg)
-{
- unsigned long size, base;
- char *p;
- if (!arg)
- return -EINVAL;
- size = memparse(arg, &p);
- if (arg == p)
- return -EINVAL;
- if (*p == '@') {
- base = memparse(p+1, &p);
- /* FIXME: Do I want a sanity check to validate the
- * memory range? Yes you do, but it's too early for
- * e820 -AK */
- crashk_res.start = base;
- crashk_res.end = base + size - 1;
- }
- return 0;
-}
-early_param("crashkernel", setup_crashkernel);
-
void arch_crash_save_vmcoreinfo(void)
{
#ifdef CONFIG_ARCH_DISCONTIGMEM_ENABLE
--- a/arch/x86_64/kernel/setup.c
+++ b/arch/x86_64/kernel/setup.c
@@ -194,6 +194,37 @@ static inline void copy_edd(void)
}
#endif
+#ifdef CONFIG_KEXEC
+static void __init reserve_crashkernel(void)
+{
+ unsigned long long free_mem;
+ unsigned long long crash_size, crash_base;
+ int ret;
+
+ free_mem = ((unsigned long long)max_low_pfn - min_low_pfn) << PAGE_SHIFT;
+
+ ret = parse_crashkernel(boot_command_line, free_mem,
+ &crash_size, &crash_base);
+ if (ret == 0 && crash_size) {
+ if (crash_base > 0) {
+ printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
+ "for crashkernel (System RAM: %ldMB)\n",
+ (unsigned long)(crash_size >> 20),
+ (unsigned long)(crash_base >> 20),
+ (unsigned long)(free_mem >> 20));
+ crashk_res.start = crash_base;
+ crashk_res.end = crash_base + crash_size - 1;
+ reserve_bootmem(crash_base, crash_size);
+ } else
+ printk(KERN_INFO "crashkernel reservation failed - "
+ "you have to specify a base address\n");
+ }
+}
+#else
+static inline void __init reserve_crashkernel(void)
+{}
+#endif
+
#define EBDA_ADDR_POINTER 0x40E
unsigned __initdata ebda_addr;
@@ -365,13 +396,7 @@ void __init setup_arch(char **cmdline_p)
}
}
#endif
-#ifdef CONFIG_KEXEC
- if (crashk_res.start != crashk_res.end) {
- reserve_bootmem_generic(crashk_res.start,
- crashk_res.end - crashk_res.start + 1);
- }
-#endif
-
+ reserve_crashkernel();
paging_init();
#ifdef CONFIG_PCI
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 4/7] Use extended crashkernel command line on ia64
2007-09-25 18:22 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
` (2 preceding siblings ...)
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 5/7] Use extended crashkernel command line on ppc64 Bernhard Walle
` (2 subsequent siblings)
6 siblings, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-25 18:23 UTC (permalink / raw)
To: kexec, akpm; +Cc: linux-kernel, linux-arch, linux-ia64
[-- Attachment #1: crashkernel-ia64 --]
[-- Type: text/plain, Size: 4984 bytes --]
This patch adapts IA64 to use the generic parse_crashkernel() function
instead of its own parsing for the crashkernel command line.
Because the total amount of System RAM must be known when calling this
function, efi_memmap_init() is modified to return its accumulated total_memory
variable.
Also, the crashkernel handling is moved in an own function in
arch/ia64/kernel/setup.c to make the code more readable.
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
arch/ia64/kernel/efi.c | 4 +-
arch/ia64/kernel/setup.c | 88 +++++++++++++++++++++++----------------------
include/asm-ia64/meminit.h | 2 -
3 files changed, 50 insertions(+), 44 deletions(-)
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -967,7 +967,7 @@ find_memmap_space (void)
* to use. We can allocate partial granules only if the unavailable
* parts exist, and are WB.
*/
-void
+unsigned long
efi_memmap_init(unsigned long *s, unsigned long *e)
{
struct kern_memdesc *k, *prev = NULL;
@@ -1084,6 +1084,8 @@ efi_memmap_init(unsigned long *s, unsign
/* reserve the memory we are using for kern_memmap */
*s = (u64)kern_memmap;
*e = (u64)++k;
+
+ return total_memory;
}
void
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -208,6 +208,48 @@ static int __init register_memory(void)
__initcall(register_memory);
+
+#ifdef CONFIG_KEXEC
+static void __init setup_crashkernel(unsigned long total, int *n)
+{
+ unsigned long long base = 0, size = 0;
+ int ret;
+
+ ret = parse_crashkernel(boot_command_line, total,
+ &size, &base);
+ if (ret == 0 && size > 0) {
+ if (!base) {
+ sort_regions(rsvd_region, *n);
+ base = kdump_find_rsvd_region(size,
+ rsvd_region, *n);
+ }
+ if (base != ~0UL) {
+ printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
+ "for crashkernel (System RAM: %ldMB)\n",
+ (unsigned long)(size >> 20),
+ (unsigned long)(base >> 20),
+ (unsigned long)(total >> 20));
+ rsvd_region[*n].start =
+ (unsigned long)__va(base);
+ rsvd_region[*n].end =
+ (unsigned long)__va(base + size);
+ (*n)++;
+ crashk_res.start = base;
+ crashk_res.end = base + size - 1;
+ }
+ }
+ efi_memmap_res.start = ia64_boot_param->efi_memmap;
+ efi_memmap_res.end = efi_memmap_res.start +
+ ia64_boot_param->efi_memmap_size;
+ boot_param_res.start = __pa(ia64_boot_param);
+ boot_param_res.end = boot_param_res.start +
+ sizeof(*ia64_boot_param);
+}
+#else
+static inline void __init setup_crashkernel(unsigned long total, int *n)
+{}
+#endif
+
/**
* reserve_memory - setup reserved memory areas
*
@@ -219,6 +261,7 @@ void __init
reserve_memory (void)
{
int n = 0;
+ unsigned long total_memory;
/*
* none of the entries in this table overlap
@@ -254,50 +297,11 @@ reserve_memory (void)
n++;
#endif
- efi_memmap_init(&rsvd_region[n].start, &rsvd_region[n].end);
+ total_memory = efi_memmap_init(&rsvd_region[n].start, &rsvd_region[n].end);
n++;
-#ifdef CONFIG_KEXEC
- /* crashkernel=size@offset specifies the size to reserve for a crash
- * kernel. If offset is 0, then it is determined automatically.
- * By reserving this memory we guarantee that linux never set's it
- * up as a DMA target.Useful for holding code to do something
- * appropriate after a kernel panic.
- */
- {
- char *from = strstr(boot_command_line, "crashkernel=");
- unsigned long base, size;
- if (from) {
- size = memparse(from + 12, &from);
- if (*from == '@')
- base = memparse(from+1, &from);
- else
- base = 0;
- if (size) {
- if (!base) {
- sort_regions(rsvd_region, n);
- base = kdump_find_rsvd_region(size,
- rsvd_region, n);
- }
- if (base != ~0UL) {
- rsvd_region[n].start =
- (unsigned long)__va(base);
- rsvd_region[n].end =
- (unsigned long)__va(base + size);
- n++;
- crashk_res.start = base;
- crashk_res.end = base + size - 1;
- }
- }
- }
- efi_memmap_res.start = ia64_boot_param->efi_memmap;
- efi_memmap_res.end = efi_memmap_res.start +
- ia64_boot_param->efi_memmap_size;
- boot_param_res.start = __pa(ia64_boot_param);
- boot_param_res.end = boot_param_res.start +
- sizeof(*ia64_boot_param);
- }
-#endif
+ setup_crashkernel(total_memory, &n);
+
/* end of memory marker */
rsvd_region[n].start = ~0UL;
rsvd_region[n].end = ~0UL;
--- a/include/asm-ia64/meminit.h
+++ b/include/asm-ia64/meminit.h
@@ -35,7 +35,7 @@ extern void find_memory (void);
extern void reserve_memory (void);
extern void find_initrd (void);
extern int filter_rsvd_memory (unsigned long start, unsigned long end, void *arg);
-extern void efi_memmap_init(unsigned long *, unsigned long *);
+extern unsigned long efi_memmap_init(unsigned long *s, unsigned long *e);
extern int find_max_min_low_pfn (unsigned long , unsigned long, void *);
extern unsigned long vmcore_find_descriptor_size(unsigned long address);
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 5/7] Use extended crashkernel command line on ppc64
2007-09-25 18:22 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
` (3 preceding siblings ...)
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 19:45 ` Andrew Morton
2007-09-25 18:23 ` [patch 6/7] Use extended crashkernel command line on sh Bernhard Walle
2007-09-25 18:23 ` [patch 7/7] Add documentation for extended crashkernel syntax Bernhard Walle
6 siblings, 1 reply; 21+ messages in thread
From: Bernhard Walle @ 2007-09-25 18:23 UTC (permalink / raw)
To: kexec, akpm; +Cc: linux-kernel, linux-arch, linuxppc-dev
[-- Attachment #1: crashkernel-ppc64 --]
[-- Type: text/plain, Size: 2560 bytes --]
This patch adapts the ppc64 code to use the generic parse_crashkernel()
function introduced in the generic patch of that series.
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
arch/powerpc/kernel/machine_kexec.c | 52 ++++++++++++++++++------------------
1 file changed, 26 insertions(+), 26 deletions(-)
--- a/arch/powerpc/kernel/machine_kexec.c
+++ b/arch/powerpc/kernel/machine_kexec.c
@@ -61,45 +61,39 @@ NORET_TYPE void machine_kexec(struct kim
for(;;);
}
-static int __init early_parse_crashk(char *p)
+void __init reserve_crashkernel(void)
{
- unsigned long size;
-
- if (!p)
- return 1;
-
- size = memparse(p, &p);
+ unsigned long long crash_size, crash_base;
+ int ret;
- if (*p == '@')
- crashk_res.start = memparse(p + 1, &p);
- else
- crashk_res.start = KDUMP_KERNELBASE;
-
- crashk_res.end = crashk_res.start + size - 1;
-
- return 0;
-}
-early_param("crashkernel", early_parse_crashk);
+ /* this is necessary because of lmb_phys_mem_size() */
+ lmb_analyze();
-void __init reserve_crashkernel(void)
-{
- unsigned long size;
+ /* use common parsing */
+ ret = parse_crashkernel(boot_command_line, lmb_phys_mem_size(),
+ &crash_size, &crash_base);
+ if (ret == 0 && crash_size > 0) {
+ if (crash_base == 0)
+ crash_base = KDUMP_KERNELBASE;
+ crashk_res.start = crash_base;
+ } else {
+ /* handle the device tree */
+ crash_size = crashk_res.end - crashk_res.start + 1;
+ }
- if (crashk_res.start == 0)
+ if (crash_size == 0)
return;
/* We might have got these values via the command line or the
* device tree, either way sanitise them now. */
- size = crashk_res.end - crashk_res.start + 1;
-
if (crashk_res.start != KDUMP_KERNELBASE)
printk("Crash kernel location must be 0x%x\n",
KDUMP_KERNELBASE);
crashk_res.start = KDUMP_KERNELBASE;
- size = PAGE_ALIGN(size);
- crashk_res.end = crashk_res.start + size - 1;
+ crash_size = PAGE_ALIGN(crash_size);
+ crashk_res.end = crashk_res.start + crash_size - 1;
/* Crash kernel trumps memory limit */
if (memory_limit && memory_limit <= crashk_res.end) {
@@ -108,7 +102,13 @@ void __init reserve_crashkernel(void)
memory_limit);
}
- lmb_reserve(crashk_res.start, size);
+ printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
+ "for crashkernel (System RAM: %ldMB)\n",
+ (unsigned long)(crash_size >> 20),
+ (unsigned long)(crashk_res.start >> 20),
+ (unsigned long)(lmb_phys_mem_size() >> 20));
+
+ lmb_reserve(crashk_res.start, crash_size);
}
int overlaps_crashkernel(unsigned long start, unsigned long size)
--
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 5/7] Use extended crashkernel command line on ppc64
2007-09-25 18:23 ` [patch 5/7] Use extended crashkernel command line on ppc64 Bernhard Walle
@ 2007-09-25 19:45 ` Andrew Morton
2007-09-26 8:15 ` Bernhard Walle
0 siblings, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2007-09-25 19:45 UTC (permalink / raw)
To: Bernhard Walle; +Cc: kexec, linux-kernel, linux-arch, linuxppc-dev
On Tue, 25 Sep 2007 20:23:02 +0200 Bernhard Walle <bwalle@suse.de> wrote:
> This patch adapts the ppc64 code to use the generic parse_crashkernel()
> function introduced in the generic patch of that series.
>
>
I really don't like to see patches get a wholesale replacement, especially
when they've been looked at by a few people and have had some testing, etc.
It's not possible to see what changed and people need to re-review stuff
they've already reviewed, etc.
So I almost always undo this mess, turn the patches back into incremental
ones, see what pops out.
This patch is actually:
diff -puN arch/powerpc/kernel/machine_kexec.c~use-extended-crashkernel-command-line-on-ppc64-update arch/powerpc/kernel/machine_kexec.c
--- a/arch/powerpc/kernel/machine_kexec.c~use-extended-crashkernel-command-line-on-ppc64-update
+++ a/arch/powerpc/kernel/machine_kexec.c
@@ -63,7 +63,7 @@ NORET_TYPE void machine_kexec(struct kim
void __init reserve_crashkernel(void)
{
- unsigned long long crash_size = 0, crash_base;
+ unsigned long long crash_size, crash_base;
int ret;
/* this is necessary because of lmb_phys_mem_size() */
_
which I suspect will now create a compiler warning.
unsigned long long crash_size, crash_base;
int ret;
/* this is necessary because of lmb_phys_mem_size() */
lmb_analyze();
/* use common parsing */
ret = parse_crashkernel(boot_command_line, lmb_phys_mem_size(),
&crash_size, &crash_base);
if (ret == 0 && crash_size > 0) {
if (crash_base == 0)
crash_base = KDUMP_KERNELBASE;
crashk_res.start = crash_base;
} else {
/* handle the device tree */
crash_size = crashk_res.end - crashk_res.start + 1;
}
if (crash_size == 0)
return;
If so, the use of uninitialized_var() would be better than the unneeded
initialization-to-zero.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [patch 5/7] Use extended crashkernel command line on ppc64
2007-09-25 19:45 ` Andrew Morton
@ 2007-09-26 8:15 ` Bernhard Walle
0 siblings, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-26 8:15 UTC (permalink / raw)
To: Andrew Morton; +Cc: kexec, linux-kernel, linux-arch, linuxppc-dev
Hi,
* Andrew Morton <akpm@linux-foundation.org> [2007-09-25 21:45]:
> On Tue, 25 Sep 2007 20:23:02 +0200 Bernhard Walle <bwalle@suse.de> wrote:
>
> > This patch adapts the ppc64 code to use the generic parse_crashkernel()
> > function introduced in the generic patch of that series.
>
> I really don't like to see patches get a wholesale replacement, especially
> when they've been looked at by a few people and have had some testing, etc.
> It's not possible to see what changed and people need to re-review stuff
> they've already reviewed, etc.
Sorry, I didn't know that this is the preferred way. I just thought
it's more clear if the patches are replaced if the patch set is
finally pushed into Linus' tree.
> diff -puN arch/powerpc/kernel/machine_kexec.c~use-extended-crashkernel-command-line-on-ppc64-update arch/powerpc/kernel/machine_kexec.c
> --- a/arch/powerpc/kernel/machine_kexec.c~use-extended-crashkernel-command-line-on-ppc64-update
> +++ a/arch/powerpc/kernel/machine_kexec.c
> @@ -63,7 +63,7 @@ NORET_TYPE void machine_kexec(struct kim
>
> void __init reserve_crashkernel(void)
> {
> - unsigned long long crash_size = 0, crash_base;
> + unsigned long long crash_size, crash_base;
> int ret;
>
> /* this is necessary because of lmb_phys_mem_size() */
> _
>
>
> which I suspect will now create a compiler warning.
No, it doesn't. I just verified this.
CHK include/linux/compile.h
CC arch/powerpc/kernel/machine_kexec.o
LD arch/powerpc/kernel/built-in.o
Thanks,
Bernhard
--
Bernhard Walle <bwalle@suse.de>, Architecture Maintenance
SUSE LINUX Products GmbH, Nürnberg, Germany
GF: Markus Rex, HRB 16746 (AG Nürnberg)
OpenPGP: F61F 34CC 09CA FB82 C9F6 BA4B 8865 3696 DDAF 6454
^ permalink raw reply [flat|nested] 21+ messages in thread
* [patch 6/7] Use extended crashkernel command line on sh
2007-09-25 18:22 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
` (4 preceding siblings ...)
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 ` [patch 7/7] Add documentation for extended crashkernel syntax Bernhard Walle
6 siblings, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-25 18:23 UTC (permalink / raw)
To: kexec, akpm; +Cc: linux-kernel, linux-arch, linuxsh-dev, Paul Mundt
[-- Attachment #1: crashkernel-sh --]
[-- Type: text/plain, Size: 3233 bytes --]
This patch removes the crashkernel parsing from arch/sh/kernel/machine_kexec.c
and calls the generic function, introduced in the generic patch, in
setup_bootmem_allocator().
This is necessary because the amount of System RAM must be known in this
function now because of the new syntax.
NOTE: Due to the lack of a SH processor, this patch is untested (and
uncompiled). Because the code in that area is quite similar as i386/x86_64
(contrary to PPC and IA64), it should compile and work. However, if someone of
the SH people could test for me and provide feedback, that would be very nice.
Signed-off-by: Bernhard Walle <bwalle@suse.de>
Acked-by: Paul Mundt <lethal@linux-sh.org>
---
arch/sh/kernel/machine_kexec.c | 21 ---------------------
arch/sh/kernel/setup.c | 38 +++++++++++++++++++++++++++++++++-----
2 files changed, 33 insertions(+), 26 deletions(-)
--- a/arch/sh/kernel/machine_kexec.c
+++ b/arch/sh/kernel/machine_kexec.c
@@ -104,24 +104,3 @@ NORET_TYPE void machine_kexec(struct kim
(*rnk)(page_list, reboot_code_buffer, image->start, vbr_reg);
}
-/* crashkernel=size@addr specifies the location to reserve for
- * a crash kernel. By reserving this memory we guarantee
- * that linux never sets it up as a DMA target.
- * Useful for holding code to do something appropriate
- * after a kernel panic.
- */
-static int __init parse_crashkernel(char *arg)
-{
- unsigned long size, base;
- size = memparse(arg, &arg);
- if (*arg == '@') {
- base = memparse(arg+1, &arg);
- /* FIXME: Do I want a sanity check
- * to validate the memory range?
- */
- crashk_res.start = base;
- crashk_res.end = base + size - 1;
- }
- return 0;
-}
-early_param("crashkernel", parse_crashkernel);
--- a/arch/sh/kernel/setup.c
+++ b/arch/sh/kernel/setup.c
@@ -128,6 +128,37 @@ static void __init register_bootmem_low_
free_bootmem(PFN_PHYS(curr_pfn), PFN_PHYS(pages));
}
+#ifdef CONFIG_KEXEC
+static void __init reserve_crashkernel(void)
+{
+ unsigned long long free_mem;
+ unsigned long long crash_size, crash_base;
+ int ret;
+
+ free_mem = ((unsigned long long)max_low_pfn - min_low_pfn) << PAGE_SHIFT;
+
+ ret = parse_crashkernel(boot_command_line, free_mem,
+ &crash_size, &crash_base);
+ if (ret == 0 && crash_size) {
+ if (crash_base > 0) {
+ printk(KERN_INFO "Reserving %ldMB of memory at %ldMB "
+ "for crashkernel (System RAM: %ldMB)\n",
+ (unsigned long)(crash_size >> 20),
+ (unsigned long)(crash_base >> 20),
+ (unsigned long)(free_mem >> 20));
+ crashk_res.start = crash_base;
+ crashk_res.end = crash_base + crash_size - 1;
+ reserve_bootmem(crash_base, crash_size);
+ } else
+ printk(KERN_INFO "crashkernel reservation failed - "
+ "you have to specify a base address\n");
+ }
+}
+#else
+static inline void __init reserve_crashkernel(void)
+{}
+#endif
+
void __init setup_bootmem_allocator(unsigned long free_pfn)
{
unsigned long bootmap_size;
@@ -189,11 +220,8 @@ void __init setup_bootmem_allocator(unsi
}
}
#endif
-#ifdef CONFIG_KEXEC
- if (crashk_res.start != crashk_res.end)
- reserve_bootmem(crashk_res.start,
- crashk_res.end - crashk_res.start + 1);
-#endif
+
+ reserve_crashkernel();
}
#ifndef CONFIG_NEED_MULTIPLE_NODES
--
^ permalink raw reply [flat|nested] 21+ messages in thread* [patch 7/7] Add documentation for extended crashkernel syntax
2007-09-25 18:22 [patch 0/7] Add extended crashkernel command line syntax Bernhard Walle
` (5 preceding siblings ...)
2007-09-25 18:23 ` [patch 6/7] Use extended crashkernel command line on sh Bernhard Walle
@ 2007-09-25 18:23 ` Bernhard Walle
6 siblings, 0 replies; 21+ messages in thread
From: Bernhard Walle @ 2007-09-25 18:23 UTC (permalink / raw)
To: kexec, akpm; +Cc: linux-kernel, linux-arch
[-- Attachment #1: crashkernel-documentation --]
[-- Type: text/plain, Size: 1991 bytes --]
This adds the documentation for the extended crashkernel syntax into
Documentation/kdump/kdump.txt.
Signed-off-by: Bernhard Walle <bwalle@suse.de>
---
Documentation/kdump/kdump.txt | 26 ++++++++++++++++++++++++++
Documentation/kernel-parameters.txt | 7 +++++++
2 files changed, 33 insertions(+)
--- a/Documentation/kdump/kdump.txt
+++ b/Documentation/kdump/kdump.txt
@@ -231,6 +231,32 @@ Dump-capture kernel config options (Arch
any space below the alignment point will be wasted.
+Extended crashkernel syntax
+===========================
+
+While the "crashkernel=size[@offset]" syntax is sufficient for most
+configurations, sometimes it's handy to have the reserved memory dependent
+on the value of System RAM -- that's mostly for distributors that pre-setup
+the kernel command line to avoid a unbootable system after some memory has
+been removed from the machine.
+
+The syntax is:
+
+ crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
+ range=start-[end]
+
+For example:
+
+ crashkernel=512M-2G:64M,2G-:128M
+
+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
+ 3) if the RAM size is larger than 2G, then reserve 128M
+
+
Boot into System Kernel
=======================
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -495,6 +495,13 @@ and is between 256 and 4096 characters.
[KNL] Reserve a chunk of physical memory to
hold a kernel to switch to with kexec on panic.
+ crashkernel=range1:size1[,range2:size2,...][@offset]
+ [KNL] Same as above, but depends on the memory
+ in the running system. The syntax of range is
+ start-[end] where start and end are both
+ a memory unit (amount[KMG]). See also
+ Documentation/kdump/kdump.txt for a example.
+
cs4232= [HW,OSS]
Format: <io>,<irq>,<dma>,<dma2>,<mpuio>,<mpuirq>
--
^ permalink raw reply [flat|nested] 21+ messages in thread