[patch] crashdump: fix undefined reference to `elfcorehdr_addr' - Kernel

This is a discussion on [patch] crashdump: fix undefined reference to `elfcorehdr_addr' - Kernel ; please apply before -rc1. Ingo -----------> From 72db7cba50b6a05825f8a287f74002cc38f04fb7 Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Sat, 26 Jul 2008 11:22:33 +0200 Subject: [PATCH] crashdump: fix undefined reference to `elfcorehdr_addr' fix build bug introduced by 95b68dec0d5 "calgary iommu: use ...

+ Reply to Thread
Page 1 of 2 1 2 LastLast
Results 1 to 20 of 28

Thread: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

  1. [patch] crashdump: fix undefined reference to `elfcorehdr_addr'


    please apply before -rc1.

    Ingo

    ----------->
    From 72db7cba50b6a05825f8a287f74002cc38f04fb7 Mon Sep 17 00:00:00 2001
    From: Ingo Molnar
    Date: Sat, 26 Jul 2008 11:22:33 +0200
    Subject: [PATCH] crashdump: fix undefined reference to `elfcorehdr_addr'

    fix build bug introduced by 95b68dec0d5 "calgary iommu: use the first
    kernels TCE tables in kdump":

    arch/x86/kernel/built-in.o: In function `calgary_iommu_init':
    (.init.text+0x8399): undefined reference to `elfcorehdr_addr'
    arch/x86/kernel/built-in.o: In function `calgary_iommu_init':
    (.init.text+0x856c): undefined reference to `elfcorehdr_addr'
    arch/x86/kernel/built-in.o: In function `detect_calgary':
    (.init.text+0x8c68): undefined reference to `elfcorehdr_addr'
    arch/x86/kernel/built-in.o: In function `detect_calgary':
    (.init.text+0x8d0c): undefined reference to `elfcorehdr_addr'

    make elfcorehdr_addr a generally available symbol.

    Signed-off-by: Ingo Molnar
    ---
    include/linux/crash_dump.h | 6 ++++++
    1 files changed, 6 insertions(+), 0 deletions(-)

    diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
    index 6cd39a9..025e4f5 100644
    --- a/include/linux/crash_dump.h
    +++ b/include/linux/crash_dump.h
    @@ -8,7 +8,13 @@
    #include

    #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);
    extern const struct file_operations proc_vmcore_operations;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    On Sat, 26 Jul 2008 11:25:19 +0200 Ingo Molnar wrote:

    >
    > please apply before -rc1.
    >
    > Ingo
    >
    > ----------->
    > >From 72db7cba50b6a05825f8a287f74002cc38f04fb7 Mon Sep 17 00:00:00 2001

    > From: Ingo Molnar
    > Date: Sat, 26 Jul 2008 11:22:33 +0200
    > Subject: [PATCH] crashdump: fix undefined reference to `elfcorehdr_addr'
    >
    > fix build bug introduced by 95b68dec0d5 "calgary iommu: use the first
    > kernels TCE tables in kdump":
    >
    > arch/x86/kernel/built-in.o: In function `calgary_iommu_init':
    > (.init.text+0x8399): undefined reference to `elfcorehdr_addr'
    > arch/x86/kernel/built-in.o: In function `calgary_iommu_init':
    > (.init.text+0x856c): undefined reference to `elfcorehdr_addr'
    > arch/x86/kernel/built-in.o: In function `detect_calgary':
    > (.init.text+0x8c68): undefined reference to `elfcorehdr_addr'
    > arch/x86/kernel/built-in.o: In function `detect_calgary':
    > (.init.text+0x8d0c): undefined reference to `elfcorehdr_addr'
    >
    > make elfcorehdr_addr a generally available symbol.
    >
    > Signed-off-by: Ingo Molnar
    > ---
    > include/linux/crash_dump.h | 6 ++++++
    > 1 files changed, 6 insertions(+), 0 deletions(-)
    >
    > diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
    > index 6cd39a9..025e4f5 100644
    > --- a/include/linux/crash_dump.h
    > +++ b/include/linux/crash_dump.h
    > @@ -8,7 +8,13 @@
    > #include
    >
    > #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);
    > extern const struct file_operations proc_vmcore_operations;


    spose that'll fix it. But it seems odd that is_kdump_kernel() will
    return false if CONFIG_PROC_VMCORE=n, CONFIG_CRASH_DUMP=y. I mean,
    it's still a crashdump kernel, is it not?


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    On Sat, Jul 26, 2008 at 03:10:34AM -0700, Andrew Morton wrote:
    > On Sat, 26 Jul 2008 11:25:19 +0200 Ingo Molnar wrote:
    >
    > >
    > > please apply before -rc1.
    > >
    > > Ingo
    > >
    > > ----------->
    > > >From 72db7cba50b6a05825f8a287f74002cc38f04fb7 Mon Sep 17 00:00:00 2001

    > > From: Ingo Molnar
    > > Date: Sat, 26 Jul 2008 11:22:33 +0200
    > > Subject: [PATCH] crashdump: fix undefined reference to `elfcorehdr_addr'
    > >
    > > fix build bug introduced by 95b68dec0d5 "calgary iommu: use the first
    > > kernels TCE tables in kdump":
    > >
    > > arch/x86/kernel/built-in.o: In function `calgary_iommu_init':
    > > (.init.text+0x8399): undefined reference to `elfcorehdr_addr'
    > > arch/x86/kernel/built-in.o: In function `calgary_iommu_init':
    > > (.init.text+0x856c): undefined reference to `elfcorehdr_addr'
    > > arch/x86/kernel/built-in.o: In function `detect_calgary':
    > > (.init.text+0x8c68): undefined reference to `elfcorehdr_addr'
    > > arch/x86/kernel/built-in.o: In function `detect_calgary':
    > > (.init.text+0x8d0c): undefined reference to `elfcorehdr_addr'
    > >
    > > make elfcorehdr_addr a generally available symbol.
    > >
    > > Signed-off-by: Ingo Molnar
    > > ---
    > > include/linux/crash_dump.h | 6 ++++++
    > > 1 files changed, 6 insertions(+), 0 deletions(-)
    > >
    > > diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
    > > index 6cd39a9..025e4f5 100644
    > > --- a/include/linux/crash_dump.h
    > > +++ b/include/linux/crash_dump.h
    > > @@ -8,7 +8,13 @@
    > > #include
    > >
    > > #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);
    > > extern const struct file_operations proc_vmcore_operations;

    >
    > spose that'll fix it. But it seems odd that is_kdump_kernel() will
    > return false if CONFIG_PROC_VMCORE=n, CONFIG_CRASH_DUMP=y. I mean,
    > it's still a crashdump kernel, is it not?


    Perhaps is_kdump_kernel() ought to be renamed kernel_has_vmcore().

    To my mind, is_kdump_kernel() should really look something like this:

    #ifdef CONFIG_CRASH_DUMP
    static inline int is_kdump_kernel(void) { return 1; }
    #else
    static inline int is_kdump_kernel(void) { return 0; }
    #endif

    But that can probably just be handled by any relevant code
    using CONFIG_CRASH_DUMP as necessary.



    --
    Horms

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    [ Updated Vivek's email address to his vgoyal@redhat.com in CC list
    Added Terry Loftin, Tony Luck, Erik Biedermann and linux-ia64 to CC list ]

    On Mon, Jul 28, 2008 at 09:45:31AM +1000, Simon Horman wrote:
    > > > diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
    > > > index 6cd39a9..025e4f5 100644
    > > > --- a/include/linux/crash_dump.h
    > > > +++ b/include/linux/crash_dump.h
    > > > @@ -8,7 +8,13 @@
    > > > #include
    > > >
    > > > #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);
    > > > extern const struct file_operations proc_vmcore_operations;

    > >
    > > spose that'll fix it. But it seems odd that is_kdump_kernel() will
    > > return false if CONFIG_PROC_VMCORE=n, CONFIG_CRASH_DUMP=y. I mean,
    > > it's still a crashdump kernel, is it not?

    >
    > Perhaps is_kdump_kernel() ought to be renamed kernel_has_vmcore().
    >
    > To my mind, is_kdump_kernel() should really look something like this:
    >
    > #ifdef CONFIG_CRASH_DUMP
    > static inline int is_kdump_kernel(void) { return 1; }
    > #else
    > static inline int is_kdump_kernel(void) { return 0; }
    > #endif
    >
    > But that can probably just be handled by any relevant code
    > using CONFIG_CRASH_DUMP as necessary.


    Hi,

    I started looking into a simple fix to change the name of
    the is_kdump_kernel() to kernel_has_vmcore(), which is what
    the code in its current incarnatation does.

    This also lead to cleaning the usage of elfcorehdr_addr,
    which is in the folloing messy state after recent changes.

    #ifdef CONFIG_PROC_VMCORE
    * Declared non-static include/linux/crash_dump.h
    * Initialised in fs/proc/vmcore.c
    #else
    * Declared and initialised as static in include/linux/crash_dump.h
    * Only used by is_kdump_kernel() which is a static function
    also in include/linux/crash_dump.h
    #endif


    Howerver, in the course of doing this I came to thinking that actually
    this code won't solve the problem at hand in the case where
    CONFIG_CRASH_DUMP is defined but CONFIG_PROC_VMCORE is not.
    Or in other words, what happens if the calgary initialisation code
    runs in a kdump kernel that does not have CONFIG_PROC_VMCORE ?

    A similar problem appears to exist in
    arch/ia64/hp/common/sba_iommu.c:sba_init(), which currently doesn't
    compile if CONFIG_CRASH_DUMP is set but CONFIG_PROC_VMCORE is not. The
    compilation issue could be solved by using kernel_has_vmcore() (as per
    the patch below) instead of checking elfcorehdr_addr directly, but does
    it actually lead to working code?

    There has long been a strong aversion to providing the second
    kernel with flags like im_in_kexec or im_in_kdump, as its felt
    that this kind of problem is better handled by making sure that the
    hardware is in a sensible state before leaving the first-kernel.
    But this is arguably more reasonable in the kexec case than the
    kdump case.


    If there really is a need for kdump kernels to know that they are
    booting a kdumping system, then I propose one of the following:

    1) Always parse the elfcorehdr kernel command line option
    and set elfcorehdr_addr accordingly - currently this is only
    done if CONFIG_PROC_VMCORE is set.

    This is nice as it won't need any modifications to kexec-tools
    nor any command line bloat.

    A minor difficulty is working out where to initialise elfcorehdr_addr.
    Sometimes in include/linux/crash_dump.h and sometimes in
    fs/proc/vmcore.c seems horrible to me.

    Another problem is that would be alive and well in
    code that really only uses it to check if kdump was activated or not
    - a minor naming issue.

    2) Add a new kernel command line option, perhaps in_kdump

    This is bloat to get around elfcorehdr_addr initialisation and
    naming awkwardness above.

    3) Make select CONFIG_PROC_VMCORE when CONFIG_CRASH_DUMP is selected,
    or perhaps even just remove CONFIG_PROC_VMCORE and only use
    CONFIG_CRASH_DUMP instead. The effect would be the same either way.

    Pro: One less thing to be confused about

    Con: Bloat for people who want kdump without vmcore.
    I wonder what usage case that is.

    --
    Horms

    Index: linux-2.6/arch/x86/kernel/pci-calgary_64.c
    ================================================== =================
    --- linux-2.6.orig/arch/x86/kernel/pci-calgary_64.c 2008-07-28 10:10:31.000000000 +1000
    +++ linux-2.6/arch/x86/kernel/pci-calgary_64.c 2008-07-28 10:14:24.000000000 +1000
    @@ -801,7 +801,7 @@ static int __init calgary_setup_tar(stru
    tbl = pci_iommu(dev->bus);
    tbl->it_base = (unsigned long)bus_info[dev->bus->number].tce_space;

    - if (is_kdump_kernel())
    + if (kernel_has_vmcore())
    calgary_init_bitmap_from_tce_table(tbl);
    else
    tce_free(tbl, 0, tbl->it_size);
    @@ -1184,7 +1184,7 @@ static int __init calgary_init(void)
    return ret;

    /* Purely for kdump kernel case */
    - if (is_kdump_kernel())
    + if (kernel_has_vmcore())
    get_tce_space_from_tar();

    do {
    @@ -1438,7 +1438,7 @@ void __init detect_calgary(void)
    return;
    }

    - specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
    + specified_table_size = determine_tce_table_size((kernel_has_vmcore() ?
    saved_max_pfn : max_pfn) * PAGE_SIZE);

    for (bus = 0; bus < MAX_PHB_BUS_NUM; bus++) {
    @@ -1461,7 +1461,7 @@ void __init detect_calgary(void)
    * If it is kdump kernel, find and use tce tables
    * from first kernel, else allocate tce tables here
    */
    - if (!is_kdump_kernel()) {
    + if (!kernel_has_vmcore()) {
    tbl = alloc_tce_table();
    if (!tbl)
    goto cleanup;
    Index: linux-2.6/fs/proc/vmcore.c
    ================================================== =================
    --- linux-2.6.orig/fs/proc/vmcore.c 2008-07-28 09:51:30.000000000 +1000
    +++ linux-2.6/fs/proc/vmcore.c 2008-07-28 09:51:53.000000000 +1000
    @@ -32,8 +32,7 @@ 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;
    +unsigned long long elfcorehdr_addr;

    struct proc_dir_entry *proc_vmcore = NULL;

    Index: linux-2.6/include/linux/crash_dump.h
    ================================================== =================
    --- linux-2.6.orig/include/linux/crash_dump.h 2008-07-28 09:46:29.000000000 +1000
    +++ linux-2.6/include/linux/crash_dump.h 2008-07-28 10:43:03.000000000 +1000
    @@ -11,8 +11,13 @@

    #ifdef CONFIG_PROC_VMCORE
    extern unsigned long long elfcorehdr_addr;
    +
    +static inline int kernel_has_vmcore(void)
    +{
    + return (elfcorehdr_addr != ELFCORE_ADDR_MAX) ? 1 : 0;
    +}
    #else
    -static const unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
    +static inline int kernel_has_vmcore(void) { return 0; }
    #endif

    extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
    @@ -28,12 +33,6 @@ extern struct proc_dir_entry *proc_vmcor

    #define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))

    -static inline int is_kdump_kernel(void)
    -{
    - return (elfcorehdr_addr != ELFCORE_ADDR_MAX) ? 1 : 0;
    -}
    -#else /* !CONFIG_CRASH_DUMP */
    -static inline int is_kdump_kernel(void) { return 0; }
    #endif /* CONFIG_CRASH_DUMP */

    extern unsigned long saved_max_pfn;

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    The rfc patch I made to rename is_kdump_kernel to kernel_has_vmcore,
    which I appended to my previous post should have looked more like the
    following. Although, as I noted in my previous post, its more a starting
    point for discussion than a solution to the problem at hand.

    Index: linux-2.6/arch/x86/kernel/pci-calgary_64.c
    ================================================== =================
    --- linux-2.6.orig/arch/x86/kernel/pci-calgary_64.c 2008-07-28 10:10:31.000000000 +1000
    +++ linux-2.6/arch/x86/kernel/pci-calgary_64.c 2008-07-28 10:14:24.000000000 +1000
    @@ -801,7 +801,7 @@ static int __init calgary_setup_tar(stru
    tbl = pci_iommu(dev->bus);
    tbl->it_base = (unsigned long)bus_info[dev->bus->number].tce_space;

    - if (is_kdump_kernel())
    + if (kernel_has_vmcore())
    calgary_init_bitmap_from_tce_table(tbl);
    else
    tce_free(tbl, 0, tbl->it_size);
    @@ -1184,7 +1184,7 @@ static int __init calgary_init(void)
    return ret;

    /* Purely for kdump kernel case */
    - if (is_kdump_kernel())
    + if (kernel_has_vmcore())
    get_tce_space_from_tar();

    do {
    @@ -1438,7 +1438,7 @@ void __init detect_calgary(void)
    return;
    }

    - specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
    + specified_table_size = determine_tce_table_size((kernel_has_vmcore() ?
    saved_max_pfn : max_pfn) * PAGE_SIZE);

    for (bus = 0; bus < MAX_PHB_BUS_NUM; bus++) {
    @@ -1461,7 +1461,7 @@ void __init detect_calgary(void)
    * If it is kdump kernel, find and use tce tables
    * from first kernel, else allocate tce tables here
    */
    - if (!is_kdump_kernel()) {
    + if (!kernel_has_vmcore()) {
    tbl = alloc_tce_table();
    if (!tbl)
    goto cleanup;
    Index: linux-2.6/fs/proc/vmcore.c
    ================================================== =================
    --- linux-2.6.orig/fs/proc/vmcore.c 2008-07-28 09:51:30.000000000 +1000
    +++ linux-2.6/fs/proc/vmcore.c 2008-07-28 09:51:53.000000000 +1000
    @@ -32,8 +32,7 @@ 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;
    +unsigned long long elfcorehdr_addr;

    struct proc_dir_entry *proc_vmcore = NULL;

    Index: linux-2.6/include/linux/crash_dump.h
    ================================================== =================
    --- linux-2.6.orig/include/linux/crash_dump.h 2008-07-28 09:46:29.000000000 +1000
    +++ linux-2.6/include/linux/crash_dump.h 2008-07-28 10:43:03.000000000 +1000
    @@ -11,8 +11,13 @@

    #ifdef CONFIG_PROC_VMCORE
    extern unsigned long long elfcorehdr_addr;
    +
    +static inline int kernel_has_vmcore(void)
    +{
    + return (elfcorehdr_addr != ELFCORE_ADDR_MAX) ? 1 : 0;
    +}
    #else
    -static const unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
    +static inline int kernel_has_vmcore(void) { return 0; }
    #endif

    extern ssize_t copy_oldmem_page(unsigned long, char *, size_t,
    @@ -28,12 +33,6 @@ extern struct proc_dir_entry *proc_vmcor

    #define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))

    -static inline int is_kdump_kernel(void)
    -{
    - return (elfcorehdr_addr != ELFCORE_ADDR_MAX) ? 1 : 0;
    -}
    -#else /* !CONFIG_CRASH_DUMP */
    -static inline int is_kdump_kernel(void) { return 0; }
    #endif /* CONFIG_CRASH_DUMP */

    extern unsigned long saved_max_pfn;
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    On Mon, Jul 28, 2008 at 12:45:28PM +1000, Simon Horman wrote:
    > The rfc patch I made to rename is_kdump_kernel to kernel_has_vmcore,
    > which I appended to my previous post should have looked more like the
    > following. Although, as I noted in my previous post, its more a starting
    > point for discussion than a solution to the problem at hand.


    Sorry, one more time. I forgot to quilt refresh.

    Index: linux-2.6/arch/x86/kernel/pci-calgary_64.c
    ================================================== =================
    --- linux-2.6.orig/arch/x86/kernel/pci-calgary_64.c 2008-07-28 10:10:31.000000000 +1000
    +++ linux-2.6/arch/x86/kernel/pci-calgary_64.c 2008-07-28 10:14:24.000000000 +1000
    @@ -801,7 +801,7 @@ static int __init calgary_setup_tar(stru
    tbl = pci_iommu(dev->bus);
    tbl->it_base = (unsigned long)bus_info[dev->bus->number].tce_space;

    - if (is_kdump_kernel())
    + if (kernel_has_vmcore())
    calgary_init_bitmap_from_tce_table(tbl);
    else
    tce_free(tbl, 0, tbl->it_size);
    @@ -1184,7 +1184,7 @@ static int __init calgary_init(void)
    return ret;

    /* Purely for kdump kernel case */
    - if (is_kdump_kernel())
    + if (kernel_has_vmcore())
    get_tce_space_from_tar();

    do {
    @@ -1438,7 +1438,7 @@ void __init detect_calgary(void)
    return;
    }

    - specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
    + specified_table_size = determine_tce_table_size((kernel_has_vmcore() ?
    saved_max_pfn : max_pfn) * PAGE_SIZE);

    for (bus = 0; bus < MAX_PHB_BUS_NUM; bus++) {
    @@ -1461,7 +1461,7 @@ void __init detect_calgary(void)
    * If it is kdump kernel, find and use tce tables
    * from first kernel, else allocate tce tables here
    */
    - if (!is_kdump_kernel()) {
    + if (!kernel_has_vmcore()) {
    tbl = alloc_tce_table();
    if (!tbl)
    goto cleanup;
    Index: linux-2.6/fs/proc/vmcore.c
    ================================================== =================
    --- linux-2.6.orig/fs/proc/vmcore.c 2008-07-28 09:51:30.000000000 +1000
    +++ linux-2.6/fs/proc/vmcore.c 2008-07-28 09:51:53.000000000 +1000
    @@ -32,8 +32,7 @@ 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;
    +unsigned long long elfcorehdr_addr;

    struct proc_dir_entry *proc_vmcore = NULL;

    Index: linux-2.6/include/linux/crash_dump.h
    ================================================== =================
    --- linux-2.6.orig/include/linux/crash_dump.h 2008-07-28 09:46:29.000000000 +1000
    +++ linux-2.6/include/linux/crash_dump.h 2008-07-28 12:11:57.000000000 +1000
    @@ -9,12 +9,6 @@

    #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);
    extern const struct file_operations proc_vmcore_operations;
    @@ -28,13 +22,18 @@ extern struct proc_dir_entry *proc_vmcor

    #define vmcore_elf_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))

    -static inline int is_kdump_kernel(void)
    -{
    - return (elfcorehdr_addr != ELFCORE_ADDR_MAX) ? 1 : 0;
    -}
    -#else /* !CONFIG_CRASH_DUMP */
    -static inline int is_kdump_kernel(void) { return 0; }
    #endif /* CONFIG_CRASH_DUMP */

    extern unsigned long saved_max_pfn;
    #endif /* LINUX_CRASHDUMP_H */
    +
    +#ifdef CONFIG_PROC_VMCORE
    +extern unsigned long long elfcorehdr_addr;
    +
    +static inline int kernel_has_vmcore(void)
    +{
    + return (elfcorehdr_addr != ELFCORE_ADDR_MAX) ? 1 : 0;
    +}
    +#else
    +static inline int kernel_has_vmcore(void) { return 0; }
    +#endif
    Index: linux-2.6/arch/ia64/hp/common/sba_iommu.c
    ================================================== =================
    --- linux-2.6.orig/arch/ia64/hp/common/sba_iommu.c 2008-07-28 11:51:36.000000000 +1000
    +++ linux-2.6/arch/ia64/hp/common/sba_iommu.c 2008-07-28 12:11:27.000000000 +1000
    @@ -2070,14 +2070,13 @@ sba_init(void)
    if (!ia64_platform_is("hpzx1") && !ia64_platform_is("hpzx1_swiotlb"))
    return 0;

    -#if defined(CONFIG_IA64_GENERIC) && defined(CONFIG_PROC_VMCORE) && \
    - defined(CONFIG_PROC_FS)
    +#if defined(CONFIG_IA64_GENERIC)
    /* If we are booting a kdump kernel, the sba_iommu will
    * cause devices that were not shutdown properly to MCA
    * as soon as they are turned back on. Our only option for
    * a successful kdump kernel boot is to use the swiotlb.
    */
    - if (elfcorehdr_addr < ELFCORE_ADDR_MAX) {
    + if (kernel_has_vmcore()) {
    if (swiotlb_late_init_with_default_size(64 * (1<<20)) != 0)
    panic("Unable to initialize software I/O TLB:"
    " Try machvec=dig boot option");
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    Simon Horman writes:

    >
    > Hi,
    >
    > I started looking into a simple fix to change the name of
    > the is_kdump_kernel() to kernel_has_vmcore(), which is what
    > the code in its current incarnatation does.
    >
    > This also lead to cleaning the usage of elfcorehdr_addr,
    > which is in the folloing messy state after recent changes.
    >
    > #ifdef CONFIG_PROC_VMCORE
    > * Declared non-static include/linux/crash_dump.h
    > * Initialised in fs/proc/vmcore.c
    > #else
    > * Declared and initialised as static in include/linux/crash_dump.h
    > * Only used by is_kdump_kernel() which is a static function
    > also in include/linux/crash_dump.h
    > #endif
    >
    >
    > Howerver, in the course of doing this I came to thinking that actually
    > this code won't solve the problem at hand in the case where
    > CONFIG_CRASH_DUMP is defined but CONFIG_PROC_VMCORE is not.
    > Or in other words, what happens if the calgary initialisation code
    > runs in a kdump kernel that does not have CONFIG_PROC_VMCORE ?
    >
    > A similar problem appears to exist in
    > arch/ia64/hp/common/sba_iommu.c:sba_init(), which currently doesn't
    > compile if CONFIG_CRASH_DUMP is set but CONFIG_PROC_VMCORE is not. The
    > compilation issue could be solved by using kernel_has_vmcore() (as per
    > the patch below) instead of checking elfcorehdr_addr directly, but does
    > it actually lead to working code?
    >
    > There has long been a strong aversion to providing the second
    > kernel with flags like im_in_kexec or im_in_kdump, as its felt
    > that this kind of problem is better handled by making sure that the
    > hardware is in a sensible state before leaving the first-kernel.
    > But this is arguably more reasonable in the kexec case than the
    > kdump case.


    That and because we can generally solve the specific problem with
    a general feature. Something we can enable/disable on the
    command line if needed. Right now this is especially interesting
    as on several architectures distros are not building special kdump
    kernels but have a single kernel binary that works in both cases.

    Skimming through your patches this is a case we really do need to
    implement and handle cleanly.

    Currently we leave DMA running in the kexec on panic case. We avoid
    problems by only running out of a reserved area of memory.

    As as general strategy that is fine. However we have not implemented that
    strategy in the case of IOMMUs. And we are having trouble with IOMMUs.

    My hunch is that we should implement options to reserve a section of
    the iommu and to tell to the iommu to use the previously reserved section.
    Although turning iommus off altogether and simply using swiotlb
    may be acceptable. In which case we should just force usage of the swiotlb
    on the command line in /sbin/kexec.

    Eric
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    On Sun, Jul 27, 2008 at 10:39:30PM -0700, Eric W. Biederman wrote:
    > Simon Horman writes:
    >
    > > There has long been a strong aversion to providing the second
    > > kernel with flags like im_in_kexec or im_in_kdump, as its felt
    > > that this kind of problem is better handled by making sure that
    > > the hardware is in a sensible state before leaving the
    > > first-kernel. But this is arguably more reasonable in the kexec
    > > case than the kdump case.

    >
    > That and because we can generally solve the specific problem with a
    > general feature. Something we can enable/disable on the command
    > line if needed. Right now this is especially interesting as on
    > several architectures distros are not building special kdump kernels
    > but have a single kernel binary that works in both cases.
    >
    > Skimming through your patches this is a case we really do need to
    > implement and handle cleanly.
    >
    > Currently we leave DMA running in the kexec on panic case. We avoid
    > problems by only running out of a reserved area of memory.
    >
    > As as general strategy that is fine. However we have not
    > implemented that strategy in the case of IOMMUs. And we are having
    > trouble with IOMMUs.
    >
    > My hunch is that we should implement options to reserve a section of
    > the iommu and to tell to the iommu to use the previously reserved
    > section. Although turning iommus off altogether and simply using
    > swiotlb may be acceptable. In which case we should just force usage
    > of the swiotlb on the command line in /sbin/kexec.


    With an isolation-capable IOMMU (such as Calgary, VT-d and AMD's
    IOMMU) on the I/O path, as long as we want to keep DMAs running and
    going through to memory, we need to keep the IOMMU running, with the
    same set of permissions and translation tables. If we don't mind DMAs
    failing, we need to gracefully handle IOMMU DMA faults (where
    possible---Calgary can't do it at the moment). What we do instead with
    Calgary (c.f., the patch that instigated this discussion) is a hack,
    we "reinitialize" the IOMMU with the old IOMMU's translation tables so
    that DMAs continue working.

    My preference would be to have stopped all DMAs in the old kernel,
    which would've made this nastiness go away. Can you explain in simple
    words why we can't or won't do that?

    Cheers,
    Muli
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'


    * Simon Horman wrote:

    > On Mon, Jul 28, 2008 at 12:45:28PM +1000, Simon Horman wrote:
    > > The rfc patch I made to rename is_kdump_kernel to kernel_has_vmcore,
    > > which I appended to my previous post should have looked more like the
    > > following. Although, as I noted in my previous post, its more a starting
    > > point for discussion than a solution to the problem at hand.

    >
    > Sorry, one more time. I forgot to quilt refresh.


    doesnt apply cleanly to latest -git:

    Hunk #1 FAILED at 2070.
    1 out of 1 hunk FAILED -- saving rejects to file arch/ia64/hp/common/sba_iommu.c.rej

    due to a crossing change i guess. Also, i guess this should go via -mm
    as it touches fs/proc/vmcore.c and include/linux/crash_dump.h. The x86
    bits look good to me.

    Acked-by: Ingo Molnar

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    On Mon, Jul 28, 2008 at 11:51:19AM +1000, Simon Horman wrote:
    > [ Updated Vivek's email address to his vgoyal@redhat.com in CC list
    > Added Terry Loftin, Tony Luck, Erik Biedermann and linux-ia64 to CC list ]
    >
    > On Mon, Jul 28, 2008 at 09:45:31AM +1000, Simon Horman wrote:
    > > > > diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
    > > > > index 6cd39a9..025e4f5 100644
    > > > > --- a/include/linux/crash_dump.h
    > > > > +++ b/include/linux/crash_dump.h
    > > > > @@ -8,7 +8,13 @@
    > > > > #include
    > > > >
    > > > > #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);
    > > > > extern const struct file_operations proc_vmcore_operations;
    > > >
    > > > spose that'll fix it. But it seems odd that is_kdump_kernel() will
    > > > return false if CONFIG_PROC_VMCORE=n, CONFIG_CRASH_DUMP=y. I mean,
    > > > it's still a crashdump kernel, is it not?

    > >
    > > Perhaps is_kdump_kernel() ought to be renamed kernel_has_vmcore().
    > >
    > > To my mind, is_kdump_kernel() should really look something like this:
    > >
    > > #ifdef CONFIG_CRASH_DUMP
    > > static inline int is_kdump_kernel(void) { return 1; }
    > > #else
    > > static inline int is_kdump_kernel(void) { return 0; }
    > > #endif
    > >
    > > But that can probably just be handled by any relevant code
    > > using CONFIG_CRASH_DUMP as necessary.

    >
    > Hi,
    >
    > I started looking into a simple fix to change the name of
    > the is_kdump_kernel() to kernel_has_vmcore(), which is what
    > the code in its current incarnatation does.
    >
    > This also lead to cleaning the usage of elfcorehdr_addr,
    > which is in the folloing messy state after recent changes.
    >
    > #ifdef CONFIG_PROC_VMCORE
    > * Declared non-static include/linux/crash_dump.h
    > * Initialised in fs/proc/vmcore.c
    > #else
    > * Declared and initialised as static in include/linux/crash_dump.h
    > * Only used by is_kdump_kernel() which is a static function
    > also in include/linux/crash_dump.h
    > #endif
    >
    >
    > Howerver, in the course of doing this I came to thinking that actually
    > this code won't solve the problem at hand in the case where
    > CONFIG_CRASH_DUMP is defined but CONFIG_PROC_VMCORE is not.
    > Or in other words, what happens if the calgary initialisation code
    > runs in a kdump kernel that does not have CONFIG_PROC_VMCORE ?
    >
    > A similar problem appears to exist in
    > arch/ia64/hp/common/sba_iommu.c:sba_init(), which currently doesn't
    > compile if CONFIG_CRASH_DUMP is set but CONFIG_PROC_VMCORE is not. The
    > compilation issue could be solved by using kernel_has_vmcore() (as per
    > the patch below) instead of checking elfcorehdr_addr directly, but does
    > it actually lead to working code?
    >
    > There has long been a strong aversion to providing the second
    > kernel with flags like im_in_kexec or im_in_kdump, as its felt
    > that this kind of problem is better handled by making sure that the
    > hardware is in a sensible state before leaving the first-kernel.
    > But this is arguably more reasonable in the kexec case than the
    > kdump case.
    >
    >
    > If there really is a need for kdump kernels to know that they are
    > booting a kdumping system, then I propose one of the following:
    >
    > 1) Always parse the elfcorehdr kernel command line option
    > and set elfcorehdr_addr accordingly - currently this is only
    > done if CONFIG_PROC_VMCORE is set.
    >
    > This is nice as it won't need any modifications to kexec-tools
    > nor any command line bloat.
    >
    > A minor difficulty is working out where to initialise elfcorehdr_addr.
    > Sometimes in include/linux/crash_dump.h and sometimes in
    > fs/proc/vmcore.c seems horrible to me.
    >
    > Another problem is that would be alive and well in
    > code that really only uses it to check if kdump was activated or not
    > - a minor naming issue.
    >


    Hi Simon,

    There are some kernel bits (like iommu initialization patch), which need to
    take special action if they are booting after a kexec on panic (Generally we
    are referring it to booting into kdump kernel) and that's why the notion
    is_kdump_kernel().

    To me, is_kdump_kernel() symbolizes whether I am booting after kexec on
    panic and not just the fact if CONFIG_CRASH_DUMP is enabled or not in this
    kernel.

    So I would think that lets not rename it to kernel_has_vmcore(), instead
    lets write few lines of comments before function is_kdump_kernel() to
    clarify its meaning.

    Secondly, we are using elfcorehdr_addr to determine whether this kernel
    is booting after a panic so elfcorehdr_addr is not just limited to
    CONFIG_PROC_VMCORE now and we should probably pull it out of
    fs/proc/vmcore.c. How about declaring and initializing this variable in
    kernel/kexec.c under CONFIG_CRASH_DUMP and always parse elfcorehdr_addr
    irrespective of the setting of CONFIG_PROC_VMCORE?

    > 2) Add a new kernel command line option, perhaps in_kdump
    >
    > This is bloat to get around elfcorehdr_addr initialisation and
    > naming awkwardness above.
    >
    > 3) Make select CONFIG_PROC_VMCORE when CONFIG_CRASH_DUMP is selected,
    > or perhaps even just remove CONFIG_PROC_VMCORE and only use
    > CONFIG_CRASH_DUMP instead. The effect would be the same either way.
    >
    > Pro: One less thing to be confused about
    >
    > Con: Bloat for people who want kdump without vmcore.
    > I wonder what usage case that is.


    Argument was people can use /dev/oldmem and not use /proc/vmcore. So far
    I don't know anybody who uses /dev/oldmem to capture dump and not
    /proc/vmcore.

    Thanks
    Vivek
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    On Mon, Jul 28, 2008 at 09:24:46AM +0300, Muli Ben-Yehuda wrote:
    > On Sun, Jul 27, 2008 at 10:39:30PM -0700, Eric W. Biederman wrote:
    > > Simon Horman writes:
    > >
    > > > There has long been a strong aversion to providing the second
    > > > kernel with flags like im_in_kexec or im_in_kdump, as its felt
    > > > that this kind of problem is better handled by making sure that
    > > > the hardware is in a sensible state before leaving the
    > > > first-kernel. But this is arguably more reasonable in the kexec
    > > > case than the kdump case.

    > >
    > > That and because we can generally solve the specific problem with a
    > > general feature. Something we can enable/disable on the command
    > > line if needed. Right now this is especially interesting as on
    > > several architectures distros are not building special kdump kernels
    > > but have a single kernel binary that works in both cases.
    > >
    > > Skimming through your patches this is a case we really do need to
    > > implement and handle cleanly.
    > >
    > > Currently we leave DMA running in the kexec on panic case. We avoid
    > > problems by only running out of a reserved area of memory.
    > >
    > > As as general strategy that is fine. However we have not
    > > implemented that strategy in the case of IOMMUs. And we are having
    > > trouble with IOMMUs.
    > >
    > > My hunch is that we should implement options to reserve a section of
    > > the iommu and to tell to the iommu to use the previously reserved
    > > section. Although turning iommus off altogether and simply using
    > > swiotlb may be acceptable. In which case we should just force usage
    > > of the swiotlb on the command line in /sbin/kexec.

    >
    > With an isolation-capable IOMMU (such as Calgary, VT-d and AMD's
    > IOMMU) on the I/O path, as long as we want to keep DMAs running and
    > going through to memory, we need to keep the IOMMU running, with the
    > same set of permissions and translation tables. If we don't mind DMAs
    > failing, we need to gracefully handle IOMMU DMA faults (where
    > possible---Calgary can't do it at the moment). What we do instead with
    > Calgary (c.f., the patch that instigated this discussion) is a hack,
    > we "reinitialize" the IOMMU with the old IOMMU's translation tables so
    > that DMAs continue working.
    >


    Hi Muli,

    Agree, using old kernel's TCE tables is a hack. As Eric pointed out,
    is there a reason why swiotlb will not work here? (I guess using
    swiotlb will mean disabling iommu and that will again fault if
    DMA is going on).

    So one of the solutions, as eric suggested, will be to reserve some
    entries in first kernel and then pass that info to second kernel and
    let second kernel use thos entries for setting up DMA and let the
    DMA's of first kernel run untouched.

    This patch is effectively doing that (using previous kernel's TCE table),
    except the fact that there are no gurantees that there are free table
    entries when kdump kernel wants to perform a DMA of its own. Should
    probably work though in most of the cases.

    > My preference would be to have stopped all DMAs in the old kernel,
    > which would've made this nastiness go away. Can you explain in simple
    > words why we can't or won't do that?
    >


    Is there a reliable way of stopping all ongoing DMAs after kernel crash?

    Thanks
    Vivek
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    Vivek Goyal writes:

    > On Mon, Jul 28, 2008 at 09:24:46AM +0300, Muli Ben-Yehuda wrote:
    >>
    >> With an isolation-capable IOMMU (such as Calgary, VT-d and AMD's
    >> IOMMU) on the I/O path, as long as we want to keep DMAs running and
    >> going through to memory, we need to keep the IOMMU running, with the
    >> same set of permissions and translation tables. If we don't mind DMAs
    >> failing, we need to gracefully handle IOMMU DMA faults (where
    >> possible---Calgary can't do it at the moment). What we do instead with
    >> Calgary (c.f., the patch that instigated this discussion) is a hack,
    >> we "reinitialize" the IOMMU with the old IOMMU's translation tables so
    >> that DMAs continue working.
    >>

    >
    > Hi Muli,
    >
    > Agree, using old kernel's TCE tables is a hack. As Eric pointed out,
    > is there a reason why swiotlb will not work here? (I guess using
    > swiotlb will mean disabling iommu and that will again fault if
    > DMA is going on).
    >
    > So one of the solutions, as eric suggested, will be to reserve some
    > entries in first kernel and then pass that info to second kernel and
    > let second kernel use thos entries for setting up DMA and let the
    > DMA's of first kernel run untouched.
    >
    > This patch is effectively doing that (using previous kernel's TCE table),
    > except the fact that there are no gurantees that there are free table
    > entries when kdump kernel wants to perform a DMA of its own. Should
    > probably work though in most of the cases.
    >
    >> My preference would be to have stopped all DMAs in the old kernel,
    >> which would've made this nastiness go away. Can you explain in simple
    >> words why we can't or won't do that?

    >
    > Is there a reliable way of stopping all ongoing DMAs after kernel crash?


    When I investigated this there was no reliable way to stop DMAs from devices,
    in a generic way, and a callback to each device in the system in the kexec
    on panic path is less reliable then simply avoiding running out of regions
    where those DMAs are running.

    So it would be quite reasonable to drop all in flight DMA at the iommu.

    We do need a way to allow the drivers in the kernel running after the
    panic to use DMA. Which is where the idea of reserving a region of
    the iommu comes from.

    Eric
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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


    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
    ---

    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;
    _
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    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
    ---

    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;
    _
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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



    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
    ---

    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
    #include

    +/* 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
    _
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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



    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
    ---

    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)
    {
    _
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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




    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
    ---

    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
    #include

    +/* 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
    _
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

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

    Vivek Goyal 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"

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  20. Re: [patch] crashdump: fix undefined reference to `elfcorehdr_addr'

    On Mon, Jul 28, 2008 at 09:31:10AM -0400, Vivek Goyal wrote:
    > On Mon, Jul 28, 2008 at 11:51:19AM +1000, Simon Horman wrote:
    > > [ Updated Vivek's email address to his vgoyal@redhat.com in CC list
    > > Added Terry Loftin, Tony Luck, Erik Biedermann and linux-ia64 to CC list ]


    [snip]

    > > 1) Always parse the elfcorehdr kernel command line option
    > > and set elfcorehdr_addr accordingly - currently this is only
    > > done if CONFIG_PROC_VMCORE is set.
    > >
    > > This is nice as it won't need any modifications to kexec-tools
    > > nor any command line bloat.
    > >
    > > A minor difficulty is working out where to initialise elfcorehdr_addr.
    > > Sometimes in include/linux/crash_dump.h and sometimes in
    > > fs/proc/vmcore.c seems horrible to me.
    > >
    > > Another problem is that would be alive and well in
    > > code that really only uses it to check if kdump was activated or not
    > > - a minor naming issue.
    > >

    >
    > Hi Simon,
    >
    > There are some kernel bits (like iommu initialization patch), which need to
    > take special action if they are booting after a kexec on panic (Generally we
    > are referring it to booting into kdump kernel) and that's why the notion
    > is_kdump_kernel().
    >
    > To me, is_kdump_kernel() symbolizes whether I am booting after kexec on
    > panic and not just the fact if CONFIG_CRASH_DUMP is enabled or not in this
    > kernel.
    >
    > So I would think that lets not rename it to kernel_has_vmcore(), instead
    > lets write few lines of comments before function is_kdump_kernel() to
    > clarify its meaning.
    >
    > Secondly, we are using elfcorehdr_addr to determine whether this kernel
    > is booting after a panic so elfcorehdr_addr is not just limited to
    > CONFIG_PROC_VMCORE now and we should probably pull it out of
    > fs/proc/vmcore.c. How about declaring and initializing this variable in
    > kernel/kexec.c under CONFIG_CRASH_DUMP and always parse elfcorehdr_addr
    > irrespective of the setting of CONFIG_PROC_VMCORE?


    Agreed. Like Eric I think that this is a reasonable solution given
    the current state of things. I'll reply to your patches after looking
    at them a bit more closely.

    > > 2) Add a new kernel command line option, perhaps in_kdump
    > >
    > > This is bloat to get around elfcorehdr_addr initialisation and
    > > naming awkwardness above.
    > >
    > > 3) Make select CONFIG_PROC_VMCORE when CONFIG_CRASH_DUMP is selected,
    > > or perhaps even just remove CONFIG_PROC_VMCORE and only use
    > > CONFIG_CRASH_DUMP instead. The effect would be the same either way.
    > >
    > > Pro: One less thing to be confused about
    > >
    > > Con: Bloat for people who want kdump without vmcore.
    > > I wonder what usage case that is.

    >
    > Argument was people can use /dev/oldmem and not use /proc/vmcore. So far
    > I don't know anybody who uses /dev/oldmem to capture dump and not
    > /proc/vmcore.


    What I was getting at is that frangly the three variables
    CONFIG_KEXEC, CONFIG_CRASH_DUMP and CONFIG_PROC_VMCORE seem to
    confuse people. I've seen them used incorrectly several times now.
    So if there is a way to simplyfy things, even slightly, I think
    that would be a good idea. But if there isn't, so be it.

    --
    Horms

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread
Page 1 of 2 1 2 LastLast