[Patch 0/7] Implement crashkernel=auto

[Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

This series of patch implements automatically reserved memory for crashkernel,
by introducing a new boot option "crashkernel=auto". This idea is from Neil.

In case of breaking user-space applications, it modifies this boot option after
it decides how much memory should be reserved.

On different arch, the threshold and reserved memory size is different. Please
refer patch 7/7 which contains an update for the documentation.

Note: This patchset was only tested on x86_64 with differernt memory sizes.

Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: Neil Horman <nhorman@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Anton Vorontsov <avorontsov@ru.mvista.com>

[Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Amerigo Wang]

Introduce a new config option KEXEC_AUTO_RESERVE for x86.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
Index: linux-2.6/arch/x86/Kconfig
===================================================================
--- linux-2.6.orig/arch/x86/Kconfig
+++ linux-2.6/arch/x86/Kconfig
@@ -1482,6 +1482,16 @@ config KEXEC
 	  support.  As of this writing the exact hardware interface is
 	  strongly in flux, so no good recommendation can be made.
 
+config KEXEC_AUTO_RESERVE
+	bool "automatically reserve memory for kexec kernel"
+	depends on KEXEC
+	default y
+	---help---
+	  Automatically reserve memory for a kexec kernel, so that you don't
+	  need to specify numbers for the "crashkernel=X@Y" boot option,
+	  instead you can use "crashkernel=auto".
+	  On x86, 128M is reserved.
+
 config CRASH_DUMP
 	bool "kernel crash dumps"
 	depends on X86_64 || (X86_32 && HIGHMEM)

Re: [Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Neil Horman]

On Wed, Aug 05, 2009 at 07:19:12AM -0400, Amerigo Wang wrote:
> 
> Introduce a new config option KEXEC_AUTO_RESERVE for x86.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> 
> ---
> Index: linux-2.6/arch/x86/Kconfig
> ===================================================================
> --- linux-2.6.orig/arch/x86/Kconfig
> +++ linux-2.6/arch/x86/Kconfig
> @@ -1482,6 +1482,16 @@ config KEXEC
>  	  support.  As of this writing the exact hardware interface is
>  	  strongly in flux, so no good recommendation can be made.
>  
> +config KEXEC_AUTO_RESERVE
> +	bool "automatically reserve memory for kexec kernel"
> +	depends on KEXEC
> +	default y
> +	---help---
> +	  Automatically reserve memory for a kexec kernel, so that you don't
> +	  need to specify numbers for the "crashkernel=X@Y" boot option,
> +	  instead you can use "crashkernel=auto".
> +	  On x86, 128M is reserved.
> +
>  config CRASH_DUMP
>  	bool "kernel crash dumps"
>  	depends on X86_64 || (X86_32 && HIGHMEM)

Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Andi Kleen]

Amerigo Wang <amwang@redhat.com> writes:

> Introduce a new config option KEXEC_AUTO_RESERVE for x86.

The description of the feature belongs in the changelog.

I like the basic idea, but:

> +config KEXEC_AUTO_RESERVE
> +	bool "automatically reserve memory for kexec kernel"
> +	depends on KEXEC
> +	default y
> +	---help---
> +	  Automatically reserve memory for a kexec kernel, so that you don't
> +	  need to specify numbers for the "crashkernel=X@Y" boot option,
> +	  instead you can use "crashkernel=auto".
> +	  On x86, 128M is reserved.

The obvious problem is the hardcoded 128MB (and 128MB is very large
for a crash kernel anyways)

More useful would seem a crashkernel=size@auto

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Eric W. Biederman]

Andi Kleen <andi@firstfloor.org> writes:

> Amerigo Wang <amwang@redhat.com> writes:
>
>> Introduce a new config option KEXEC_AUTO_RESERVE for x86.
>
> The description of the feature belongs in the changelog.
>
> I like the basic idea, but:
>
>> +config KEXEC_AUTO_RESERVE
>> +	bool "automatically reserve memory for kexec kernel"
>> +	depends on KEXEC
>> +	default y
>> +	---help---
>> +	  Automatically reserve memory for a kexec kernel, so that you don't
>> +	  need to specify numbers for the "crashkernel=X@Y" boot option,
>> +	  instead you can use "crashkernel=auto".
>> +	  On x86, 128M is reserved.
>
> The obvious problem is the hardcoded 128MB (and 128MB is very large
> for a crash kernel anyways)
>
> More useful would seem a crashkernel=size@auto

That is actually called "crashkernel=size" and we have had that for quite
a while.  Although some of the init scripts have problems.

Eric

Re: [Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Amerigo Wang]

Andi Kleen wrote:
> Amerigo Wang <amwang@redhat.com> writes:
>
>   
>> Introduce a new config option KEXEC_AUTO_RESERVE for x86.
>>     
>
> The description of the feature belongs in the changelog.
>
> I like the basic idea, but:
>
>   
>> +config KEXEC_AUTO_RESERVE
>> +	bool "automatically reserve memory for kexec kernel"
>> +	depends on KEXEC
>> +	default y
>> +	---help---
>> +	  Automatically reserve memory for a kexec kernel, so that you don't
>> +	  need to specify numbers for the "crashkernel=X@Y" boot option,
>> +	  instead you can use "crashkernel=auto".
>> +	  On x86, 128M is reserved.
>>     
>
> The obvious problem is the hardcoded 128MB (and 128MB is very large
> for a crash kernel anyways)
>   


I think that size has to be hardcoded, or we can make it a bit 
changeable,  according to the page size.... e.g. on PPC and IA64, page 
size can be 16K or more, but x86's page size is always 4K I think.

Hmm, yes, I choose such a large size in order to be safe, but since you 
feel this is too large, how about 64M on x86? (On x86_64 Fedora and 
RHEL, the size of a kernel binary is about 2M~3M.)
> More useful would seem a crashkernel=size@auto
>   
We already have this, just use "crashkernel=size@0". :)

Thanks.

Re: [Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Andi Kleen]

>> More useful would seem a crashkernel=size@auto
>>   
> We already have this, just use "crashkernel=size@0". :)

When it's already there then I don't see the point of the feature at all.

Hardcoding the size doesn't really make any sense to me, especially
a suspicious one like 128MB.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Amerigo Wang]

Andi Kleen wrote:
>>> More useful would seem a crashkernel=size@auto
>>>   
>>>       
>> We already have this, just use "crashkernel=size@0". :)
>>     
>
> When it's already there then I don't see the point of the feature at all.
>
> Hardcoding the size doesn't really make any sense to me, especially
> a suspicious one like 128MB.
>   

Hi, Andi,

The point here is the size, hmm... I just got an idea, how about 
reserving the same memory of the size of kernel itself? For example, if 
the kernel itself is 3M, we reserved 8M ( x2 and then rounded to 2^n) 
for the kexec kernel?

Any comments?

Thanks!

Re: [Patch 1/7] x86: add CONFIG_KEXEC_AUTO_RESERVE

[Amerigo Wang]

Andi Kleen wrote:
> Hardcoding the size doesn't really make any sense to me, especially
> a suspicious one like 128MB.
>   

Er, don't worry about this is too large, I am working on a patch which 
will allow us to remove the unnecessary reserved memory from user space. :)

[Patch 2/7] x86: implement crashkernel=auto

[Amerigo Wang]

Implement "crashkernel=auto" for x86 first, other arch will be added in the
following patches.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
Index: linux-2.6/kernel/kexec.c
===================================================================
--- linux-2.6.orig/kernel/kexec.c
+++ linux-2.6/kernel/kexec.c
@@ -37,6 +37,7 @@
 #include <asm/io.h>
 #include <asm/system.h>
 #include <asm/sections.h>
+#include <asm/setup.h>
 
 /* Per cpu memory for storing cpu states in case of system crash. */
 note_buf_t* crash_notes;
@@ -1297,6 +1298,38 @@ int __init parse_crashkernel(char 		 *cm
 
 	ck_cmdline += 12; /* strlen("crashkernel=") */
 
+#ifdef CONFIG_KEXEC_AUTO_RESERVE
+	if (strncmp(ck_cmdline, "auto", 4) == 0) {
+		unsigned long long size;
+		char tmp[32];
+
+		size = arch_default_crash_size(system_ram);
+		if (size != 0) {
+			*crash_size = size;
+			*crash_base = arch_default_crash_base();
+			size = scnprintf(tmp, sizeof(tmp), "%luM@%luM",
+					(unsigned long)(*crash_size)>>20,
+					(unsigned long)(*crash_base)>>20);
+			/* size can't be <= 4. */
+			if (likely((size - 4 + strlen(cmdline))
+					< COMMAND_LINE_SIZE - 1)) {
+				memmove(ck_cmdline + size, ck_cmdline + 4,
+					strlen(cmdline) - (ck_cmdline + 4 - cmdline) + 1);
+				memcpy(ck_cmdline, tmp, size);
+			}
+			return 0;
+		} else {
+			/*
+			 * We can't reserve memory auotmatcally,
+			 * remove "crashkernel=" from cmdline.
+			 */
+			ck_cmdline += 4; /* strlen("auto") */
+			memmove(ck_cmdline - 16, ck_cmdline,
+				strlen(cmdline) - (ck_cmdline - cmdline) + 1);
+			return -ENOMEM;
+		}
+	}
+#endif
 	/*
 	 * if the commandline contains a ':', then that's the extended
 	 * syntax -- if not, it must be the classic syntax
Index: linux-2.6/arch/x86/include/asm/kexec.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/kexec.h
+++ linux-2.6/arch/x86/include/asm/kexec.h
@@ -61,6 +61,29 @@
 # define KEXEC_ARCH KEXEC_ARCH_X86_64
 #endif
 
+#ifdef CONFIG_KEXEC_AUTO_RESERVE
+#ifndef KEXEC_AUTO_RESERVED_SIZE
+#define KEXEC_AUTO_RESERVED_SIZE 1ULL<<27 /* 128M */
+#endif
+#ifndef KEXEC_AUTO_THRESHOLD
+#define KEXEC_AUTO_THRESHOLD 1ULL<<32 /* 4G */
+#endif
+static inline
+unsigned long long arch_default_crash_size(unsigned long long total_size)
+{
+	if (total_size < KEXEC_AUTO_THRESHOLD)
+		return 0;
+	else
+		return KEXEC_AUTO_RESERVED_SIZE;
+}
+static inline
+unsigned long long arch_default_crash_base(void)
+{
+	/* On x86, 0 means find the base address automatically. */
+	return 0;
+}
+#endif
+
 /*
  * CPU does not save ss and sp on stack if execution is already
  * running in kernel mode at the time of NMI occurrence. This code

Re: [Patch 2/7] x86: implement crashkernel=auto

[Neil Horman]

On Wed, Aug 05, 2009 at 07:19:22AM -0400, Amerigo Wang wrote:
> 
> Implement "crashkernel=auto" for x86 first, other arch will be added in the
> following patches.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> 
> ---
> Index: linux-2.6/kernel/kexec.c
> ===================================================================
> --- linux-2.6.orig/kernel/kexec.c
> +++ linux-2.6/kernel/kexec.c
> @@ -37,6 +37,7 @@
>  #include <asm/io.h>
>  #include <asm/system.h>
>  #include <asm/sections.h>
> +#include <asm/setup.h>
>  
>  /* Per cpu memory for storing cpu states in case of system crash. */
>  note_buf_t* crash_notes;
> @@ -1297,6 +1298,38 @@ int __init parse_crashkernel(char 		 *cm
>  
>  	ck_cmdline += 12; /* strlen("crashkernel=") */
>  
> +#ifdef CONFIG_KEXEC_AUTO_RESERVE
> +	if (strncmp(ck_cmdline, "auto", 4) == 0) {
> +		unsigned long long size;
> +		char tmp[32];
> +
> +		size = arch_default_crash_size(system_ram);
> +		if (size != 0) {
> +			*crash_size = size;
> +			*crash_base = arch_default_crash_base();
> +			size = scnprintf(tmp, sizeof(tmp), "%luM@%luM",
> +					(unsigned long)(*crash_size)>>20,
> +					(unsigned long)(*crash_base)>>20);
> +			/* size can't be <= 4. */
> +			if (likely((size - 4 + strlen(cmdline))
> +					< COMMAND_LINE_SIZE - 1)) {
> +				memmove(ck_cmdline + size, ck_cmdline + 4,
> +					strlen(cmdline) - (ck_cmdline + 4 - cmdline) + 1);
> +				memcpy(ck_cmdline, tmp, size);
> +			}
> +			return 0;
> +		} else {
> +			/*
> +			 * We can't reserve memory auotmatcally,
> +			 * remove "crashkernel=" from cmdline.
> +			 */
> +			ck_cmdline += 4; /* strlen("auto") */
> +			memmove(ck_cmdline - 16, ck_cmdline,
> +				strlen(cmdline) - (ck_cmdline - cmdline) + 1);
> +			return -ENOMEM;
> +		}
> +	}
> +#endif
>  	/*
>  	 * if the commandline contains a ':', then that's the extended
>  	 * syntax -- if not, it must be the classic syntax
> Index: linux-2.6/arch/x86/include/asm/kexec.h
> ===================================================================
> --- linux-2.6.orig/arch/x86/include/asm/kexec.h
> +++ linux-2.6/arch/x86/include/asm/kexec.h
> @@ -61,6 +61,29 @@
>  # define KEXEC_ARCH KEXEC_ARCH_X86_64
>  #endif
>  
> +#ifdef CONFIG_KEXEC_AUTO_RESERVE
> +#ifndef KEXEC_AUTO_RESERVED_SIZE
> +#define KEXEC_AUTO_RESERVED_SIZE 1ULL<<27 /* 128M */
> +#endif
> +#ifndef KEXEC_AUTO_THRESHOLD
> +#define KEXEC_AUTO_THRESHOLD 1ULL<<32 /* 4G */
> +#endif
> +static inline
> +unsigned long long arch_default_crash_size(unsigned long long total_size)
> +{
> +	if (total_size < KEXEC_AUTO_THRESHOLD)
> +		return 0;
> +	else
> +		return KEXEC_AUTO_RESERVED_SIZE;
> +}
> +static inline
> +unsigned long long arch_default_crash_base(void)
> +{
> +	/* On x86, 0 means find the base address automatically. */
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * CPU does not save ss and sp on stack if execution is already
>   * running in kernel mode at the time of NMI occurrence. This code


What about all the other arches that support kexec?  ia64/ppc[64]/s390/etc?
Don't they need an implementation of arch_default_crash_size?  Or perhaps better
still you should put this definition in asm-generic, so that it can be
overridden per-arch if need be, but you always have something to fall back on.

Neil

Re: [Patch 2/7] x86: implement crashkernel=auto

[Amerigo Wang]

Neil Horman wrote:
> What about all the other arches that support kexec?  ia64/ppc[64]/s390/etc?
> Don't they need an implementation of arch_default_crash_size?  Or perhaps better
> still you should put this definition in asm-generic, so that it can be
> overridden per-arch if need be, but you always have something to fall back on.
>   

Hmm, I think you mean we need ARCH_HAS_XXXX....
OK, it is a good idea, I will try it.

Thanks for your review, Neil.

RE: [Patch 2/7] x86: implement crashkernel=auto

[Yu, Fenghua]

>+#ifdef CONFIG_KEXEC_AUTO_RESERVE
>+	if (strncmp(ck_cmdline, "auto", 4) == 0) {
>+		unsigned long long size;
>+		char tmp[32];
>+
>+		size = arch_default_crash_size(system_ram);
>+		if (size != 0) {
>+			*crash_size = size;
>+			*crash_base = arch_default_crash_base();
>+			size = scnprintf(tmp, sizeof(tmp), "%luM@%luM",
>+					(unsigned long)(*crash_size)>>20,
>+					(unsigned long)(*crash_base)>>20);
>+			/* size can't be <= 4. */
>+			if (likely((size - 4 + strlen(cmdline))
>+					< COMMAND_LINE_SIZE - 1)) {
>+				memmove(ck_cmdline + size, ck_cmdline + 4,
>+					strlen(cmdline) - (ck_cmdline + 4 - cmdline)
>+ 1);
>+				memcpy(ck_cmdline, tmp, size);
>+			}

Here the variable "size" has two different meanings. First it used for crash size. Then its meaning is changed to buffer size in ck_cmdline. And types are different too. The type for crash size is unsigned long long. But scnprintf() return int.

Could you use two variables to represent the two meanings for less confusion?

Thanks.

-Fenghua

Re: [Patch 2/7] x86: implement crashkernel=auto

[Amerigo Wang]

Yu, Fenghua wrote:
>> +#ifdef CONFIG_KEXEC_AUTO_RESERVE
>> +	if (strncmp(ck_cmdline, "auto", 4) == 0) {
>> +		unsigned long long size;
>> +		char tmp[32];
>> +
>> +		size = arch_default_crash_size(system_ram);
>> +		if (size != 0) {
>> +			*crash_size = size;
>> +			*crash_base = arch_default_crash_base();
>> +			size = scnprintf(tmp, sizeof(tmp), "%luM@%luM",
>> +					(unsigned long)(*crash_size)>>20,
>> +					(unsigned long)(*crash_base)>>20);
>> +			/* size can't be <= 4. */
>> +			if (likely((size - 4 + strlen(cmdline))
>> +					< COMMAND_LINE_SIZE - 1)) {
>> +				memmove(ck_cmdline + size, ck_cmdline + 4,
>> +					strlen(cmdline) - (ck_cmdline + 4 - cmdline)
>> + 1);
>> +				memcpy(ck_cmdline, tmp, size);
>> +			}
>>     
>
> Here the variable "size" has two different meanings. First it used for crash size. Then its meaning is changed to buffer size in ck_cmdline. And types are different too. The type for crash size is unsigned long long. But scnprintf() return int.
>
> Could you use two variables to represent the two meanings for less confusion?
>   

Sure, OK, will do.

Thanks.

[Patch 3/7] ia64: add CONFIG_KEXEC_AUTO_RESERVE

[Amerigo Wang]

Introduce a new config option KEXEC_AUTO_RESERVE for ia64.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
Index: linux-2.6/arch/ia64/Kconfig
===================================================================
--- linux-2.6.orig/arch/ia64/Kconfig
+++ linux-2.6/arch/ia64/Kconfig
@@ -582,6 +582,16 @@ config KEXEC
 	  support.  As of this writing the exact hardware interface is
 	  strongly in flux, so no good recommendation can be made.
 
+config KEXEC_AUTO_RESERVE
+	bool "automatically reserve memory for kexec kernel"
+	depends on KEXEC
+	default y
+	---help---
+	  Automatically reserve memory for a kexec kernel, so that you don't
+	  need to specify numbers for the "crashkernel=X@Y" boot option,
+	  instead you can use "crashkernel=auto".
+	  On IA64, 256M is reserved.
+
 config CRASH_DUMP
 	  bool "kernel crash dumps"
 	  depends on IA64_MCA_RECOVERY && !IA64_HP_SIM && (!SMP || HOTPLUG_CPU)

Re: [Patch 3/7] ia64: add CONFIG_KEXEC_AUTO_RESERVE

[Neil Horman]

On Wed, Aug 05, 2009 at 07:19:31AM -0400, Amerigo Wang wrote:
> 
> Introduce a new config option KEXEC_AUTO_RESERVE for ia64.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> 
> ---
> Index: linux-2.6/arch/ia64/Kconfig
> ===================================================================
> --- linux-2.6.orig/arch/ia64/Kconfig
> +++ linux-2.6/arch/ia64/Kconfig
> @@ -582,6 +582,16 @@ config KEXEC
>  	  support.  As of this writing the exact hardware interface is
>  	  strongly in flux, so no good recommendation can be made.
>  
> +config KEXEC_AUTO_RESERVE
> +	bool "automatically reserve memory for kexec kernel"
> +	depends on KEXEC
> +	default y
> +	---help---
> +	  Automatically reserve memory for a kexec kernel, so that you don't
> +	  need to specify numbers for the "crashkernel=X@Y" boot option,
> +	  instead you can use "crashkernel=auto".
> +	  On IA64, 256M is reserved.
> +
>  config CRASH_DUMP
>  	  bool "kernel crash dumps"
>  	  depends on IA64_MCA_RECOVERY && !IA64_HP_SIM && (!SMP || HOTPLUG_CPU)

Acked-by: Neil Horman <nhorman@tuxdriver.com>

[Patch 4/7] ia64: implement crashkernel=auto

[Amerigo Wang]

Since in patch 2/7 we already implement the generic part, this will
add the rest part for ia64.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
Index: linux-2.6/arch/ia64/include/asm/kexec.h
===================================================================
--- linux-2.6.orig/arch/ia64/include/asm/kexec.h
+++ linux-2.6/arch/ia64/include/asm/kexec.h
@@ -19,6 +19,29 @@
                 flush_icache_range(page_addr, page_addr + PAGE_SIZE); \
         } while(0)
 
+#ifdef CONFIG_KEXEC_AUTO_RESERVE
+#ifndef KEXEC_AUTO_RESERVED_SIZE
+#define KEXEC_AUTO_RESERVED_SIZE 1ULL<<28 /* 256M */
+#endif
+#ifndef KEXEC_AUTO_THRESHOLD
+#define KEXEC_AUTO_THRESHOLD 1ULL<<32 /* 4G */
+#endif
+static inline
+unsigned long long arch_default_crash_size(unsigned long long total_size)
+{
+	if (total_size < KEXEC_AUTO_THRESHOLD)
+		return 0;
+	else
+		return KEXEC_AUTO_RESERVED_SIZE;
+}
+static inline
+unsigned long long arch_default_crash_base(void)
+{
+	/* On ia64, 0 means find the base address automatically. */
+	return 0;
+}
+#endif
+
 extern struct kimage *ia64_kimage;
 extern const unsigned int relocate_new_kernel_size;
 extern void relocate_new_kernel(unsigned long, unsigned long,

Re: [Patch 4/7] ia64: implement crashkernel=auto

[Neil Horman]

On Wed, Aug 05, 2009 at 07:19:41AM -0400, Amerigo Wang wrote:
> 
> Since in patch 2/7 we already implement the generic part, this will
> add the rest part for ia64.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> 
> ---
> Index: linux-2.6/arch/ia64/include/asm/kexec.h
> ===================================================================
> --- linux-2.6.orig/arch/ia64/include/asm/kexec.h
> +++ linux-2.6/arch/ia64/include/asm/kexec.h
> @@ -19,6 +19,29 @@
>                  flush_icache_range(page_addr, page_addr + PAGE_SIZE); \
>          } while(0)
>  
> +#ifdef CONFIG_KEXEC_AUTO_RESERVE
> +#ifndef KEXEC_AUTO_RESERVED_SIZE
> +#define KEXEC_AUTO_RESERVED_SIZE 1ULL<<28 /* 256M */
> +#endif
> +#ifndef KEXEC_AUTO_THRESHOLD
> +#define KEXEC_AUTO_THRESHOLD 1ULL<<32 /* 4G */
> +#endif
> +static inline
> +unsigned long long arch_default_crash_size(unsigned long long total_size)
> +{
> +	if (total_size < KEXEC_AUTO_THRESHOLD)
> +		return 0;
> +	else
> +		return KEXEC_AUTO_RESERVED_SIZE;
> +}
> +static inline
> +unsigned long long arch_default_crash_base(void)
> +{
> +	/* On ia64, 0 means find the base address automatically. */
> +	return 0;
> +}
> +#endif
> +
>  extern struct kimage *ia64_kimage;
>  extern const unsigned int relocate_new_kernel_size;
>  extern void relocate_new_kernel(unsigned long, unsigned long,


Scratch my previous comment about the other arches, I see you're doing them in
individual patches here.  You should probably re-organize the set so that these
definitions are in place prior to the addition of the call sites though (just to
avoid inadvertent build breaks if someone tries to turn AUTO_RESERE on in the
middle of this commit series.  And I still think making this a definition in
asm-generic might be worth while, since it seems to be the same or simmilar for
all arches.

Neil

[Patch 5/7] powerpc: add CONFIG_KEXEC_AUTO_RESERVE

[Amerigo Wang]

Introduce a new config option KEXEC_AUTO_RESERVE for powerpc.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
Index: linux-2.6/arch/powerpc/Kconfig
===================================================================
--- linux-2.6.orig/arch/powerpc/Kconfig
+++ linux-2.6/arch/powerpc/Kconfig
@@ -346,6 +346,16 @@ config KEXEC
 	  support.  As of this writing the exact hardware interface is
 	  strongly in flux, so no good recommendation can be made.
 
+config KEXEC_AUTO_RESERVE
+	bool "automatically reserve memory for kexec kernel"
+	depends on KEXEC
+	default y
+	---help---
+	  Automatically reserve memory for a kexec kernel, so that you don't
+	  need to specify numbers for the "crashkernel=X@Y" boot option,
+	  instead you can use "crashkernel=auto".
+	  On PPC, 256M is reserved and only when you have memory > 4G.
+
 config CRASH_DUMP
 	bool "Build a kdump crash kernel"
 	depends on PPC64 || 6xx

Re: [Patch 5/7] powerpc: add CONFIG_KEXEC_AUTO_RESERVE

[Neil Horman]

On Wed, Aug 05, 2009 at 07:19:51AM -0400, Amerigo Wang wrote:
> 
> Introduce a new config option KEXEC_AUTO_RESERVE for powerpc.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> 
> ---
> Index: linux-2.6/arch/powerpc/Kconfig
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/Kconfig
> +++ linux-2.6/arch/powerpc/Kconfig
> @@ -346,6 +346,16 @@ config KEXEC
>  	  support.  As of this writing the exact hardware interface is
>  	  strongly in flux, so no good recommendation can be made.
>  
> +config KEXEC_AUTO_RESERVE
> +	bool "automatically reserve memory for kexec kernel"
> +	depends on KEXEC
> +	default y
> +	---help---
> +	  Automatically reserve memory for a kexec kernel, so that you don't
> +	  need to specify numbers for the "crashkernel=X@Y" boot option,
> +	  instead you can use "crashkernel=auto".
> +	  On PPC, 256M is reserved and only when you have memory > 4G.
> +
>  config CRASH_DUMP
>  	bool "Build a kdump crash kernel"
>  	depends on PPC64 || 6xx

Acked-by: Neil Horman <nhorman@tuxdriver.com>

[Patch 6/7] powerpc: implement crashkernel=auto

[Amerigo Wang]

Since in patch 2/7 we already implement the generic part, this will
add the rest part for powerpc.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
Index: linux-2.6/arch/powerpc/include/asm/kexec.h
===================================================================
--- linux-2.6.orig/arch/powerpc/include/asm/kexec.h
+++ linux-2.6/arch/powerpc/include/asm/kexec.h
@@ -39,6 +39,29 @@ typedef void (*crash_shutdown_t)(void);
 
 #ifdef CONFIG_KEXEC
 
+#ifdef CONFIG_KEXEC_AUTO_RESERVE
+#ifndef KEXEC_AUTO_RESERVED_SIZE
+#define KEXEC_AUTO_RESERVED_SIZE 1ULL<<28 /* 256M */
+#endif
+#ifndef KEXEC_AUTO_THRESHOLD
+#define KEXEC_AUTO_THRESHOLD 1ULL<<32 /* 4G */
+#endif
+static inline
+unsigned long long arch_default_crash_size(unsigned long long total_size)
+{
+	if (total_size < KEXEC_AUTO_THRESHOLD)
+		return 0;
+	else
+		return KEXEC_AUTO_RESERVED_SIZE;
+}
+static inline
+unsigned long long arch_default_crash_base(void)
+{
+	/* On ppc, 0 means find the base address automatically. */
+	return 0;
+}
+#endif
+
 /*
  * This function is responsible for capturing register states if coming
  * via panic or invoking dump using sysrq-trigger.

Re: [Patch 6/7] powerpc: implement crashkernel=auto

[Neil Horman]

On Wed, Aug 05, 2009 at 07:20:00AM -0400, Amerigo Wang wrote:
> 
> Since in patch 2/7 we already implement the generic part, this will
> add the rest part for powerpc.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> 
> ---
> Index: linux-2.6/arch/powerpc/include/asm/kexec.h
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/include/asm/kexec.h
> +++ linux-2.6/arch/powerpc/include/asm/kexec.h
> @@ -39,6 +39,29 @@ typedef void (*crash_shutdown_t)(void);
>  
>  #ifdef CONFIG_KEXEC
>  
> +#ifdef CONFIG_KEXEC_AUTO_RESERVE
> +#ifndef KEXEC_AUTO_RESERVED_SIZE
> +#define KEXEC_AUTO_RESERVED_SIZE 1ULL<<28 /* 256M */
> +#endif
> +#ifndef KEXEC_AUTO_THRESHOLD
> +#define KEXEC_AUTO_THRESHOLD 1ULL<<32 /* 4G */
> +#endif
> +static inline
> +unsigned long long arch_default_crash_size(unsigned long long total_size)
> +{
> +	if (total_size < KEXEC_AUTO_THRESHOLD)
> +		return 0;
> +	else
> +		return KEXEC_AUTO_RESERVED_SIZE;
> +}
> +static inline
> +unsigned long long arch_default_crash_base(void)
> +{
> +	/* On ppc, 0 means find the base address automatically. */
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * This function is responsible for capturing register states if coming
>   * via panic or invoking dump using sysrq-trigger.

Same comment here, this looks like it really belongs in asm-generic to me.
Neil

[Patch 7/7] doc: update the kdump document

[Amerigo Wang]

Update the document for kdump.

Signed-off-by: WANG Cong <amwang@redhat.com>

---
Index: linux-2.6/Documentation/kdump/kdump.txt
===================================================================
--- linux-2.6.orig/Documentation/kdump/kdump.txt
+++ linux-2.6/Documentation/kdump/kdump.txt
@@ -147,6 +147,15 @@ System kernel config options
    analysis tools require a vmlinux with debug symbols in order to read
    and analyze a dump file.
 
+4) Enable "automatically reserve memory for kexec kernel" in
+   "Processor type and features."
+
+   CONFIG_KEXEC_AUTO_RESERVE=y
+
+   This will let you to use "crashkernel=auto", instead of specifying
+   numbers for "crashkernel=". Note, you need to have enough memory.
+   The threshold and reserved memory size are arch-dependent.
+
 Dump-capture kernel config options (Arch Independent)
 -----------------------------------------------------
 
@@ -266,6 +275,13 @@ This would mean:
     2) if the RAM size is between 512M and 2G (exclusive), then reserve 64M
     3) if the RAM size is larger than 2G, then reserve 128M
 
+Or you can use:
+
+    crashkernel=auto
+
+if you have enough memory. The threshold is 4G, below which this won't work.
+Also the automatically reserved memory size would be 128M on x86, 256M on
+other platforms that have KEXEC.
 
 
 Boot into System Kernel

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Amerigo Wang <amwang@redhat.com> writes:

> This series of patch implements automatically reserved memory for crashkernel,
> by introducing a new boot option "crashkernel=auto". This idea is from Neil.
>
> In case of breaking user-space applications, it modifies this boot option after
> it decides how much memory should be reserved.
>
> On different arch, the threshold and reserved memory size is different. Please
> refer patch 7/7 which contains an update for the documentation.
>
> Note: This patchset was only tested on x86_64 with differernt memory sizes.

This seems like a silly hard code. Especially for a feature distros don't
care enough about to implement a working initrd for.

Has anyone bothered to justify those large amounts of memory?
Where does the 128M go?

Please pardon me for being a cynic but I don't see the command line option
being the bottleneck for real users to make this work.

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Neil Horman]

On Wed, Aug 05, 2009 at 06:33:57AM -0700, Eric W. Biederman wrote:
> Amerigo Wang <amwang@redhat.com> writes:
> 
> > This series of patch implements automatically reserved memory for crashkernel,
> > by introducing a new boot option "crashkernel=auto". This idea is from Neil.
> >
> > In case of breaking user-space applications, it modifies this boot option after
> > it decides how much memory should be reserved.
> >
> > On different arch, the threshold and reserved memory size is different. Please
> > refer patch 7/7 which contains an update for the documentation.
> >
> > Note: This patchset was only tested on x86_64 with differernt memory sizes.
> 
> This seems like a silly hard code. Especially for a feature distros don't
> care enough about to implement a working initrd for.
> 
> Has anyone bothered to justify those large amounts of memory?
> Where does the 128M go?
> 
> Please pardon me for being a cynic but I don't see the command line option
> being the bottleneck for real users to make this work.
> 
> Eric

Lots of the impetus behind this results from a desire to have kexec configured
and setup up during install.  Having the kernel allocate a default size block of
RAM lets you do that without the need for an interim reboot.  You could of
course boot the installer kernel with a crashkernel line pre-selected  suppose,
but then you have to go to the trouble of figuring that allocation size out each
time.  This gives you a nice convienent way to get a reasonable block of memory
without the need to do all that extra work.
Neil

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Neil Horman <nhorman@redhat.com> writes:

> On Wed, Aug 05, 2009 at 06:33:57AM -0700, Eric W. Biederman wrote:
>> Amerigo Wang <amwang@redhat.com> writes:
>> 
>> > This series of patch implements automatically reserved memory for crashkernel,
>> > by introducing a new boot option "crashkernel=auto". This idea is from Neil.
>> >
>> > In case of breaking user-space applications, it modifies this boot option after
>> > it decides how much memory should be reserved.
>> >
>> > On different arch, the threshold and reserved memory size is different. Please
>> > refer patch 7/7 which contains an update for the documentation.
>> >
>> > Note: This patchset was only tested on x86_64 with differernt memory sizes.
>> 
>> This seems like a silly hard code. Especially for a feature distros don't
>> care enough about to implement a working initrd for.
>> 
>> Has anyone bothered to justify those large amounts of memory?
>> Where does the 128M go?
>> 
>> Please pardon me for being a cynic but I don't see the command line option
>> being the bottleneck for real users to make this work.
>> 
>> Eric
>
> Lots of the impetus behind this results from a desire to have kexec configured
> and setup up during install.  Having the kernel allocate a default size block of
> RAM lets you do that without the need for an interim reboot.

I assume you mean kexec on panic.  kexec should be fine and you can arguably
solve this problem with a little bit of userspace glue and a kexec of yourself
during bootup.

> You could of
> course boot the installer kernel with a crashkernel line pre-selected  suppose,
> but then you have to go to the trouble of figuring that allocation size out each
> time.  This gives you a nice convienent way to get a reasonable block of memory
> without the need to do all that extra work.

My big concern is that you are moving policy into the kernel, when it isn't at
all clear that policy is the right thing to do, and the existing mechanisms give
you enough rope to do this all in userspace.

You also have to build (or at least load) the whole kdump image after
the system boots, and configure someplace for this to be saved.

What class of problems do you expect to catch with this?

What has me puzzled is that the mkdumprd that ships with fedora isn't
usable without patching, and it seems to be steadily getting worse.  If the
concern was about getting better bug reports I would expect getting this
functionality into fedora would be where you would be focusing your efforts.

Am I missing something obvious?

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> Neil Horman <nhorman@redhat.com> writes:
>   
>> You could of
>> course boot the installer kernel with a crashkernel line pre-selected  suppose,
>> but then you have to go to the trouble of figuring that allocation size out each
>> time.  This gives you a nice convienent way to get a reasonable block of memory
>> without the need to do all that extra work.
>>     
>
> My big concern is that you are moving policy into the kernel, when it isn't at
> all clear that policy is the right thing to do, and the existing mechanisms give
> you enough rope to do this all in userspace.
>   


How? This doesn't remove the existing mechanism, just provides a new 
choice for user like me who doesn't know how much memory should be 
reserved, or who simply doesn't want to concern this since he/she has 
very enough memory.

> You also have to build (or at least load) the whole kdump image after
> the system boots, and configure someplace for this to be saved.
>
> What class of problems do you expect to catch with this?
>   

Again, try to save the user from choosing numbers for "crashkernel=".
> What has me puzzled is that the mkdumprd that ships with fedora isn't
> usable without patching, and it seems to be steadily getting worse.  

Please explain why it is not usable? The patch won't break the 
userspace, since it modifies the "crashkernel=" command line dynamically.

Thanks.

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Amerigo Wang <amwang@redhat.com> writes:

> Eric W. Biederman wrote:
>> Neil Horman <nhorman@redhat.com> writes:
>>   
>>> You could of
>>> course boot the installer kernel with a crashkernel line pre-selected  suppose,
>>> but then you have to go to the trouble of figuring that allocation size out each
>>> time.  This gives you a nice convienent way to get a reasonable block of memory
>>> without the need to do all that extra work.
>>>     
>>
>> My big concern is that you are moving policy into the kernel, when it isn't at
>> all clear that policy is the right thing to do, and the existing mechanisms give
>> you enough rope to do this all in userspace.
>>   
>
>
> How? This doesn't remove the existing mechanism, just provides a new choice for
> user like me who doesn't know how much memory should be reserved, or who simply
> doesn't want to concern this since he/she has very enough memory.

Having end users not caring is fine.  My problem with this patch is that it
appears that no one is stepping up and taking responsibility for the crahsdump
mechanism, ensuring it will work, ensuring it will work for users.  I see
us solving problems in the wrong place because people are not stepping
up and solving them in the proper place.

To actually use it there are much bigger problems then supplying the size
of the crashdump area.

My personal experience is that to make this work I had to list every kernel
module I needed in the order that they must be loaded, in /etc/kdump.conf
for my filesystem.  I had to modify mkdumprd so that it actually generated
a dump ramdisk, and I think I am forgetting some other manual hack that
I had to use as well.

So if you have to do something manually I think the problem is user space,
and not the kernel.

In general I figure that whoever builds the kernel and initrd should be
responsible for testing and figuring out the amount of memory needed.
The primary kernel has no idea what is going to loaded in there and
as such no real idea how much memory is needed.

>> You also have to build (or at least load) the whole kdump image after
>> the system boots, and configure someplace for this to be saved.
>>
>> What class of problems do you expect to catch with this?
>>   
>
> Again, try to save the user from choosing numbers for "crashkernel=".

The user being kernel developers?  Whoever builds the kernel and initrd
should be responsible for testing and figuring this out.

In a distro context installers etc should be able to setup good defaults
so end users don't have to worry about this.

>> What has me puzzled is that the mkdumprd that ships with fedora isn't
>> usable without patching, and it seems to be steadily getting worse.  
>
> Please explain why it is not usable? The patch won't break the userspace, since
> it modifies the "crashkernel=" command line dynamically.

No the crashdump mechanism is useless because user space is already
broken and unusable.  At least on fedora and I assume by extension
redhat.

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> In general I figure that whoever builds the kernel and initrd should be
> responsible for testing and figuring out the amount of memory needed.
> The primary kernel has no idea what is going to loaded in there and
> as such no real idea how much memory is needed.
>   

Yeah, that is exactly why I _didn't_ pick the idea of reserving memory 
automatically and silently without "crashkernel=auto".

If a user specifies "crashkernel=auto", that means he/she has no idea 
how much memory to be reserved, he/she wants to let the kernel to 
decide. Kernel should know better than the user in this situation.

>>> You also have to build (or at least load) the whole kdump image after
>>> the system boots, and configure someplace for this to be saved.
>>>
>>> What class of problems do you expect to catch with this?
>>>   
>>>       
>> Again, try to save the user from choosing numbers for "crashkernel=".
>>     
>
> The user being kernel developers?  Whoever builds the kernel and initrd
> should be responsible for testing and figuring this out.
>
> In a distro context installers etc should be able to setup good defaults
> so end users don't have to worry about this.
>
>   
For kernel developers, "crashkernel=auto" should save a lot. You seem 
agree with this one.

For users, they rely on the distro which can always specify 
"crashkernel=auto" now, not different numbers for different arch, since 
"crashkernel=auto" is designed to be safe for all cases. Also saves many 
work...

>>> What has me puzzled is that the mkdumprd that ships with fedora isn't
>>> usable without patching, and it seems to be steadily getting worse.  
>>>       
>> Please explain why it is not usable? The patch won't break the userspace, since
>> it modifies the "crashkernel=" command line dynamically.
>>     
>
> No the crashdump mechanism is useless because user space is already
> broken and unusable.

Again, why broken?

Thanks.

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Amerigo Wang <amwang@redhat.com> writes:

>> No the crashdump mechanism is useless because user space is already
>> broken and unusable.
>
> Again, why broken?

To get a stock stat drive by hand I had to list about 5 kernel modules
in the right magic order in /etc/kdump.conf

Neither mount by label or mount by uuid when specified in /etc/kdump.conf
I had to hack mkdumprd to get an initrd that even finds the proper disk
to mount.

Short version it takes a huge amount of expertise to get what ships with
fedora to pass the trivial alt-sysrq-c test.  It would probably be about
as easy to write you own custom initrd by hand.

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> Amerigo Wang <amwang@redhat.com> writes:
>
>   
>>> No the crashdump mechanism is useless because user space is already
>>> broken and unusable.
>>>       
>> Again, why broken?
>>     
>
> To get a stock stat drive by hand I had to list about 5 kernel modules
> in the right magic order in /etc/kdump.conf
>
> Neither mount by label or mount by uuid when specified in /etc/kdump.conf
> I had to hack mkdumprd to get an initrd that even finds the proper disk
> to mount.
>   

You are saying that there is some difficulty to make a initrd for kdump, 
but I am sorry that I can't see any relations between this and my patch. 
What is your point here?

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Amerigo Wang <amwang@redhat.com> writes:

> Eric W. Biederman wrote:
>> Amerigo Wang <amwang@redhat.com> writes:
>>
>>   
>>>> No the crashdump mechanism is useless because user space is already
>>>> broken and unusable.
>>>>       
>>> Again, why broken?
>>>     
>>
>> To get a stock stat drive by hand I had to list about 5 kernel modules
>> in the right magic order in /etc/kdump.conf
>>
>> Neither mount by label or mount by uuid when specified in /etc/kdump.conf
>> I had to hack mkdumprd to get an initrd that even finds the proper disk
>> to mount.
>>   
>
> You are saying that there is some difficulty to make a initrd for kdump, but I
> am sorry that I can't see any relations between this and my patch. What is your
> point here?

You are trying to make it easier for end users.

I am saying the problem is in user space.

I am saying also that the kernel doesn't have a clue what you are
going to load with kexec on panic to handle panics.  Maybe it is a
custom stand alone binary that only needs 5K.  So the kernel doesn't
have a clue what the right size to reserve.

I think if what you were proposing was part of some coherent story for
a complete implementation I would consider it more.  Instead this just
appears to be a reaction to how frustrating the user space
implementation is, and fixing things in the kernel instead of in user
space.

The fact that user space is broken to the point of usability on fedora
simply reinforces the point to me that the problem is there not in the
kernel.  So I am pushing back and saying get your user space act together
and then this kernel option won't matter.

I am further saying that this selecting how much memory to use is the least
of your problems.

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> Amerigo Wang <amwang@redhat.com> writes:
>
>   
>> Eric W. Biederman wrote:
>>     
>>> Amerigo Wang <amwang@redhat.com> writes:
>>>
>>>   
>>>       
>>>>> No the crashdump mechanism is useless because user space is already
>>>>> broken and unusable.
>>>>>       
>>>>>           
>>>> Again, why broken?
>>>>     
>>>>         
>>> To get a stock stat drive by hand I had to list about 5 kernel modules
>>> in the right magic order in /etc/kdump.conf
>>>
>>> Neither mount by label or mount by uuid when specified in /etc/kdump.conf
>>> I had to hack mkdumprd to get an initrd that even finds the proper disk
>>> to mount.
>>>   
>>>       
>> You are saying that there is some difficulty to make a initrd for kdump, but I
>> am sorry that I can't see any relations between this and my patch. What is your
>> point here?
>>     
>
> You are trying to make it easier for end users.
>
> I am saying the problem is in user space.
>
> I am saying also that the kernel doesn't have a clue what you are
> going to load with kexec on panic to handle panics.  Maybe it is a
> custom stand alone binary that only needs 5K.  So the kernel doesn't
> have a clue what the right size to reserve.
>   

So what? If you have 8G memory, would you mind 128M-5K memory to be wasted?

The kernel doesn't have to reserve the exact amount of memory that a 
kexec kernel will use, it just finds a big enough size for all cases 
which already assumes the physical memory is large enough.
> I think if what you were proposing was part of some coherent story for
> a complete implementation I would consider it more.  Instead this just
> appears to be a reaction to how frustrating the user space
> implementation is, and fixing things in the kernel instead of in user
> space.
>   

Yes, exactly, in fact I am doing another part which will allow us to 
take back of the reserved memory at run-time.

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Amerigo Wang <amwang@redhat.com> writes:


> The kernel doesn't have to reserve the exact amount of memory that a kexec
> kernel will use, it just finds a big enough size for all cases which already
> assumes the physical memory is large enough.


>> I think if what you were proposing was part of some coherent story for
>> a complete implementation I would consider it more.  Instead this just
>> appears to be a reaction to how frustrating the user space
>> implementation is, and fixing things in the kernel instead of in user
>> space.
>>   
>
> Yes, exactly, in fact I am doing another part which will allow us to take back
> of the reserved memory at run-time.

Alright. Let's look at that.

I would make the restriction you can't resize the area while a kexec
on panic image is loaded, and growing the area would not be a
realistic option.

If crash_kernel=auto happens in the context of being able to shrink
the area from user space the definition is simple.  We reserve as much
memory as we think we can without affecting performance, stability,
reliability.

We can use an initial approximation of perhaps 1/32nd of low memory
(aka directly mapped memory), and I don't see a point in making the
code arch dependent at all.  We should run the size approximation past
the folks on linux-mm as they are more likely to know how much memory
reduction we can tolerate without problems.

We can then plan on user space saying hey that is more than I need:
shrink that, and load the kexec on panic kernel.

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Hmm.  I half take it back.  How is crashkernel=auto and then shrinking
the reserved size better than the extended syntax Bernhard Walle
introduced nearly two years ago?

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> Hmm.  I half take it back.  How is crashkernel=auto and then shrinking
> the reserved size better than the extended syntax Bernhard Walle
> introduced nearly two years ago?
>   

You mean something like crashkernel=512M-2G:64M,2G-:128M?
Isn't it longer and more complex than "crashkernel=auto" for an end user?

Hmm, this makes me to think that "crashkernel=auto" can be a replacement 
for that extended syntax...

Re: [Patch 0/7] Implement crashkernel=auto

[Bernhard Walle]

Eric W. Biederman schrieb:
> Hmm.  I half take it back.  How is crashkernel=auto and then shrinking
> the reserved size better than the extended syntax Bernhard Walle
> introduced nearly two years ago?

BTW: Ubuntu ships by default with

	crashkernel=384M-2G:64M@16M,2G-:128M@16M

What is the complexity for the user? I didn't edit /boot/grub/menu.lst,
I just installed kexec-tools and a few other kdump-related packages, and
then this was in my menu.lst.

Don't make the kernel complex. Make the userspace complex.


Regards,
Bernhard

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> Amerigo Wang <amwang@redhat.com> writes:
>   
>> Yes, exactly, in fact I am doing another part which will allow us to take back
>> of the reserved memory at run-time.
>>     
>
> Alright. Let's look at that.
>
> I would make the restriction you can't resize the area while a kexec
> on panic image is loaded, and growing the area would not be a
> realistic option.
>
>   
Sure, I have no plan to do growing reserved memory at run-time... only 
freeing  or shrinking it...

> If crash_kernel=auto happens in the context of being able to shrink
> the area from user space the definition is simple.  We reserve as much
> memory as we think we can without affecting performance, stability,
> reliability.
>
> We can use an initial approximation of perhaps 1/32nd of low memory
> (aka directly mapped memory), and I don't see a point in making the
> code arch dependent at all.  We should run the size approximation past
> the folks on linux-mm as they are more likely to know how much memory
> reduction we can tolerate without problems.
>
>   

Yup, agreed.

> We can then plan on user space saying hey that is more than I need:
> shrink that, and load the kexec on panic kernel.
>   

Exactly... but the interface still needs to be discussed...

Currently, we have two options:

1) add a new flag to kexec_load(2) to tell the kernel to shrink the memory;
2) use /proc/iomem, let the user to decide which and how much of the 
reserved memory should be removed.

Any thoughts?

Thanks.

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Let me put this concrete proposal on the table.

The problem:

With the current set of crashkernel= options we are asking the
distribution installer to perform magic.  Moving as much of this logic
into a normal init script for better maintenance is desirable.

My proposal:

Implement crashkernel=max which reserves as much memory as is
reasonable for a crash kernel, without seriously affecting stability,
performance, and reliability.

As an initial approximation I would use a 32nd of low memory.

In addition implement:

/sys/kernel/crash_size

That can be written to (with enough privileges when no crash kernel is
loaded) reduce the amount of memory reserved by the crash kernel.

Bernhard does that sound useful to you?

Amerigo does that seem reasonable?

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Andi Kleen]

> As an initial approximation I would use a 32nd of low memory.

That means a 1TB machine will have a 32GB crash kernel.

Surely that's excessive?!?

It would be repeating all the same mistakes people made with hash tables
several years ago.

> 
> That can be written to (with enough privileges when no crash kernel is
> loaded) reduce the amount of memory reserved by the crash kernel.
> 
> Bernhard does that sound useful to you?
> 
> Amerigo does that seem reasonable?

It doesn't sound reasonable to Andi.

Why do you even want to grow the crash kernel that much? Is there
any real problem with a 64-128MB crash kernel?

-Andi
> 

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [Patch 0/7] Implement crashkernel=auto

[Bernhard Walle]

Andi Kleen schrieb:
>> As an initial approximation I would use a 32nd of low memory.
> 
> That means a 1TB machine will have a 32GB crash kernel.
> 
> Surely that's excessive?!?
> 
> It would be repeating all the same mistakes people made with hash tables
> several years ago.

The idea of Eric was to shrink the reserved memory in an init script. I
doubt that the 1 TB machine will have any problems or performance issue
when booting with (1 TB - 32 GB) memory.

> It doesn't sound reasonable to Andi.
> 
> Why do you even want to grow the crash kernel that much? Is there
> any real problem with a 64-128MB crash kernel?

Try it out. No chance for 64-128MB crashkernel on "medium" IA64 machines.


Regards,
Bernhard

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Andi Kleen <andi@firstfloor.org> writes:

>> As an initial approximation I would use a 32nd of low memory.
>
> That means a 1TB machine will have a 32GB crash kernel.
>
> Surely that's excessive?!?
>
> It would be repeating all the same mistakes people made with hash tables
> several years ago.
>
>> 
>> That can be written to (with enough privileges when no crash kernel is
>> loaded) reduce the amount of memory reserved by the crash kernel.
>> 
>> Bernhard does that sound useful to you?
>> 
>> Amerigo does that seem reasonable?
>
> It doesn't sound reasonable to Andi.
>
> Why do you even want to grow the crash kernel that much? Is there
> any real problem with a 64-128MB crash kernel?

Because it is absolutely ridiculous in size and user space will have
to take up the work of trimming back down to something reasonable in
the init script.

At a practical level crash dump userlands do things like fsck
filesystems before they mount them.  For truly large machines there
was a desire to parallelize core dump writing to different disks.  I
don't know if that has been implemented yet, but in that case you
certainly more ram for buffers tends to be useful.

I think if we are going to go beyond having a magic boot command
line (that we have today) that parametrizes the amount of memory
to reserve based on how much memory we have in the system.  We need
to put user space in control.  We can only put user space in control
if we initially reserve too much and let it release the memory it
won't use.

That would allow removing magic from installers and leaving it to 
installed packages.  Which seems a lot more maintainable. 

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Bernhard Walle]

Eric W. Biederman schrieb:
> 
> With the current set of crashkernel= options we are asking the
> distribution installer to perform magic.  Moving as much of this logic
> into a normal init script for better maintenance is desirable.

Not (necessarily) the installer but the program that configures kdump.
system-config-kdump on Red Hat, YaST on SUSE.

> Bernhard does that sound useful to you?

I don't see any problems. I don't know how much effort is it to free
already reserved crashkernel memory, but I guess it's not really
complicated. Maybe that "1/32" should be specified on the command line like


	crashkernel=>>5

(for 1/32*system_memory == system_memory>>5), OTOH I have no real strong
opinion.


Regards,
Bernhard

Re: [Patch 0/7] Implement crashkernel=auto

[Eric W. Biederman]

Bernhard Walle <bernhard.walle@gmx.de> writes:

> Eric W. Biederman schrieb:
>> 
>> With the current set of crashkernel= options we are asking the
>> distribution installer to perform magic.  Moving as much of this logic
>> into a normal init script for better maintenance is desirable.
>
> Not (necessarily) the installer but the program that configures kdump.
> system-config-kdump on Red Hat, YaST on SUSE.

Right.  Somehow I thought YaST was the installer my mistake.

>> Bernhard does that sound useful to you?
>
> I don't see any problems. I don't know how much effort is it to free
> already reserved crashkernel memory, but I guess it's not really
> complicated. 

Right.

> Maybe that "1/32" should be specified on the command line like
>
>
> 	crashkernel=>>5
>
> (for 1/32*system_memory == system_memory>>5), OTOH I have no real strong
> opinion.

The idea is for the system to give us as much as it can stand and
userspace gives the rest back.  The maximum memory any particular
kernel can stand to give up is a tractable kernel level problem, and
we can make it autotune like any other kernel tunable.  What a crash
kernel needs totally depends on the implementation.

Eric

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> Let me put this concrete proposal on the table.
>
> The problem:
>
> With the current set of crashkernel= options we are asking the
> distribution installer to perform magic.  Moving as much of this logic
> into a normal init script for better maintenance is desirable.
>
> My proposal:
>
> Implement crashkernel=max which reserves as much memory as is
> reasonable for a crash kernel, without seriously affecting stability,
> performance, and reliability.
>   

This is almost exactly what I want with crashkernel=auto....
So there's no big difference, except the name.

> As an initial approximation I would use a 32nd of low memory.
>   

Hmm, I think Bernhard's proposal is fine for this case, i.e. we can 
introduce a new syntax, "crashkernel=>>X" which means we reserve 1/2^X 
of system memory.

What do you think?

> In addition implement:
>
> /sys/kernel/crash_size
>
> That can be written to (with enough privileges when no crash kernel is
> loaded) reduce the amount of memory reserved by the crash kernel.
>   

Yeah, this is nice!

Thanks.

Re: [Patch 0/7] Implement crashkernel=auto

[Amerigo Wang]

Eric W. Biederman wrote:
> Amerigo Wang <amwang@redhat.com> writes:
>
>   
>> This series of patch implements automatically reserved memory for crashkernel,
>> by introducing a new boot option "crashkernel=auto". This idea is from Neil.
>>
>> In case of breaking user-space applications, it modifies this boot option after
>> it decides how much memory should be reserved.
>>
>> On different arch, the threshold and reserved memory size is different. Please
>> refer patch 7/7 which contains an update for the documentation.
>>
>> Note: This patchset was only tested on x86_64 with differernt memory sizes.
>>     
>
> This seems like a silly hard code. Especially for a feature distros don't
> care enough about to implement a working initrd for.
>
> Has anyone bothered to justify those large amounts of memory?
> Where does the 128M go?
>   

If 128M is too big, we can make it to be 64M, that is no problem.
I am very open to this. :)

> Please pardon me for being a cynic but I don't see the command line option
> being the bottleneck for real users to make this work.
>
>   
Well, take /me as an example, to be honest, I still have no idea how 
much memory I should reserve for s390/sh, if I would use kdump on sh, it 
*is* my bottleneck.

Thanks.