[PATCH 0/4] ARM kdump fixes

[PATCH 0/4] ARM kdump fixes

[Mika Westerberg]

Hello all,

Here is a set of patches which I hope allows us to build
crashkernels on ARM platforms. The first patch is needed so that we
can get rid of a compiler warning when building /proc/vmcore on
32-bit only platform. I think it should go via Andrew's tree (if
accepted, that is).

Patches 2 and 3 should replace following patches which are in
Russell's patch tracker:

	6121/1 allow passing an ELF64 header to elf_check_arch()
	-> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6121/1

	6123/1 kdump: add CONFIG_CRASH_DUMP Kconfig option
	-> http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=6123/1

The last patch converts reserve_crashkernel() to use memblock.

Thanks,
MW

Mika Westerberg (4):
  proc/vmcore: allow archs to override vmcore_elf_check_arch()
  ARM: provide zero vmcore_elf64_check_arch()
  ARM: add CONFIG_CRASH_DUMP to Kconfig
  ARM: memblock: convert reserve_crashkernel() to use memblock

 arch/arm/Kconfig           |   13 +++++++++++++
 arch/arm/include/asm/elf.h |    2 ++
 arch/arm/kernel/setup.c    |   21 +++++++--------------
 fs/proc/vmcore.c           |    2 +-
 include/linux/crash_dump.h |    9 ++++++++-
 5 files changed, 31 insertions(+), 16 deletions(-)

-- 
1.7.2.3

[PATCH 1/4] proc/vmcore: allow archs to override vmcore_elf_check_arch()

[Mika Westerberg]

From: Mika Westerberg <ext-mika.1.westerberg@nokia.com>

Allow architectures to redefine this macro if needed. This is useful for
example in architectures where 64-bit ELF vmcores are not supported.
Specifying zero vmcore_elf64_check_arch() allows compiler to optimize
away unnecessary parts of parse_crash_elf64_headers().

We also rename the macro to vmcore_elf64_check_arch() to reflect that it
is used for 64-bit vmcores only.

Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
---
 fs/proc/vmcore.c           |    2 +-
 include/linux/crash_dump.h |    9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 2367fb3..74802bc5 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -499,7 +499,7 @@ static int __init parse_crash_elf64_headers(void)
 	/* Do some basic Verification. */
 	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0 ||
 		(ehdr.e_type != ET_CORE) ||
-		!vmcore_elf_check_arch(&ehdr) ||
+		!vmcore_elf64_check_arch(&ehdr) ||
 		ehdr.e_ident[EI_CLASS] != ELFCLASS64 ||
 		ehdr.e_ident[EI_VERSION] != EV_CURRENT ||
 		ehdr.e_version != EV_CURRENT ||
diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 0026f26..088cd4a 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -20,7 +20,14 @@ extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
 #define vmcore_elf_check_arch_cross(x) 0
 #endif
 
-#define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
+/*
+ * Architecture code can redefine this if there are any special checks
+ * needed for 64-bit ELF vmcores. In case of 32-bit only architecture,
+ * this can be set to zero.
+ */
+#ifndef vmcore_elf64_check_arch
+#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
+#endif
 
 /*
  * is_kdump_kernel() checks whether this kernel is booting after a panic of
-- 
1.7.2.3

Re: [PATCH 1/4] proc/vmcore: allow archs to override vmcore_elf_check_arch()

[Mika Westerberg]

Hi Andrew,

Do you have any comments on this patch? If it is ok, could you consider picking
it up?

I should've probably sent it as a separate patch since it is now burried into
"ARM kdump fixes" -thread and thus harder to spot. Sorry about that.

Regards,
MW

On Tue, Nov 09, 2010 at 11:06:10AM +0200, ext Mika Westerberg wrote:
> From: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> 
> Allow architectures to redefine this macro if needed. This is useful for
> example in architectures where 64-bit ELF vmcores are not supported.
> Specifying zero vmcore_elf64_check_arch() allows compiler to optimize
> away unnecessary parts of parse_crash_elf64_headers().
> 
> We also rename the macro to vmcore_elf64_check_arch() to reflect that it
> is used for 64-bit vmcores only.
> 
> Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> ---
>  fs/proc/vmcore.c           |    2 +-
>  include/linux/crash_dump.h |    9 ++++++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 2367fb3..74802bc5 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -499,7 +499,7 @@ static int __init parse_crash_elf64_headers(void)
>  	/* Do some basic Verification. */
>  	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0 ||
>  		(ehdr.e_type != ET_CORE) ||
> -		!vmcore_elf_check_arch(&ehdr) ||
> +		!vmcore_elf64_check_arch(&ehdr) ||
>  		ehdr.e_ident[EI_CLASS] != ELFCLASS64 ||
>  		ehdr.e_ident[EI_VERSION] != EV_CURRENT ||
>  		ehdr.e_version != EV_CURRENT ||
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index 0026f26..088cd4a 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -20,7 +20,14 @@ extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
>  #define vmcore_elf_check_arch_cross(x) 0
>  #endif
>  
> -#define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> +/*
> + * Architecture code can redefine this if there are any special checks
> + * needed for 64-bit ELF vmcores. In case of 32-bit only architecture,
> + * this can be set to zero.
> + */
> +#ifndef vmcore_elf64_check_arch
> +#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> +#endif
>  
>  /*
>   * is_kdump_kernel() checks whether this kernel is booting after a panic of
> -- 
> 1.7.2.3

Re: [PATCH 1/4] proc/vmcore: allow archs to override vmcore_elf_check_arch()

[Andrew Morton]

On Tue,  9 Nov 2010 11:06:10 +0200
Mika Westerberg <mika.westerberg@iki.fi> wrote:

> From: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> 
> Allow architectures to redefine this macro if needed. This is useful for
> example in architectures where 64-bit ELF vmcores are not supported.
> Specifying zero vmcore_elf64_check_arch() allows compiler to optimize
> away unnecessary parts of parse_crash_elf64_headers().
> 
> We also rename the macro to vmcore_elf64_check_arch() to reflect that it
> is used for 64-bit vmcores only.
> 
> Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> ---
>  fs/proc/vmcore.c           |    2 +-
>  include/linux/crash_dump.h |    9 ++++++++-
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> index 2367fb3..74802bc5 100644
> --- a/fs/proc/vmcore.c
> +++ b/fs/proc/vmcore.c
> @@ -499,7 +499,7 @@ static int __init parse_crash_elf64_headers(void)
>  	/* Do some basic Verification. */
>  	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0 ||
>  		(ehdr.e_type != ET_CORE) ||
> -		!vmcore_elf_check_arch(&ehdr) ||
> +		!vmcore_elf64_check_arch(&ehdr) ||
>  		ehdr.e_ident[EI_CLASS] != ELFCLASS64 ||
>  		ehdr.e_ident[EI_VERSION] != EV_CURRENT ||
>  		ehdr.e_version != EV_CURRENT ||
> diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> index 0026f26..088cd4a 100644
> --- a/include/linux/crash_dump.h
> +++ b/include/linux/crash_dump.h
> @@ -20,7 +20,14 @@ extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
>  #define vmcore_elf_check_arch_cross(x) 0
>  #endif
>  
> -#define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> +/*
> + * Architecture code can redefine this if there are any special checks
> + * needed for 64-bit ELF vmcores. In case of 32-bit only architecture,
> + * this can be set to zero.
> + */
> +#ifndef vmcore_elf64_check_arch
> +#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> +#endif
>  

Looks OK to me.  I'd suggest that this patch be merged along with the
others, via whatever tree they're taken into.  Russell's ARM tree, I
assume.

Should elf_check_arch() be renamed to elf64_check_arch()?

Ditto vmcore_elf_check_arch_cross()?

Re: [PATCH 1/4] proc/vmcore: allow archs to override vmcore_elf_check_arch()

[Russell King - ARM Linux]

On Tue, Nov 16, 2010 at 02:28:50PM -0800, Andrew Morton wrote:
> On Tue,  9 Nov 2010 11:06:10 +0200
> Mika Westerberg <mika.westerberg@iki.fi> wrote:
> 
> > From: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> > 
> > Allow architectures to redefine this macro if needed. This is useful for
> > example in architectures where 64-bit ELF vmcores are not supported.
> > Specifying zero vmcore_elf64_check_arch() allows compiler to optimize
> > away unnecessary parts of parse_crash_elf64_headers().
> > 
> > We also rename the macro to vmcore_elf64_check_arch() to reflect that it
> > is used for 64-bit vmcores only.
> > 
> > Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> > ---
> >  fs/proc/vmcore.c           |    2 +-
> >  include/linux/crash_dump.h |    9 ++++++++-
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> > index 2367fb3..74802bc5 100644
> > --- a/fs/proc/vmcore.c
> > +++ b/fs/proc/vmcore.c
> > @@ -499,7 +499,7 @@ static int __init parse_crash_elf64_headers(void)
> >  	/* Do some basic Verification. */
> >  	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0 ||
> >  		(ehdr.e_type != ET_CORE) ||
> > -		!vmcore_elf_check_arch(&ehdr) ||
> > +		!vmcore_elf64_check_arch(&ehdr) ||
> >  		ehdr.e_ident[EI_CLASS] != ELFCLASS64 ||
> >  		ehdr.e_ident[EI_VERSION] != EV_CURRENT ||
> >  		ehdr.e_version != EV_CURRENT ||
> > diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> > index 0026f26..088cd4a 100644
> > --- a/include/linux/crash_dump.h
> > +++ b/include/linux/crash_dump.h
> > @@ -20,7 +20,14 @@ extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
> >  #define vmcore_elf_check_arch_cross(x) 0
> >  #endif
> >  
> > -#define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> > +/*
> > + * Architecture code can redefine this if there are any special checks
> > + * needed for 64-bit ELF vmcores. In case of 32-bit only architecture,
> > + * this can be set to zero.
> > + */
> > +#ifndef vmcore_elf64_check_arch
> > +#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> > +#endif
> >  
> 
> Looks OK to me.  I'd suggest that this patch be merged along with the
> others, via whatever tree they're taken into.  Russell's ARM tree, I
> assume.
> 
> Should elf_check_arch() be renamed to elf64_check_arch()?

Some background...

Well, the reason this has come up with is because someone's brilliant
idea was to pass either a 32-bit or 64-bit ELF header to elf_check_arch,
and hope it worked it out for itself.

This works when elf_check_arch() is a macro, because the structure
members are identically named - the compiler gets to sort it out by
itself.

However, if you have a complex elf_check_arch() (as we do on ARM) which
requires it to be implemented as a C function, you end up with warnings
when the crash dump code wants to pass a 64-bit ELF header into
elf_check_arch().

Now, this would all be simple except that fs/binfmt_elf.c is coded to
be oblivious to whether an architecture is 32-bit or 64-bit - so the
architecture has to decide what kind of header elf_check_arch() actually
takes.  We can't say that elf_check_arch() always takes a 32-bit ELF
header.

I'm not sure what the right way out of this is - other than requiring
the crash dump code to avoid using elf_check_arch() itself, and provide
a pair of new macros - elf32_check_arch() and elf64_check_arch().  For
those architectures (basically everything but ARM) where their elf_check_arch()
is a macro, the elf32 and elf64 versions can be mere preprocessor aliases.
On ARM, we can then have our properly-typed functions.

The last point I'll make is that elf_check_arch() on ARM is also used to
limit the type of user executable that can be run, to prevent binaries
with incompatible floating point implementations from running or where
the support code for the floating point hardware is missing from the
kernel.  Some of these checks probably don't make sense for the crash
dump code.

So, as far as the above patch goes, I think it's sane, as it now
allows architectures to deal with the 64-bit/32-bit ELF issues in the
crash dump code in a more sane manner.

Re: [PATCH 1/4] proc/vmcore: allow archs to override vmcore_elf_check_arch()

[Mika Westerberg]

On Tue, Nov 16, 2010 at 10:50:42PM +0000, ext Russell King - ARM Linux wrote:
[...]
> 
> So, as far as the above patch goes, I think it's sane, as it now
> allows architectures to deal with the 64-bit/32-bit ELF issues in the
> crash dump code in a more sane manner.

Russell,

Is it ok if I submit this patch (along with patches 2 and 3) to your patch
system as Andrew suggested, or do you prefer to merge it via some other tree?

Thanks,
MW

Re: [PATCH 1/4] proc/vmcore: allow archs to override vmcore_elf_check_arch()

[Russell King - ARM Linux]

On Tue, Nov 16, 2010 at 02:28:50PM -0800, Andrew Morton wrote:
> On Tue,  9 Nov 2010 11:06:10 +0200
> Mika Westerberg <mika.westerberg@iki.fi> wrote:
> 
> > From: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> > 
> > Allow architectures to redefine this macro if needed. This is useful for
> > example in architectures where 64-bit ELF vmcores are not supported.
> > Specifying zero vmcore_elf64_check_arch() allows compiler to optimize
> > away unnecessary parts of parse_crash_elf64_headers().
> > 
> > We also rename the macro to vmcore_elf64_check_arch() to reflect that it
> > is used for 64-bit vmcores only.
> > 
> > Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
> > ---
> >  fs/proc/vmcore.c           |    2 +-
> >  include/linux/crash_dump.h |    9 ++++++++-
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
> > index 2367fb3..74802bc5 100644
> > --- a/fs/proc/vmcore.c
> > +++ b/fs/proc/vmcore.c
> > @@ -499,7 +499,7 @@ static int __init parse_crash_elf64_headers(void)
> >  	/* Do some basic Verification. */
> >  	if (memcmp(ehdr.e_ident, ELFMAG, SELFMAG) != 0 ||
> >  		(ehdr.e_type != ET_CORE) ||
> > -		!vmcore_elf_check_arch(&ehdr) ||
> > +		!vmcore_elf64_check_arch(&ehdr) ||
> >  		ehdr.e_ident[EI_CLASS] != ELFCLASS64 ||
> >  		ehdr.e_ident[EI_VERSION] != EV_CURRENT ||
> >  		ehdr.e_version != EV_CURRENT ||
> > diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
> > index 0026f26..088cd4a 100644
> > --- a/include/linux/crash_dump.h
> > +++ b/include/linux/crash_dump.h
> > @@ -20,7 +20,14 @@ extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
> >  #define vmcore_elf_check_arch_cross(x) 0
> >  #endif
> >  
> > -#define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> > +/*
> > + * Architecture code can redefine this if there are any special checks
> > + * needed for 64-bit ELF vmcores. In case of 32-bit only architecture,
> > + * this can be set to zero.
> > + */
> > +#ifndef vmcore_elf64_check_arch
> > +#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
> > +#endif
> >  
> 
> Looks OK to me.  I'd suggest that this patch be merged along with the
> others, via whatever tree they're taken into.  Russell's ARM tree, I
> assume.
> 
> Should elf_check_arch() be renamed to elf64_check_arch()?
> 
> Ditto vmcore_elf_check_arch_cross()?

BTW, it would be nice to have some Acked-by's or other attributations on
this patch, so it looks like it had some review.

[PATCH 2/4] ARM: provide zero vmcore_elf64_check_arch()

[Mika Westerberg]

From: Mika Westerberg <ext-mika.1.westerberg@nokia.com>

Since we don't support 64-bit ELF vmcores. This also prevents the
following warning:

fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
fs/proc/vmcore.c:502: warning: passing argument 1 of 'elf_check_arch'
  from incompatible pointer type

Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
---
 arch/arm/include/asm/elf.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
index 8bb66bc..c3cd875 100644
--- a/arch/arm/include/asm/elf.h
+++ b/arch/arm/include/asm/elf.h
@@ -99,6 +99,8 @@ struct elf32_hdr;
 extern int elf_check_arch(const struct elf32_hdr *);
 #define elf_check_arch elf_check_arch
 
+#define vmcore_elf64_check_arch(x) (0)
+
 extern int arm_elf_read_implies_exec(const struct elf32_hdr *, int);
 #define elf_read_implies_exec(ex,stk) arm_elf_read_implies_exec(&(ex), stk)
 
-- 
1.7.2.3

[PATCH 3/4] ARM: add CONFIG_CRASH_DUMP to Kconfig

[Mika Westerberg]

From: Mika Westerberg <ext-mika.1.westerberg@nokia.com>

Add CONFIG_CRASH_DUMP configuration option which is used by dump
capture kernels.

Signed-off-by: Mika Westerberg <ext-mika.1.westerberg@nokia.com>
---
 arch/arm/Kconfig |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a19a526..78774d9 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1650,6 +1650,19 @@ config ATAGS_PROC
 	  Should the atags used to boot the kernel be exported in an "atags"
 	  file in procfs. Useful with kexec.
 
+config CRASH_DUMP
+	bool "Build kdump crash kernel (EXPERIMENTAL)"
+	depends on EXPERIMENTAL
+	help
+	  Generate crash dump after being started by kexec. This should
+	  be normally only set in special crash dump kernels which are
+	  loaded in the main kernel with kexec-tools into a specially
+	  reserved region and then later executed after a crash by
+	  kdump/kexec. The crash dump kernel must be compiled to a
+	  memory address not used by the main kernel
+
+	  For more details see Documentation/kdump/kdump.txt
+
 config AUTO_ZRELADDR
 	bool "Auto calculation of the decompressed kernel image address"
 	depends on !ZBOOT_ROM && !ARCH_U300
-- 
1.7.2.3

[PATCH 4/4] ARM: memblock: convert reserve_crashkernel() to use memblock

[Mika Westerberg]

Since we now use memblock we can convert reserve_crashkernel() to
take advantage of the memblock API.

While we are at it, fix the typo in the function kerneldoc as well.

Signed-off-by: Mika Westerberg <mika.westerberg@iki.fi>
---
 arch/arm/kernel/setup.c |   21 +++++++--------------
 1 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 336f14e..c477a2e 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -720,16 +720,8 @@ static int __init customize_machine(void)
 arch_initcall(customize_machine);
 
 #ifdef CONFIG_KEXEC
-static inline unsigned long long get_total_mem(void)
-{
-	unsigned long total;
-
-	total = max_low_pfn - min_low_pfn;
-	return total << PAGE_SHIFT;
-}
-
 /**
- * reserve_crashkernel() - reserves memory are for crash kernel
+ * reserve_crashkernel() - reserves memory area for crash kernel
  *
  * This function reserves memory area given in "crashkernel=" kernel command
  * line parameter. The memory reserved is used by a dump capture kernel when
@@ -738,16 +730,17 @@ static inline unsigned long long get_total_mem(void)
 static void __init reserve_crashkernel(void)
 {
 	unsigned long long crash_size, crash_base;
-	unsigned long long total_mem;
 	int ret;
 
-	total_mem = get_total_mem();
-	ret = parse_crashkernel(boot_command_line, total_mem,
+	/* this is necessary because of memblock_phys_mem_size() */
+	memblock_analyze();
+
+	ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
 				&crash_size, &crash_base);
 	if (ret)
 		return;
 
-	ret = reserve_bootmem(crash_base, crash_size, BOOTMEM_EXCLUSIVE);
+	ret = memblock_reserve(crash_base, crash_size);
 	if (ret < 0) {
 		printk(KERN_WARNING "crashkernel reservation failed - "
 		       "memory is in use (0x%lx)\n", (unsigned long)crash_base);
@@ -758,7 +751,7 @@ static void __init reserve_crashkernel(void)
 	       "for crashkernel (System RAM: %ldMB)\n",
 	       (unsigned long)(crash_size >> 20),
 	       (unsigned long)(crash_base >> 20),
-	       (unsigned long)(total_mem >> 20));
+	       (unsigned long)(memblock_phys_mem_size() >> 20));
 
 	crashk_res.start = crash_base;
 	crashk_res.end = crash_base + crash_size - 1;
-- 
1.7.2.3

Re: [PATCH 4/4] ARM: memblock: convert reserve_crashkernel() to use memblock

[Russell King - ARM Linux]

On Tue, Nov 09, 2010 at 11:06:13AM +0200, Mika Westerberg wrote:
>  static void __init reserve_crashkernel(void)
>  {
>  	unsigned long long crash_size, crash_base;
> -	unsigned long long total_mem;
>  	int ret;
>  
> -	total_mem = get_total_mem();
> -	ret = parse_crashkernel(boot_command_line, total_mem,
> +	/* this is necessary because of memblock_phys_mem_size() */
> +	memblock_analyze();

I think you need to check with the memblock people whether its legal to
call memblock_analyze() multiple times.  What do other arches do for
this?

Secondly, when paging_init() returns, bootmem has been initialized, and
memory taken from bootmem to feed the zone allocators.  This memory is
not registered back into memblock.  Allocations from memblock after
paging_init() has returned will lead to overlaps with bootmem, and
therefore corruption.

So, this code is better off left as-is, rather than trying to convert it
to what mistakenly appears "the latest thing".

Re: [PATCH 4/4] ARM: memblock: convert reserve_crashkernel() to use memblock

[Mika Westerberg]

On Sat, Nov 13, 2010 at 01:07:00PM +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 09, 2010 at 11:06:13AM +0200, Mika Westerberg wrote:
> >  static void __init reserve_crashkernel(void)
> >  {
> >  	unsigned long long crash_size, crash_base;
> > -	unsigned long long total_mem;
> >  	int ret;
> >  
> > -	total_mem = get_total_mem();
> > -	ret = parse_crashkernel(boot_command_line, total_mem,
> > +	/* this is necessary because of memblock_phys_mem_size() */
> > +	memblock_analyze();
> 
> I think you need to check with the memblock people whether its legal to
> call memblock_analyze() multiple times.  What do other arches do for
> this?

At least powerpc and sh do it like the above.

> Secondly, when paging_init() returns, bootmem has been initialized, and
> memory taken from bootmem to feed the zone allocators.  This memory is
> not registered back into memblock.  Allocations from memblock after
> paging_init() has returned will lead to overlaps with bootmem, and
> therefore corruption.

Ok.

> So, this code is better off left as-is, rather than trying to convert it
> to what mistakenly appears "the latest thing".

Yeah, sounds like it wasn't such a good idea. Please ignore this
patch then.

Thanks,
MW