kexec/kdump: Use generic elf code on ia64

Zou Nan hai nanhai.zou at intel.com
Tue May 8 02:55:44 EDT 2007


On Tue, 2007-05-08 at 14:32, Simon Horman wrote:
> Hi Nanhai,
> 
> could you take a moment to look at this patch to see if it
> addresses your concern about stack usage?
> 
> -- 
> Horms
>   H: http://www.vergenet.net/~horms/
>   W: http://www.valinux.co.jp/en/
> 
> On Wed, Apr 25, 2007 at 06:09:05PM +0900, Simon Horman wrote:
> > Make use of the generic implementation of crash_save_cpu().
> > 
> > On ia64 the registers are saved by a kdump-specific function
> > ia64_dump_cpu_regs() rather than elf_core_copy_regs() which
> > is used by other architectures via crash_save_cpu(). It seems
> > that ia64_dump_cpu_regs() and elf_core_copy_regs() are indeed
> > quite different
> > 
> > In order to facilitate this kdump_elf_core_copy_regs()
> > has been created, and it is called by crash_save_cpu().
> > By default kdump_elf_core_copy_regs() is just defined to be
> > elf_core_copy_regs() and ia64 defines its own implementation,
> > which calls ia64_dump_cpu_regs().  The ia64 version also sets
> > register 47 in accordance with the code that it replaces.
> > 
> > crash_save_cpu() has also been modifued to use a pre-alocated,
> > per-cpu variable for saving the cpu registers, as per the
> > IA64 specific code that this patch removes. This is
> > in order to reduce possible stack contention at crash-time.
> > 
> > The code appears to work, however as the place where the registers
> > are captured has changed natrually some of their values have also
> changed.
> > This makes looking for problems by comparing the new and old output
> a
> > little tricky.
> > 
> > Signed-off-by: Simon Horman <horms at verge.net.au>
> > 
> > --- 
> > * This patch applies on top of the note size calculation patch
> >   that can be found in mm and at
> >  
> http://lists.linux-foundation.org/pipermail/fastboot/2007-April/006792.html
> > 
> >   Porting this patch to not require that one is quite trivial.
> >   I can supply a port or a thread containing both patches if it
> helps.
> > 
> > * Update
> >   Diff against include/asm-ia64/kexec.h instead of
> include/asm/kexec.h
> >   Ooops!
> > 
> > * Updated crash_save_cpu() to use a per-cpu variable for the
> registers.
> >   As requested by Nanhai Zou
> > 
> > * This patch does three things, should it be split?
> >   - Makes crash_save_cpu() use a per-cpu variable.
> >   - Adds the kdump_elf_core_copy_regs() macro
> >   - Has IA64 use the generic code (which requires the other two
> changes).
> > 
> > * Added CC: kexec at lists.infradead.org, which is the new place for
> kexec
> >   discussion.
> > 
> >  arch/ia64/kernel/crash.c         |   46
> ++------------------------------------
> >  arch/ia64/kernel/machine_kexec.c |    2 -
> >  include/asm-ia64/kexec.h         |    7 ++++-
> >  include/linux/kexec.h            |    5 ++++
> >  kernel/kexec.c                   |   13 +++++++++-
> >  5 files changed, 26 insertions(+), 47 deletions(-)
> > Index: linux-2.6/arch/ia64/kernel/crash.c
> > ===================================================================
> > --- linux-2.6.orig/arch/ia64/kernel/crash.c   2007-04-25
> 17:31:54.000000000 +0900
> > +++ linux-2.6/arch/ia64/kernel/crash.c        2007-04-25
> 17:31:55.000000000 +0900
> > @@ -25,44 +25,11 @@ static atomic_t kdump_cpu_frozen;
> >  atomic_t kdump_in_progress;
> >  static int kdump_on_init = 1;
> >  
> > -static inline Elf64_Word
> > -*append_elf_note(Elf64_Word *buf, char *name, unsigned type, void
> *data,
> > -             size_t data_len)
> > -{
> > -     struct elf_note *note = (struct elf_note *)buf;
> > -     note->n_namesz = strlen(name) + 1;
> > -     note->n_descsz = data_len;
> > -     note->n_type   = type;
> > -     buf += (sizeof(*note) + 3)/4;
> > -     memcpy(buf, name, note->n_namesz);
> > -     buf += (note->n_namesz + 3)/4;
> > -     memcpy(buf, data, data_len);
> > -     buf += (data_len + 3)/4;
> > -     return buf;
> > -}
> > -
> > -static void
> > -final_note(void *buf)
> > -{
> > -     memset(buf, 0, sizeof(struct elf_note));
> > -}
> > -
> > -extern void ia64_dump_cpu_regs(void *);
> > -
> > -static DEFINE_PER_CPU(struct elf_prstatus, elf_prstatus);
> > -
> >  void
> > -crash_save_this_cpu(void)
> > +ia64_kexec_elf_core_copy_regs(elf_gregset_t *elfregs, struct
> pt_regs *regs)
> >  {
> > -     void *buf;
> >       unsigned long cfm, sof, sol;
> > -
> > -     int cpu = smp_processor_id();
> > -     struct elf_prstatus *prstatus = &per_cpu(elf_prstatus, cpu);
> > -
> > -     elf_greg_t *dst = (elf_greg_t *)&(prstatus->pr_reg);
> > -     memset(prstatus, 0, sizeof(*prstatus));
> > -     prstatus->pr_pid = current->pid;
> > +     elf_greg_t *dst = (elf_greg_t *)elfregs;
> >  
> >       ia64_dump_cpu_regs(dst);
> >       cfm = dst[43];
> > @@ -70,13 +37,6 @@ crash_save_this_cpu(void)
> >       sof = cfm & 0x7f;
> >       dst[46] = (unsigned long)ia64_rse_skip_regs((unsigned long
> *)dst[46],
> >                       sof - sol);
> > -
> > -     buf = (u64 *) per_cpu_ptr(crash_notes, cpu);
> > -     if (!buf)
> > -             return;
> > -     buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_PRSTATUS,
> prstatus,
> > -                     sizeof(*prstatus));
> > -     final_note(buf);
> >  }
> >  
> >  #ifdef CONFIG_SMP
> > @@ -134,7 +94,7 @@ kdump_cpu_freeze(struct unw_frame_info *
> >       int cpuid;
> >       local_irq_disable();
> >       cpuid = smp_processor_id();
> > -     crash_save_this_cpu();
> > +     crash_save_cpu(NULL, smp_processor_id());
> >       current->thread.ksp = (__u64)info->sw - 16;
> >       atomic_inc(&kdump_cpu_frozen);
> >       kdump_status[cpuid] = 1;
> > Index: linux-2.6/include/asm-ia64/kexec.h
> > ===================================================================
> > --- linux-2.6.orig/include/asm-ia64/kexec.h   2007-04-25
> 17:31:54.000000000 +0900
> > +++ linux-2.6/include/asm-ia64/kexec.h        2007-04-25
> 17:31:55.000000000 +0900
> > @@ -19,6 +19,9 @@
> >                  flush_icache_range(page_addr, page_addr +
> PAGE_SIZE); \
> >          } while(0)
> >  
> > +#define kexec_elf_core_copy_regs(elfregs, regs) \
> > +     ia64_kexec_elf_core_copy_regs(elfregs, regs)
> > +
> >  extern struct kimage *ia64_kimage;
> >  extern const unsigned int relocate_new_kernel_size;
> >  extern void relocate_new_kernel(unsigned long, unsigned long,
> > @@ -32,7 +35,9 @@ extern struct resource boot_param_res;
> >  extern void kdump_smp_send_stop(void);
> >  extern void kdump_smp_send_init(void);
> >  extern void kexec_disable_iosapic(void);
> > -extern void crash_save_this_cpu(void);
> > +extern void ia64_dump_cpu_regs(void *);
> > +extern void ia64_kexec_elf_core_copy_regs(elf_gregset_t *elfregs,
> > +                                       struct pt_regs *regs);
> >  struct rsvd_region;
> >  extern unsigned long kdump_find_rsvd_region(unsigned long size,
> >               struct rsvd_region *rsvd_regions, int n);
> > Index: linux-2.6/include/linux/kexec.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/kexec.h      2007-04-25
> 17:31:54.000000000 +0900
> > +++ linux-2.6/include/linux/kexec.h   2007-04-25 17:31:55.000000000
> +0900
> > @@ -128,6 +128,11 @@ extern struct kimage *kexec_crash_image;
> >  #define kexec_flush_icache_page(page)
> >  #endif
> >  
> > +#ifndef kexec_elf_core_copy_regs
> > +#define kexec_elf_core_copy_regs(elfregs, regs) \
> > +     elf_core_copy_regs(elfregs, regs)
> > +#endif
> > +
> >  #define KEXEC_ON_CRASH  0x00000001
> >  #define KEXEC_ARCH_MASK 0xffff0000
> >  
> > Index: linux-2.6/kernel/kexec.c
> > ===================================================================
> > --- linux-2.6.orig/kernel/kexec.c     2007-04-25 17:31:54.000000000
> +0900
> > +++ linux-2.6/kernel/kexec.c  2007-04-25 17:57:12.000000000 +0900
> > @@ -1097,9 +1097,18 @@ static void final_note(u32 *buf)
> >       memcpy(buf, &note, sizeof(note));
> >  }
> >  
> > +/*
> > + * There is some concern that on architectures where struct
> elf_prstatus is
> > + * large, such as IA64, having elf_prstatus on the stack in
> > + * crash_save_cpu() could cause problems given that this function
> > + * is run after a kernel crash has occured. To aleviate this
> problem
> > + * a pre-allocated per-cpu value is used instead.
> > + */
> > +static DEFINE_PER_CPU(struct elf_prstatus, elf_prstatus);
> > +
> >  void crash_save_cpu(struct pt_regs *regs, int cpu)
> >  {
> > -     struct elf_prstatus prstatus;
> > +     struct elf_prstatus *prstatus;
> >       u32 *buf;
> >  
> >       if ((cpu < 0) || (cpu >= NR_CPUS))
> > @@ -1107,7 +1116,7 @@ void crash_save_cpu(struct pt_regs *regs
> >  
> >       /* Using ELF notes here is opportunistic.
> >        * I need a well defined structure format
> > -      * for the data I pass, and I need tags
> > +      * for the data I pass, and I need tag)s
> >        * on the data to indicate what information I have
> >        * squirrelled away.  ELF notes happen to provide
> >        * all of that, so there is no need to invent something new.
> > @@ -1115,11 +1124,12 @@ void crash_save_cpu(struct pt_regs *regs
> >       buf = (u32*)per_cpu_ptr(crash_notes, cpu);
> >       if (!buf)
> >               return;
> > -     memset(&prstatus, 0, sizeof(prstatus));
> > -     prstatus.pr_pid = current->pid;
> > -     elf_core_copy_regs(&prstatus.pr_reg, regs);
> > +     prstatus = &per_cpu(elf_prstatus, cpu);
> > +     memset(prstatus, 0, sizeof(struct elf_prstatus));
> > +     prstatus->pr_pid = current->pid;
> > +     kexec_elf_core_copy_regs(&(prstatus->pr_reg), regs);
> >       buf = append_elf_note(buf, KEXEC_CORE_NOTE_NAME, NT_PRSTATUS,
> > -                           &prstatus, sizeof(prstatus));
> > +                           prstatus, sizeof(struct elf_prstatus));
> >       final_note(buf);
> >  }
> >  
> > Index: linux-2.6/arch/ia64/kernel/machine_kexec.c
> > ===================================================================
> > --- linux-2.6.orig/arch/ia64/kernel/machine_kexec.c   2007-04-25
> 17:31:54.000000000 +0900
> > +++ linux-2.6/arch/ia64/kernel/machine_kexec.c        2007-04-25
> 17:31:55.000000000 +0900
> > @@ -84,7 +84,7 @@ static void ia64_machine_kexec(struct un
> >  
> >       BUG_ON(!image);
> >       if (image->type == KEXEC_TYPE_CRASH) {
> > -             crash_save_this_cpu();
> > +             crash_save_cpu(NULL, smp_processor_id());
> >               current->thread.ksp = (__u64)info->sw - 16;
> >       }
> >  
> > -

Hi, 
	The patch looks ok to me.  
However split the patch in 2 parts, 
1. Put pr_status to percpu data to save stack usage.
2. IA64 specific part.
   may help others to review the patches.

Thanks
Zou Nan hai

> > To unsubscribe from this list: send the line "unsubscribe
> linux-ia64" in
> > the body of a message to majordomo at vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



More information about the kexec mailing list