[PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')

[PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')

[Vivek Goyal]

Hi All,

How does following series of patches look like. I have moved
elfcorehdr_addr out of vmcore.c and pushed it to arch dependent section 
of crash dump to make sure that it can be worked with even when
CONFIG_PROC_VMCORE is disabled and CONFIG_CRASH_DUMP is enabled.

I tested it on x86_64. Compile tested it on i386 and ppc64. ia64 and
sh versions are completely untested.

Thanks
Vivek




o elfcorehdr_addr is used by not only the code under CONFIG_PROC_VMCORE but
  also by the code which is not inside CONFIG_PROC_VMCORE. For example,
  is_kdump_kernel() is used by powerpc code to determine if kernel is booting
  after a panic then use previous kernel's TCE table. So even if
  CONFIG_PROC_VMCORE is not set in second kernel, one should be able to
  correctly determine that we are booting after a panic and setup calgary
  iommu accordingly.

o So remove the assumption that elfcorehdr_addr is under CONFIG_PROC_VMCORE.

o Move definition of elfcorehdr_addr to arch dependent crash files.
  (Unfortunately crash dump does not have an arch independent file otherwise
   that would have been the best place).

o kexec.c is not the right place as one can Have CRASH_DUMP enabled in
  second kernel without KEXEC being enabled.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---

 fs/proc/vmcore.c           |    3 ---
 include/linux/crash_dump.h |   14 ++++++++++----
 2 files changed, 10 insertions(+), 7 deletions(-)

diff -puN fs/proc/vmcore.c~remove-elfcore-hdr-addr-definition-vmcore fs/proc/vmcore.c
--- linux-2.6.27-pre-rc1/fs/proc/vmcore.c~remove-elfcore-hdr-addr-definition-vmcore	2008-07-28 09:19:50.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/fs/proc/vmcore.c	2008-07-28 09:20:10.000000000 -0400
@@ -32,9 +32,6 @@ static size_t elfcorebuf_sz;
 /* Total size of vmcore file. */
 static u64 vmcore_size;
 
-/* Stores the physical address of elf header of crash image. */
-unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
-
 struct proc_dir_entry *proc_vmcore = NULL;
 
 /* Reads a page from the oldmem device from given offset. */
diff -puN include/linux/crash_dump.h~remove-elfcore-hdr-addr-definition-vmcore include/linux/crash_dump.h
--- linux-2.6.27-pre-rc1/include/linux/crash_dump.h~remove-elfcore-hdr-addr-definition-vmcore	2008-07-28 12:00:44.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/include/linux/crash_dump.h	2008-07-28 12:00:56.000000000 -0400
@@ -9,11 +9,7 @@
 
 #define ELFCORE_ADDR_MAX	(-1ULL)
 
-#ifdef CONFIG_PROC_VMCORE
 extern unsigned long long elfcorehdr_addr;
-#else
-static const unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
-#endif
 
 extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
 						unsigned long, int);
@@ -28,6 +24,16 @@ extern struct proc_dir_entry *proc_vmcor
 
 #define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
 
+/*
+ * is_kdump_kernel() checks whether this kernel is booting after a panic of
+ * previous kernel or not. This is determined by checking if previous kernel
+ * has passed the elf core header address on command line.
+ *
+ * This is not just a test if CONFIG_CRASH_DUMP is enabled or not. It will
+ * return 1 if CONFIG_CRASH_DUMP=y and if kernel is booting after a panic of
+ * previous kernel.
+ */
+
 static inline int is_kdump_kernel(void)
 {
 	return (elfcorehdr_addr != ELFCORE_ADDR_MAX) ? 1 : 0;
_

[PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section

[Vivek Goyal]

o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
  equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
  CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
  used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
  not.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---

 arch/x86/kernel/crash_dump_32.c |    3 +++
 arch/x86/kernel/crash_dump_64.c |    3 +++
 arch/x86/kernel/setup.c         |    8 +++++++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff -puN arch/x86/kernel/setup.c~fix-elfcorehdr_addr-parsing-x86 arch/x86/kernel/setup.c
--- linux-2.6.27-pre-rc1/arch/x86/kernel/setup.c~fix-elfcorehdr_addr-parsing-x86	2008-07-28 12:01:35.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/arch/x86/kernel/setup.c	2008-07-28 12:01:35.000000000 -0400
@@ -558,7 +558,13 @@ static void __init reserve_standard_io_r
 
 }
 
-#ifdef CONFIG_PROC_VMCORE
+/*
+ * Note: elfcorehdr_addr is not just limited to vmcore. It is also used by
+ * is_kdump_kernel() to determine if we are booting after a panic. Hence
+ * ifdef it under CONFIG_CRASH_DUMP and not CONFIG_PROC_VMCORE.
+ */
+
+#ifdef CONFIG_CRASH_DUMP
 /* elfcorehdr= specifies the location of elf core header
  * stored by the crashed kernel. This option will be passed
  * by kexec loader to the capture kernel.
diff -puN arch/x86/kernel/crash_dump_32.c~fix-elfcorehdr_addr-parsing-x86 arch/x86/kernel/crash_dump_32.c
--- linux-2.6.27-pre-rc1/arch/x86/kernel/crash_dump_32.c~fix-elfcorehdr_addr-parsing-x86	2008-07-28 12:01:35.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/arch/x86/kernel/crash_dump_32.c	2008-07-28 12:01:35.000000000 -0400
@@ -13,6 +13,9 @@
 
 static void *kdump_buf_page;
 
+/* Stores the physical address of elf header of crash image. */
+unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
+
 /**
  * copy_oldmem_page - copy one page from "oldmem"
  * @pfn: page frame number to be copied
diff -puN arch/x86/kernel/crash_dump_64.c~fix-elfcorehdr_addr-parsing-x86 arch/x86/kernel/crash_dump_64.c
--- linux-2.6.27-pre-rc1/arch/x86/kernel/crash_dump_64.c~fix-elfcorehdr_addr-parsing-x86	2008-07-28 12:01:35.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/arch/x86/kernel/crash_dump_64.c	2008-07-28 12:01:35.000000000 -0400
@@ -11,6 +11,9 @@
 #include <asm/uaccess.h>
 #include <asm/io.h>
 
+/* Stores the physical address of elf header of crash image. */
+unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
+
 /**
  * copy_oldmem_page - copy one page from "oldmem"
  * @pfn: page frame number to be copied
_

[PATCH 3/5] ia64: Define elfcorehdr_addr in arch dependent section

[Vivek Goyal]

o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
  equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
  CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
  used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
  not.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---

 arch/ia64/kernel/setup.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff -puN arch/ia64/kernel/setup.c~fix-elfcorehdr_addr-parsing-ia64 arch/ia64/kernel/setup.c
--- linux-2.6.27-pre-rc1/arch/ia64/kernel/setup.c~fix-elfcorehdr_addr-parsing-ia64	2008-07-28 12:16:40.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/arch/ia64/kernel/setup.c	2008-07-28 12:16:40.000000000 -0400
@@ -478,7 +478,12 @@ static __init int setup_nomca(char *s)
 }
 early_param("nomca", setup_nomca);
 
-#ifdef CONFIG_PROC_VMCORE
+/*
+ * Note: elfcorehdr_addr is not just limited to vmcore. It is also used by
+ * is_kdump_kernel() to determine if we are booting after a panic. Hence
+ * ifdef it under CONFIG_CRASH_DUMP and not CONFIG_PROC_VMCORE.
+ */
+#ifdef CONFIG_CRASH_DUMP
 /* elfcorehdr= specifies the location of elf core header
  * stored by the crashed kernel.
  */
@@ -491,7 +496,9 @@ static int __init parse_elfcorehdr(char 
 	return 0;
 }
 early_param("elfcorehdr", parse_elfcorehdr);
+#endif
 
+#ifdef CONFIG_PROC_VMCORE
 int __init reserve_elfcorehdr(unsigned long *start, unsigned long *end)
 {
 	unsigned long length;
_

[PATCH 4/5] powerpc: Define elfcorehdr_addr in arch dependent section

[Vivek Goyal]

o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
  equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
  CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
  used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
  not.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---

 arch/powerpc/kernel/crash_dump.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff -puN arch/powerpc/kernel/crash_dump.c~fix-elfcorehdr_addr-parsing-ppc64 arch/powerpc/kernel/crash_dump.c
--- linux-2.6.27-pre-rc1/arch/powerpc/kernel/crash_dump.c~fix-elfcorehdr_addr-parsing-ppc64	2008-07-28 12:14:22.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/arch/powerpc/kernel/crash_dump.c	2008-07-28 12:14:22.000000000 -0400
@@ -27,6 +27,9 @@
 #define DBG(fmt...)
 #endif
 
+/* Stores the physical address of elf header of crash image. */
+unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
+
 void __init reserve_kdump_trampoline(void)
 {
 	lmb_reserve(0, KDUMP_RESERVE_LIMIT);
@@ -66,7 +69,11 @@ void __init setup_kdump_trampoline(void)
 	DBG(" <- setup_kdump_trampoline()\n");
 }
 
-#ifdef CONFIG_PROC_VMCORE
+/*
+ * Note: elfcorehdr_addr is not just limited to vmcore. It is also used by
+ * is_kdump_kernel() to determine if we are booting after a panic. Hence
+ * ifdef it under CONFIG_CRASH_DUMP and not CONFIG_PROC_VMCORE.
+ */
 static int __init parse_elfcorehdr(char *p)
 {
 	if (p)
@@ -75,7 +82,6 @@ static int __init parse_elfcorehdr(char 
 	return 1;
 }
 __setup("elfcorehdr=", parse_elfcorehdr);
-#endif
 
 static int __init parse_savemaxmem(char *p)
 {
_

[PATCH 5/5] sh: Define elfcorehdr_addr in arch dependent section

[Vivek Goyal]

o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
  equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
  CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
  used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
  not.

o I don't see sh setup code parsing the command line for elfcorehdr_addr. I 
  am wondering how does vmcore interface work on sh. Anyway, I am atleast
  defining elfcoredhr_addr so that compilation is not broken on sh.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---

 arch/sh/kernel/crash_dump.c |    3 +++
 1 file changed, 3 insertions(+)

diff -puN arch/sh/kernel/crash_dump.c~fix-elfcorehdr_addr-sh arch/sh/kernel/crash_dump.c
--- linux-2.6.27-pre-rc1/arch/sh/kernel/crash_dump.c~fix-elfcorehdr_addr-sh	2008-07-28 12:17:12.000000000 -0400
+++ linux-2.6.27-pre-rc1-root/arch/sh/kernel/crash_dump.c	2008-07-28 12:17:12.000000000 -0400
@@ -10,6 +10,9 @@
 #include <linux/io.h>
 #include <asm/uaccess.h>
 
+/* Stores the physical address of elf header of crash image. */
+unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
+
 /**
  * copy_oldmem_page - copy one page from "oldmem"
  * @pfn: page frame number to be copied
_

Re: [PATCH 5/5] sh: Define elfcorehdr_addr in arch dependent section

[Paul Mundt]

On Mon, Jul 28, 2008 at 05:15:14PM -0400, Vivek Goyal wrote:
> o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
>   equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
>   CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
>   used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
>   not.
> 
> o I don't see sh setup code parsing the command line for elfcorehdr_addr. I 
>   am wondering how does vmcore interface work on sh. Anyway, I am atleast
>   defining elfcoredhr_addr so that compilation is not broken on sh.
> 
Hmm, you are correct, it seems like it was either lost in a merge
somewhere or I simply neglected to check it in it when I was testing this
stuff initially. Thanks for noticing!

> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>

Acked-by: Paul Mundt <lethal@linux-sh.org>

Re: [PATCH 3/5] ia64: Define elfcorehdr_addr in arch dependent section

[Simon Horman]

On Mon, Jul 28, 2008 at 05:13:14PM -0400, Vivek Goyal wrote:
> 
> o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
>   equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
>   CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
>   used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
>   not.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
> 
>  arch/ia64/kernel/setup.c |    9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff -puN arch/ia64/kernel/setup.c~fix-elfcorehdr_addr-parsing-ia64 arch/ia64/kernel/setup.c
> --- linux-2.6.27-pre-rc1/arch/ia64/kernel/setup.c~fix-elfcorehdr_addr-parsing-ia64	2008-07-28 12:16:40.000000000 -0400
> +++ linux-2.6.27-pre-rc1-root/arch/ia64/kernel/setup.c	2008-07-28 12:16:40.000000000 -0400
> @@ -478,7 +478,12 @@ static __init int setup_nomca(char *s)
>  }
>  early_param("nomca", setup_nomca);
>  
> -#ifdef CONFIG_PROC_VMCORE
> +/*
> + * Note: elfcorehdr_addr is not just limited to vmcore. It is also used by
> + * is_kdump_kernel() to determine if we are booting after a panic. Hence
> + * ifdef it under CONFIG_CRASH_DUMP and not CONFIG_PROC_VMCORE.
> + */
> +#ifdef CONFIG_CRASH_DUMP
>  /* elfcorehdr= specifies the location of elf core header
>   * stored by the crashed kernel.
>   */
> @@ -491,7 +496,9 @@ static int __init parse_elfcorehdr(char 
>  	return 0;
>  }
>  early_param("elfcorehdr", parse_elfcorehdr);
> +#endif
>  
> +#ifdef CONFIG_PROC_VMCORE
>  int __init reserve_elfcorehdr(unsigned long *start, unsigned long *end)
>  {
>  	unsigned long length;
> _

Hi Vivek,

I think that you also need the following in arch/ia64/kernel/crash_dump.c.
With this change your code compiles on ia64.

Signed-off-by: Simon Horman <horms@verge.net.au>


Index: linux-2.6/arch/ia64/kernel/crash_dump.c
===================================================================
--- linux-2.6.orig/arch/ia64/kernel/crash_dump.c	2008-07-29 14:06:57.000000000 +1000
+++ linux-2.6/arch/ia64/kernel/crash_dump.c	2008-07-29 14:09:55.000000000 +1000
@@ -8,10 +8,14 @@
 
 #include <linux/errno.h>
 #include <linux/types.h>
+#include <linux/crash_dump.h>
 
 #include <asm/page.h>
 #include <asm/uaccess.h>
 
+/* Stores the physical address of elf header of crash image. */
+unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
+
 /**
  * copy_oldmem_page - copy one page from "oldmem"
  * @pfn: page frame number to be copied

Re: [PATCH 3/5] ia64: Define elfcorehdr_addr in arch dependent section

[Vivek Goyal]

On Tue, Jul 29, 2008 at 02:42:43PM +1000, Simon Horman wrote:
> On Mon, Jul 28, 2008 at 05:13:14PM -0400, Vivek Goyal wrote:
> > 
> > o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
> >   equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
> >   CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
> >   used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
> >   not.
> > 
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> > 
> >  arch/ia64/kernel/setup.c |    9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff -puN arch/ia64/kernel/setup.c~fix-elfcorehdr_addr-parsing-ia64 arch/ia64/kernel/setup.c
> > --- linux-2.6.27-pre-rc1/arch/ia64/kernel/setup.c~fix-elfcorehdr_addr-parsing-ia64	2008-07-28 12:16:40.000000000 -0400
> > +++ linux-2.6.27-pre-rc1-root/arch/ia64/kernel/setup.c	2008-07-28 12:16:40.000000000 -0400
> > @@ -478,7 +478,12 @@ static __init int setup_nomca(char *s)
> >  }
> >  early_param("nomca", setup_nomca);
> >  
> > -#ifdef CONFIG_PROC_VMCORE
> > +/*
> > + * Note: elfcorehdr_addr is not just limited to vmcore. It is also used by
> > + * is_kdump_kernel() to determine if we are booting after a panic. Hence
> > + * ifdef it under CONFIG_CRASH_DUMP and not CONFIG_PROC_VMCORE.
> > + */
> > +#ifdef CONFIG_CRASH_DUMP
> >  /* elfcorehdr= specifies the location of elf core header
> >   * stored by the crashed kernel.
> >   */
> > @@ -491,7 +496,9 @@ static int __init parse_elfcorehdr(char 
> >  	return 0;
> >  }
> >  early_param("elfcorehdr", parse_elfcorehdr);
> > +#endif
> >  
> > +#ifdef CONFIG_PROC_VMCORE
> >  int __init reserve_elfcorehdr(unsigned long *start, unsigned long *end)
> >  {
> >  	unsigned long length;
> > _
> 
> Hi Vivek,
> 
> I think that you also need the following in arch/ia64/kernel/crash_dump.c.
> With this change your code compiles on ia64.
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> 

Thanks Simon. I had done these changes locally but somehow forgot to
include the changes in patches.

I will include these changes in my next posting of consolidated single
patch.

Thanks
Vivek

Re: [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section

[Ingo Molnar]

* Vivek Goyal <vgoyal@redhat.com> wrote:

> o Move elfcorehdr_addr definition in arch dependent crash dump file. This is
>   equivalent to defining elfcorehdr_addr under CONFIG_CRASH_DUMP instead of
>   CONFIG_PROC_VMCORE. This is needed by is_kdump_kernel() which can be
>   used irrespective of the fact whether CONFIG_PROC_VMCORE is enabled or
>   not.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
> 
>  arch/x86/kernel/crash_dump_32.c |    3 +++
>  arch/x86/kernel/crash_dump_64.c |    3 +++
>  arch/x86/kernel/setup.c         |    8 +++++++-
>  3 files changed, 13 insertions(+), 1 deletion(-)

the x86 bits look fine to me.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')

[Eric W. Biederman]

Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')

[Eric W. Biederman]

Vivek Goyal <vgoyal@redhat.com> writes:

> Hi All,
>
> How does following series of patches look like. I have moved
> elfcorehdr_addr out of vmcore.c and pushed it to arch dependent section 
> of crash dump to make sure that it can be worked with even when
> CONFIG_PROC_VMCORE is disabled and CONFIG_CRASH_DUMP is enabled.
>
> I tested it on x86_64. Compile tested it on i386 and ppc64. ia64 and
> sh versions are completely untested.

Given the current state of the code:
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

To process a kernel crash dump we pass the kernel elfcorehdr option, so testing
to see if it was passed seems reasonable.

In general I think this method of handling the problems with kdump is
too brittle to live, but in the case of iommus we certainly need to do something
different, and unfortunately iommus were not common on x86 when the original code
was merged so we have not handled them well.

Eric

Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')

[Simon Horman]

On Mon, Jul 28, 2008 at 03:47:41PM -0700, Eric W. Biederman wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > Hi All,
> >
> > How does following series of patches look like. I have moved
> > elfcorehdr_addr out of vmcore.c and pushed it to arch dependent section 
> > of crash dump to make sure that it can be worked with even when
> > CONFIG_PROC_VMCORE is disabled and CONFIG_CRASH_DUMP is enabled.
> >
> > I tested it on x86_64. Compile tested it on i386 and ppc64. ia64 and
> > sh versions are completely untested.
> 
> Given the current state of the code:
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> To process a kernel crash dump we pass the kernel elfcorehdr option,
> so testing to see if it was passed seems reasonable.
> 
> In general I think this method of handling the problems with kdump is
> too brittle to live, but in the case of iommus we certainly need to do
> something different, and unfortunately iommus were not common on x86
> when the original code was merged so we have not handled them well.

Agreed, however these patches look like they really ought to be merged
into a single patch for the sake of bisect. As things stand, applying
the first patch will break the build on each architecture with an
architecture specific until the latter is applied.

-- 
Horms

Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')

[Vivek Goyal]

On Tue, Jul 29, 2008 at 11:22:48AM +1000, Simon Horman wrote:
> On Mon, Jul 28, 2008 at 03:47:41PM -0700, Eric W. Biederman wrote:
> > Vivek Goyal <vgoyal@redhat.com> writes:
> > 
> > > Hi All,
> > >
> > > How does following series of patches look like. I have moved
> > > elfcorehdr_addr out of vmcore.c and pushed it to arch dependent section 
> > > of crash dump to make sure that it can be worked with even when
> > > CONFIG_PROC_VMCORE is disabled and CONFIG_CRASH_DUMP is enabled.
> > >
> > > I tested it on x86_64. Compile tested it on i386 and ppc64. ia64 and
> > > sh versions are completely untested.
> > 
> > Given the current state of the code:
> > Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> > 
> > To process a kernel crash dump we pass the kernel elfcorehdr option,
> > so testing to see if it was passed seems reasonable.
> > 
> > In general I think this method of handling the problems with kdump is
> > too brittle to live, but in the case of iommus we certainly need to do
> > something different, and unfortunately iommus were not common on x86
> > when the original code was merged so we have not handled them well.
> 
> Agreed, however these patches look like they really ought to be merged
> into a single patch for the sake of bisect. As things stand, applying
> the first patch will break the build on each architecture with an
> architecture specific until the latter is applied.

That's a good point. I was not very sure because changes were in 
different arches and I broke the patch. At the same time changes are
really miniscule in each arch. 

So, for the sake of not breaking compilation for git-bisect, I will
generate a single patch tomorrow. (Until and unless somebody has an
objection).

Thanks
Vivek

> 
> -- 
> Horms

Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')

[Simon Horman]

On Mon, Jul 28, 2008 at 10:28:22PM -0400, Vivek Goyal wrote:
> On Tue, Jul 29, 2008 at 11:22:48AM +1000, Simon Horman wrote:
> > On Mon, Jul 28, 2008 at 03:47:41PM -0700, Eric W. Biederman wrote:
> > > Vivek Goyal <vgoyal@redhat.com> writes:
> > > 
> > > > Hi All,
> > > >
> > > > How does following series of patches look like. I have moved
> > > > elfcorehdr_addr out of vmcore.c and pushed it to arch dependent section 
> > > > of crash dump to make sure that it can be worked with even when
> > > > CONFIG_PROC_VMCORE is disabled and CONFIG_CRASH_DUMP is enabled.
> > > >
> > > > I tested it on x86_64. Compile tested it on i386 and ppc64. ia64 and
> > > > sh versions are completely untested.
> > > 
> > > Given the current state of the code:
> > > Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> > > 
> > > To process a kernel crash dump we pass the kernel elfcorehdr option,
> > > so testing to see if it was passed seems reasonable.
> > > 
> > > In general I think this method of handling the problems with kdump is
> > > too brittle to live, but in the case of iommus we certainly need to do
> > > something different, and unfortunately iommus were not common on x86
> > > when the original code was merged so we have not handled them well.
> > 
> > Agreed, however these patches look like they really ought to be merged
> > into a single patch for the sake of bisect. As things stand, applying
> > the first patch will break the build on each architecture with an
> > architecture specific until the latter is applied.
> 
> That's a good point. I was not very sure because changes were in 
> different arches and I broke the patch. At the same time changes are
> really miniscule in each arch. 

I guessed that was why you split them up. But really the
per-arch change is very small.

> So, for the sake of not breaking compilation for git-bisect, I will
> generate a single patch tomorrow. (Until and unless somebody has an
> objection).

For combiled patch:

Acked-by: Simon Horman <horms@verge.net.au>

-- 
Horms