* [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')
[not found] ` <20080728034007.GA30450@verge.net.au>
@ 2008-07-28 21:10 ` Vivek Goyal
2008-07-28 21:11 ` [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section Vivek Goyal
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Vivek Goyal @ 2008-07-28 21:10 UTC (permalink / raw)
To: Simon Horman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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;
_
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section
2008-07-28 21:10 ` [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr') Vivek Goyal
@ 2008-07-28 21:11 ` Vivek Goyal
2008-07-28 21:13 ` [PATCH 3/5] ia64: " Vivek Goyal
2008-07-31 15:29 ` [PATCH 2/5] x86: " Ingo Molnar
2008-07-28 22:37 ` [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr') Eric W. Biederman
2008-07-28 22:47 ` Eric W. Biederman
2 siblings, 2 replies; 14+ messages in thread
From: Vivek Goyal @ 2008-07-28 21:11 UTC (permalink / raw)
To: Simon Horman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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
_
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/5] ia64: Define elfcorehdr_addr in arch dependent section
2008-07-28 21:11 ` [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section Vivek Goyal
@ 2008-07-28 21:13 ` Vivek Goyal
2008-07-28 21:14 ` [PATCH 4/5] powerpc: " Vivek Goyal
2008-07-29 4:42 ` [PATCH 3/5] ia64: " Simon Horman
2008-07-31 15:29 ` [PATCH 2/5] x86: " Ingo Molnar
1 sibling, 2 replies; 14+ messages in thread
From: Vivek Goyal @ 2008-07-28 21:13 UTC (permalink / raw)
To: Simon Horman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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;
_
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/5] powerpc: Define elfcorehdr_addr in arch dependent section
2008-07-28 21:13 ` [PATCH 3/5] ia64: " Vivek Goyal
@ 2008-07-28 21:14 ` Vivek Goyal
2008-07-28 21:15 ` [PATCH 5/5] sh: " Vivek Goyal
2008-07-29 4:42 ` [PATCH 3/5] ia64: " Simon Horman
1 sibling, 1 reply; 14+ messages in thread
From: Vivek Goyal @ 2008-07-28 21:14 UTC (permalink / raw)
To: Simon Horman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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)
{
_
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] sh: Define elfcorehdr_addr in arch dependent section
2008-07-28 21:14 ` [PATCH 4/5] powerpc: " Vivek Goyal
@ 2008-07-28 21:15 ` Vivek Goyal
2008-07-29 14:18 ` Paul Mundt
0 siblings, 1 reply; 14+ messages in thread
From: Vivek Goyal @ 2008-07-28 21:15 UTC (permalink / raw)
To: Simon Horman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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
_
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')
2008-07-28 21:10 ` [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr') Vivek Goyal
2008-07-28 21:11 ` [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section Vivek Goyal
@ 2008-07-28 22:37 ` Eric W. Biederman
2008-07-28 22:47 ` Eric W. Biederman
2 siblings, 0 replies; 14+ messages in thread
From: Eric W. Biederman @ 2008-07-28 22:37 UTC (permalink / raw)
To: Vivek Goyal
Cc: Tony Luck, linux-ia64, Paul Mundt, kexec, linux-kernel,
linuxppc-dev, Terry Loftin, Simon Horman, Paul Mackerras,
Eric W. Biederman, Andrew Morton, Linus Torvalds, Ingo Molnar
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')
2008-07-28 21:10 ` [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr') Vivek Goyal
2008-07-28 21:11 ` [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section Vivek Goyal
2008-07-28 22:37 ` [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr') Eric W. Biederman
@ 2008-07-28 22:47 ` Eric W. Biederman
2008-07-29 1:22 ` Simon Horman
2 siblings, 1 reply; 14+ messages in thread
From: Eric W. Biederman @ 2008-07-28 22:47 UTC (permalink / raw)
To: Vivek Goyal
Cc: Tony Luck, linux-ia64, Paul Mundt, kexec, linux-kernel,
linuxppc-dev, Terry Loftin, Simon Horman, Paul Mackerras,
Andrew Morton, Linus Torvalds, Ingo Molnar
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')
2008-07-28 22:47 ` Eric W. Biederman
@ 2008-07-29 1:22 ` Simon Horman
2008-07-29 2:28 ` Vivek Goyal
0 siblings, 1 reply; 14+ messages in thread
From: Simon Horman @ 2008-07-29 1:22 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Andrew Morton,
Linus Torvalds, Ingo Molnar, Vivek Goyal
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')
2008-07-29 1:22 ` Simon Horman
@ 2008-07-29 2:28 ` Vivek Goyal
2008-07-29 3:26 ` Simon Horman
0 siblings, 1 reply; 14+ messages in thread
From: Vivek Goyal @ 2008-07-29 2:28 UTC (permalink / raw)
To: Simon Horman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr')
2008-07-29 2:28 ` Vivek Goyal
@ 2008-07-29 3:26 ` Simon Horman
0 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2008-07-29 3:26 UTC (permalink / raw)
To: Vivek Goyal
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] ia64: Define elfcorehdr_addr in arch dependent section
2008-07-28 21:13 ` [PATCH 3/5] ia64: " Vivek Goyal
2008-07-28 21:14 ` [PATCH 4/5] powerpc: " Vivek Goyal
@ 2008-07-29 4:42 ` Simon Horman
2008-07-29 13:53 ` Vivek Goyal
1 sibling, 1 reply; 14+ messages in thread
From: Simon Horman @ 2008-07-29 4:42 UTC (permalink / raw)
To: Vivek Goyal
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/5] ia64: Define elfcorehdr_addr in arch dependent section
2008-07-29 4:42 ` [PATCH 3/5] ia64: " Simon Horman
@ 2008-07-29 13:53 ` Vivek Goyal
0 siblings, 0 replies; 14+ messages in thread
From: Vivek Goyal @ 2008-07-29 13:53 UTC (permalink / raw)
To: Simon Horman
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Paul Mundt, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] sh: Define elfcorehdr_addr in arch dependent section
2008-07-28 21:15 ` [PATCH 5/5] sh: " Vivek Goyal
@ 2008-07-29 14:18 ` Paul Mundt
0 siblings, 0 replies; 14+ messages in thread
From: Paul Mundt @ 2008-07-29 14:18 UTC (permalink / raw)
To: Vivek Goyal
Cc: Tony Luck, linux-ia64, kexec, linux-kernel, linuxppc-dev,
Terry Loftin, Simon Horman, Paul Mackerras, Eric W. Biederman,
Andrew Morton, Linus Torvalds, Ingo Molnar
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>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section
2008-07-28 21:11 ` [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section Vivek Goyal
2008-07-28 21:13 ` [PATCH 3/5] ia64: " Vivek Goyal
@ 2008-07-31 15:29 ` Ingo Molnar
1 sibling, 0 replies; 14+ messages in thread
From: Ingo Molnar @ 2008-07-31 15:29 UTC (permalink / raw)
To: Vivek Goyal
Cc: Tony Luck, linux-ia64, Paul Mundt, kexec, linux-kernel,
linuxppc-dev, Terry Loftin, Simon Horman, Paul Mackerras,
Eric W. Biederman, Andrew Morton, Linus Torvalds
* 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
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2008-07-31 15:30 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20080727234529.GM6175@verge.net.au>
[not found] ` <20080728015117.GA12055@verge.net.au>
[not found] ` <20080728024526.GB3334@verge.net.au>
[not found] ` <20080728034007.GA30450@verge.net.au>
2008-07-28 21:10 ` [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr') Vivek Goyal
2008-07-28 21:11 ` [PATCH 2/5] x86: Define elfcorehdr_addr in arch dependent section Vivek Goyal
2008-07-28 21:13 ` [PATCH 3/5] ia64: " Vivek Goyal
2008-07-28 21:14 ` [PATCH 4/5] powerpc: " Vivek Goyal
2008-07-28 21:15 ` [PATCH 5/5] sh: " Vivek Goyal
2008-07-29 14:18 ` Paul Mundt
2008-07-29 4:42 ` [PATCH 3/5] ia64: " Simon Horman
2008-07-29 13:53 ` Vivek Goyal
2008-07-31 15:29 ` [PATCH 2/5] x86: " Ingo Molnar
2008-07-28 22:37 ` [PATCH 1/5] Move elfcorehdr_addr out of vmcore.c (Was: Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr') Eric W. Biederman
2008-07-28 22:47 ` Eric W. Biederman
2008-07-29 1:22 ` Simon Horman
2008-07-29 2:28 ` Vivek Goyal
2008-07-29 3:26 ` Simon Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).