[patch 0/5] Add extended crashkernel command line syntax

[patch 0/5] Add extended crashkernel command line syntax

[Bernhard Walle]

This patch adds a extended crashkernel syntax that makes the value of reserved
system RAM dependent on the system RAM itself:

    crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
    range=start-[end]

For example:

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

The motivation comes from distributors that configure their crashkernel command
line automatically with some configuration tool (YaST, you know ;)). Of course
that tool knows the value of System RAM, but if the user removes RAM, then
the system becomes unbootable or at least unusable and error handling
is very difficult.

This series implements this change for i386, x86_64 and IA64. However, if
the patch is accepted, I can also add all other architectures that support
Kdump (when my grepping was correct, this is only PPC64 and SH in addition).
The "simple" syntax is of course still supported.


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

-- 

[patch 1/5] Extended crashkernel command line

[Bernhard Walle]

[-- Attachment #1: crashkernel-generic --]
[-- Type: text/plain, Size: 4687 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        |  139 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 141 insertions(+)

--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -179,6 +179,8 @@ extern note_buf_t *crash_notes;
 extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
 extern unsigned int vmcoreinfo_size;
 extern unsigned int vmcoreinfo_max_size;
+int parse_crashkernel(char *cmdline, unsigned long long system_ram,
+		unsigned long long *crash_size, unsigned long long *crash_base);
 
 
 #else /* !CONFIG_KEXEC */
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -1146,6 +1146,145 @@ 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 parse_crashkernel_mem(char 			*cmdline,
+				 unsigned long long 	*crash_size,
+				 unsigned long long 	*crash_base,
+				 unsigned long 		system_ram)
+{
+	char *cur = cmdline;
+
+	*crash_size = 0;
+	*crash_base = 0;
+
+	/* 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 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 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;
+
+	/* 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, crash_size,
+				crash_base, system_ram);
+	else
+		return parse_crashkernel_simple(ck_cmdline,
+				crash_size, crash_base);
+}
+
+
+
 void crash_save_vmcoreinfo(void)
 {
 	u32 *buf;

-- 

[patch 2/5] Use extended crashkernel command line on i386

[Bernhard Walle]

[-- Attachment #1: crashkernel-i386 --]
[-- Type: text/plain, Size: 3169 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         |   33 ++++++++++++++++++++++++++++-----
 3 files changed, 30 insertions(+), 28 deletions(-)

--- 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,33 @@ extern unsigned long __init setup_memory
 extern void zone_sizes_init(void);
 #endif /* !CONFIG_NEED_MULTIPLE_NODES */
 
+#ifdef CONFIG_KEXEC
+static void reserve_crashkernel(void)
+{
+	unsigned long long 	free_mem;
+	unsigned long long 	crash_size, crash_base;
+	int			ret;
+
+	free_mem = (max_low_pfn + highend_pfn - highstart_pfn) << PAGE_SHIFT;
+
+	ret = parse_crashkernel(boot_command_line, free_mem,
+			&crash_size, &crash_base);
+	if (ret == 0 && crash_size > 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;
+		reserve_bootmem(crash_base, crash_size);
+	}
+}
+#else
+static inline void reserve_crashkernel(void)
+{}
+#endif
+
 void __init setup_bootmem_allocator(void)
 {
 	unsigned long bootmap_size;
@@ -456,11 +483,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();
 }
 
 /*

-- 

[patch 3/5] Use extended crashkernel command line on x86_64

[Bernhard Walle]

[-- Attachment #1: crashkernel-x86_64 --]
[-- Type: text/plain, Size: 3239 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         |   35 ++++++++++++++++++++++++++++-------
 3 files changed, 30 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
@@ -196,6 +196,33 @@ static inline void copy_edd(void)
 }
 #endif
 
+#ifdef CONFIG_KEXEC
+static inline void reserve_crashkernel(void)
+{
+	unsigned long long 	free_mem;
+	unsigned long long 	crash_size, crash_base;
+	int			ret;
+
+	free_mem = max_low_pfn << PAGE_SHIFT;
+
+	ret = parse_crashkernel(boot_command_line, free_mem,
+			&crash_size, &crash_base);
+	if (ret == 0 && crash_size > 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;
+		reserve_bootmem(crash_base, crash_size);
+	}
+}
+#else
+static inline void reserve_crashkernel(void)
+{}
+#endif
+
 #define EBDA_ADDR_POINTER 0x40E
 
 unsigned __initdata ebda_addr;
@@ -388,13 +415,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

-- 

[patch 4/5] Use extended crashkernel command line on IA64

[Bernhard Walle]

[-- Attachment #1: crashkernel-ia64 --]
[-- Type: text/plain, Size: 5125 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 comulated 
total_memory variable in a pointer value. IMO that's better than making the
variable global.

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     |    5 ++
 arch/ia64/kernel/setup.c   |   88 +++++++++++++++++++++++----------------------
 include/asm-ia64/meminit.h |    3 +
 3 files changed, 52 insertions(+), 44 deletions(-)

--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -968,7 +968,7 @@ find_memmap_space (void)
  * parts exist, and are WB.
  */
 void
-efi_memmap_init(unsigned long *s, unsigned long *e)
+efi_memmap_init(unsigned long *s, unsigned long *e, unsigned long *total_memory)
 {
 	struct kern_memdesc *k, *prev = NULL;
 	u64	contig_low=0, contig_high=0;
@@ -1084,6 +1084,9 @@ efi_memmap_init(unsigned long *s, unsign
 	/* reserve the memory we are using for kern_memmap */
 	*s = (u64)kern_memmap;
 	*e = (u64)++k;
+
+	if (total_memory)
+		*total_memory = total_mem;
 }
 
 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 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 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 = 0;
 
 	/*
 	 * 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);
+	efi_memmap_init(&rsvd_region[n].start, &rsvd_region[n].end, &total_memory);
 	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,8 @@ 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 void efi_memmap_init(unsigned long *s, unsigned long *e,
+		unsigned long *total_memory);
 extern int find_max_min_low_pfn (unsigned long , unsigned long, void *);
 
 extern unsigned long vmcore_find_descriptor_size(unsigned long address);

-- 

[patch 5/5] Add documentation for extended crashkernel syntax

[Bernhard Walle]

[-- Attachment #1: crashkernel-documentation --]
[-- Type: text/plain, Size: 1315 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 ++++++++++++++++++++++++++
 1 file changed, 26 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
 =======================
 

-- 

Re: [patch 4/5] Use extended crashkernel command line on IA64

[Matthew Wilcox]

On Sun, Sep 09, 2007 at 10:39:18AM +0200, Bernhard Walle wrote:
> Because the total amount of System RAM must be known when calling this
> function, efi_memmap_init() is modified to return its comulated 

accumulated?

> total_memory variable in a pointer value. IMO that's better than making the
> variable global.

Why not make efi_memmap_init() return total_memory instead of void?

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

[Yinghai Lu]

On 9/9/07, Bernhard Walle <bwalle@suse.de> wrote:
> 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         |   35 ++++++++++++++++++++++++++++-------
>  3 files changed, 30 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
...
> --- 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
> @@ -196,6 +196,33 @@ static inline void copy_edd(void)
>  }
>  #endif
>
> +#ifdef CONFIG_KEXEC
...

CONFIG_KEXEC or CONFIG_CRASH_DUMP?

YH

Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

[Bernhard Walle]

* Yinghai Lu <yhlu.kernel@gmail.com> [2007-09-09 19:27]:
> >
> > +#ifdef CONFIG_KEXEC
> ...
> 
> CONFIG_KEXEC or CONFIG_CRASH_DUMP?

Good question. The crashkernel parameter was CONFIG_KEXEC before, and
I also wondered why, but I didn't change this because maybe there's
some reason I don't know.

Vivek, do you know why this was CONFIG_KEXEC?


Thanks,
   Bernhard

Re: [patch 4/5] Use extended crashkernel command line on IA64

[Bernhard Walle]

* Matthew Wilcox <matthew@wil.cx> [2007-09-09 15:01]:
> On Sun, Sep 09, 2007 at 10:39:18AM +0200, Bernhard Walle wrote:
> > Because the total amount of System RAM must be known when calling this
> > function, efi_memmap_init() is modified to return its comulated 
> 
> accumulated?

Yes, sorry.

> > total_memory variable in a pointer value. IMO that's better than making the
> > variable global.
> 
> Why not make efi_memmap_init() return total_memory instead of void?

Good idea:

-----

From: Bernhard Walle <bwalle@suse.de>
Subject: Use extended crashkernel command line on IA64
Cc: linux-ia64@vger.kernel.org

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 in a pointer value. IMO that's better than making the
variable global.

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 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 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 = 0;
 
 	/*
 	 * 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);

Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

[Eric W. Biederman]

Bernhard Walle <bwalle@suse.de> writes:

> * Yinghai Lu <yhlu.kernel@gmail.com> [2007-09-09 19:27]:
>> >
>> > +#ifdef CONFIG_KEXEC
>> ...
>> 
>> CONFIG_KEXEC or CONFIG_CRASH_DUMP?
>
> Good question. The crashkernel parameter was CONFIG_KEXEC before, and
> I also wondered why, but I didn't change this because maybe there's
> some reason I don't know.
>
> Vivek, do you know why this was CONFIG_KEXEC?

Probably because you use it in the primary kernel you use it.
The option reserves an area of memory for the kernel we switch
to on panic or another kernel crash.

Generally CONFIG_CRASH_DUMP seems to be about the options needed
to read out the crash dump after the fact.

Eric

Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

[Vivek Goyal]

On Sun, Sep 09, 2007 at 08:52:58PM +0200, Bernhard Walle wrote:
> * Yinghai Lu <yhlu.kernel@gmail.com> [2007-09-09 19:27]:
> > >
> > > +#ifdef CONFIG_KEXEC
> > ...
> > 
> > CONFIG_KEXEC or CONFIG_CRASH_DUMP?
> 
> Good question. The crashkernel parameter was CONFIG_KEXEC before, and
> I also wondered why, but I didn't change this because maybe there's
> some reason I don't know.
> 
> Vivek, do you know why this was CONFIG_KEXEC?
> 
> 

Bernhard,

As Eric mentioned, CONFIG_CRASH_DUMP has been used for all dump capturing
infrastructure and rest of the kexec and kexec on panic functionality
has been put under CONFIG_KEXEC. 

Keeping memory reservation under CONFIG_KEXEC helps in a sense when
somebody is not using a relocatable kernel and uses a custom kernel for dump
capture. In that case he does not have to enable CONFIG_CRASH_DUMP in the
first kernel.

Thanks
Vivek

Re: [patch 0/5] Add extended crashkernel command line syntax

[Vivek Goyal]

On Sun, Sep 09, 2007 at 10:39:14AM +0200, Bernhard Walle wrote:
> This patch adds a extended crashkernel syntax that makes the value of reserved
> system RAM dependent on the system RAM itself:
> 
>     crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset]
>     range=start-[end]
> 
> For example:
> 
>     crashkernel=512M-2G:64M,2G-:128M
> 
> The motivation comes from distributors that configure their crashkernel command
> line automatically with some configuration tool (YaST, you know ;)). Of course
> that tool knows the value of System RAM, but if the user removes RAM, then
> the system becomes unbootable or at least unusable and error handling
> is very difficult.
> 

System does not boot after removing memory because a large chunk of it has
been reserved by kdump and there is not sufficient memory left for rest of
the OS to initialize?

How often does this scenario happen? Anyway, if you run into these issues
and if it helps, I don't mind additional command line syntax for crashkernel=
 
> This series implements this change for i386, x86_64 and IA64. However, if
> the patch is accepted, I can also add all other architectures that support
> Kdump (when my grepping was correct, this is only PPC64 and SH in addition).
> The "simple" syntax is of course still supported.
> 
> 

Please make the changes for ppc64 and sh too.

Thanks
Vivek

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

[Vivek Goyal]

On Sun, Sep 09, 2007 at 10:39:15AM +0200, Bernhard Walle wrote:

[..]
> +
> +/*
> + * 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 parse_crashkernel_mem(char 			*cmdline,
> +				 unsigned long long 	*crash_size,
> +				 unsigned long long 	*crash_base,
> +				 unsigned long 		system_ram)
> +{
> +	char *cur = cmdline;
> +
> +	*crash_size = 0;
> +	*crash_base = 0;
> +
> +	/* 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);

"offset" seems to be optional in the new syntax. What happens if user does
not specify offset. I think crash_base will be set to zero and system will
try to reserve x amount of memory start at zero? That would fail?

I think we should add some intelligence for automatic selection of "offset"
if user has not specified one. Automatically choose a chunk of free memory.
This takes away the headache from user for selecting a right place. In fact
one "offset" might not be valid for all the systems. I remember, somebody
had reported that ACPI tables were mapped lower in the address space and
reserving memory for kdump had failed.

Choosing the "offset" automatically should help distributions where one
command line is supposed to work on all the systems.

Thanks
Vivek

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

[Bernhard Walle]

* Vivek Goyal <vgoyal@in.ibm.com> [2007-09-11 08:15]:
> 
> "offset" seems to be optional in the new syntax. What happens if user does
> not specify offset. I think crash_base will be set to zero and system will
> try to reserve x amount of memory start at zero? That would fail?

That's handled in the architecture specific code -- because it's
different on each architecture and the architecture specific code does
memory reservation. IA64 already can handle this case (on IA64,
specifying 0 is the same than leaving out the base address, and that's
why I wanted to keep that semantics). I think it doesn't also make
sense on i386/x86_64 to choose 0 as real base address, because the
value below 1 MB is special for booting ...

> I think we should add some intelligence for automatic selection of "offset"
> if user has not specified one. Automatically choose a chunk of free memory.
> This takes away the headache from user for selecting a right place. In fact
> one "offset" might not be valid for all the systems. I remember, somebody
> had reported that ACPI tables were mapped lower in the address space and
> reserving memory for kdump had failed.

I think you're right -- with the relocatable kernel it makes sense to
have the kernel to decide where to reservate that memory. But that's
beyond of the scope of that patch series -- however, I can come up
with a solution, after that patch has been finished.


Thanks,
   Bernhard

Re: [discuss] [patch 3/5] Use extended crashkernel command line on x86_64

[Bernhard Walle]

* Vivek Goyal <vgoyal@in.ibm.com> [2007-09-11 07:14]:
> On Sun, Sep 09, 2007 at 08:52:58PM +0200, Bernhard Walle wrote:
> > * Yinghai Lu <yhlu.kernel@gmail.com> [2007-09-09 19:27]:
> > > >
> > > > +#ifdef CONFIG_KEXEC
> > > ...
> > > 
> > > CONFIG_KEXEC or CONFIG_CRASH_DUMP?
> > 
> > Good question. The crashkernel parameter was CONFIG_KEXEC before, and
> > I also wondered why, but I didn't change this because maybe there's
> > some reason I don't know.
> > 
> > Vivek, do you know why this was CONFIG_KEXEC?
> 
> As Eric mentioned, CONFIG_CRASH_DUMP has been used for all dump capturing
> infrastructure and rest of the kexec and kexec on panic functionality
> has been put under CONFIG_KEXEC. 
> 
> Keeping memory reservation under CONFIG_KEXEC helps in a sense when
> somebody is not using a relocatable kernel and uses a custom kernel for dump
> capture. In that case he does not have to enable CONFIG_CRASH_DUMP in the
> first kernel.

Yes, you all are right ... sorry for the noise ;)


Thanks,
   Bernhard

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

[Olaf Dabrunz]

On 09-Sep-07, Bernhard Walle wrote:
> 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.

AFAIK it is the other way round: the lilo configuration-parameter
"append" appends the kernel parameters in the configuration to the
parameters given by the user on the kernel command line.

So the parameters set by the user during boot appear first on the
command line.

> Signed-off-by: Bernhard Walle <bwalle@suse.de>
> 
> ---
>  include/linux/kexec.h |    2 
>  kernel/kexec.c        |  139 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 141 insertions(+)
> 
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -179,6 +179,8 @@ extern note_buf_t *crash_notes;
>  extern u32 vmcoreinfo_note[VMCOREINFO_NOTE_SIZE/4];
>  extern unsigned int vmcoreinfo_size;
>  extern unsigned int vmcoreinfo_max_size;
> +int parse_crashkernel(char *cmdline, unsigned long long system_ram,
> +		unsigned long long *crash_size, unsigned long long *crash_base);
>  
>  [...]
>  
> +int 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;
> +
> +	/* 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=");
> +	}
> +

So perhaps something like this instead:

+	/* find crashkernel and use the first one if there are more */
+	p = strstr(p, "crashkernel=");
+	ck_cmdline = p;

> +	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))
> 
> [...]

-- 
Olaf Dabrunz (od/odabrunz), SUSE Linux Products GmbH, Nürnberg

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

[Lombard, David N]

On Tue, Sep 11, 2007 at 03:14:45PM +0200, Olaf Dabrunz wrote:
> On 09-Sep-07, Bernhard Walle wrote:
> > 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.
> 
> AFAIK it is the other way round: the lilo configuration-parameter
> "append" appends the kernel parameters in the configuration to the
> parameters given by the user on the kernel command line.
> 
> So the parameters set by the user during boot appear first on the
> command line.

Given the kernel itself processes the cmdline from front to back,
that suggests the last value is the correct value.


A survey of bootloaders shows differing behaviors:

syslinux et al.:
  APPEND options...
    Add one or more options to the kernel command line.  These are
    added both for automatic and manual boots.  The options are
    added at the very beginning of the kernel command line,
    usually permitting explicitly entered kernel options to override
    them.  This is the equivalent of the LILO "append" option.

lilo:
  append=<string>

    Appends the options specified to the parameter line passed to
    the kernel.  This is typically used to specify hardware parameters
    that can't be entirely auto-detected or for which probing may be
    dangerous. Multiple kernel parameters are separated by a blank
    space, and the string must be enclosed in double quotes.  A local
    append= appearing withing an image= section overrides any
    global append= appearing in the top section of the configuration file.
    Append= may be used only once per "image=" section.

    [There's also a local addappend string that's appended to the append string]

elilo:
  append=string       Append a string of options to kernel command line
    ...
    The user can specify a kernel and related kernel options using the image
    label. Alternatively, the user can also specify a kernel file that is
    not specified in the config file. In any case, some of the global options
    (such as append) are always concatenated to whatever the user type.

Grub:
  Interactive editing of the command line...

kboot
  Config file entries precede user's values from keyboard



-- 
David N. Lombard, Intel, Irvine, CA
I do not speak for Intel Corporation; all comments are strictly my own.

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

[Bernhard Walle]

* Lombard, David N <dnlombar@ichips.intel.com> [2007-09-11 17:32]:
> 
> lilo:
>   append=<string>
> 
>     Appends the options specified to the parameter line passed to
>     the kernel. 

Given that lilo appends the user-specified command line, I think it's
the best to honor the last value. It has also been the previous
behaviour (execpt IA64).

Having a different behaviour if the last or the first value is honored
on different architectures is not what we want, IMO.


Thanks,
   Bernhard

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

[Vivek Goyal]

On Tue, Sep 11, 2007 at 12:01:10PM +0200, Bernhard Walle wrote:
> * Vivek Goyal <vgoyal@in.ibm.com> [2007-09-11 08:15]:
> > 
> > "offset" seems to be optional in the new syntax. What happens if user does
> > not specify offset. I think crash_base will be set to zero and system will
> > try to reserve x amount of memory start at zero? That would fail?
> 
> That's handled in the architecture specific code -- because it's
> different on each architecture and the architecture specific code does
> memory reservation. IA64 already can handle this case (on IA64,
> specifying 0 is the same than leaving out the base address, and that's
> why I wanted to keep that semantics). I think it doesn't also make
> sense on i386/x86_64 to choose 0 as real base address, because the
> value below 1 MB is special for booting ...
> 

Ok. I see IA64 is handling this case. But in current patchset, i386 and
x86_64 will try to reserve memory starting at zero?  So we still got
to handle this case in i386 and x86_64?

Thanks
Vivek

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

[Bernhard Walle]

* Vivek Goyal <vgoyal@in.ibm.com> [2007-09-12 13:23]:
> On Tue, Sep 11, 2007 at 12:01:10PM +0200, Bernhard Walle wrote:
> > * Vivek Goyal <vgoyal@in.ibm.com> [2007-09-11 08:15]:
> > > 
> > > "offset" seems to be optional in the new syntax. What happens if user does
> > > not specify offset. I think crash_base will be set to zero and system will
> > > try to reserve x amount of memory start at zero? That would fail?
> > 
> > That's handled in the architecture specific code -- because it's
> > different on each architecture and the architecture specific code does
> > memory reservation. IA64 already can handle this case (on IA64,
> > specifying 0 is the same than leaving out the base address, and that's
> > why I wanted to keep that semantics). I think it doesn't also make
> > sense on i386/x86_64 to choose 0 as real base address, because the
> > value below 1 MB is special for booting ...
> > 
> 
> Ok. I see IA64 is handling this case. But in current patchset, i386 and
> x86_64 will try to reserve memory starting at zero?  So we still got
> to handle this case in i386 and x86_64?

Yes, my fault. I need to replace

+       if (ret == 0 && crash_size > 0) {

with

+       if (ret == 0 && crash_size > 0 && crash_base > 0) {

I'll repost the whole patch with all the corrections when I finished
PPC64 and SH. (I'm not in office this week, that's why I'm a bit slow.)


Thanks,
   Bernhard

Re: [patch 0/5] Add extended crashkernel command line syntax

[Bernhard Walle]

* Vivek Goyal <vgoyal@in.ibm.com> [2007-09-11 08:09]:
> 
> How often does this scenario happen? Anyway, if you run into these issues
> and if it helps, I don't mind additional command line syntax for crashkernel=

Of course, I agree, this is a rare case.

> > This series implements this change for i386, x86_64 and IA64. However, if
> > the patch is accepted, I can also add all other architectures that support
> > Kdump (when my grepping was correct, this is only PPC64 and SH in addition).
> > The "simple" syntax is of course still supported.
> 
> Please make the changes for ppc64 and sh too.

Thanks, done now. Patchset follows. It should address all issues
mentioned by you and others.


Thanks,
   Bernhard