Ken'ichi Ohmichi | 1 Aug 2011 03:27
Picon
Picon

Re: [PATCH v2 4/8] makedumpfile: Introduce routines to get type name from debuginfo.


Hi Mahesh,

A pointer size can been gotton by sizeof(void *), and pointer (virtual 
address) can been defined as "unsigned long".
I think we can make this patch simple. How about the attached patch ?

Thanks
Ken'ichi Ohmichi

diff --git a/makedumpfile.c b/makedumpfile.c
index 3ad2bd5..6955f64 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
 <at>  <at>  -34,7 +34,6  <at>  <at>  struct erase_info	*erase_info = NULL;
 unsigned long		num_erase_info = 1; /* Node 0 is unused. */

 char filename_stdout[] = FILENAME_STDOUT;
-long pointer_size;
 char config_buf[BUFSIZE_FGETS];

 /*
 <at>  <at>  -2058,10 +2057,6  <at>  <at>  get_debug_info(void)
 	 */
 	while (dwarf_nextcu(dwarfd, off, &next_off, &header_size,
 	    &abbrev_offset, &address_size, &offset_size) == 0) {
-		if (dwarf_info.cmd == DWARF_INFO_GET_PTR_SIZE) {
-			dwarf_info.struct_size = address_size;
-			break;
-		}
(Continue reading)

Vivek Goyal | 1 Aug 2011 22:16
Picon
Favicon

Re: [patch v2 01/10] kdump: Add KEXEC_CRASH_CONTROL_MEMORY_LIMIT

On Wed, Jul 27, 2011 at 02:55:05PM +0200, Michael Holzheu wrote:
> From: Michael Holzheu <holzheu@...>
> 
> On s390 there is a different KEXEC_CONTROL_MEMORY_LIMIT for the normal and
> the kdump kexec case. Therefore this patch introduces a new macro
> KEXEC_CRASH_CONTROL_MEMORY_LIMIT. This is set to
> KEXEC_CONTROL_MEMORY_LIMIT for all architectures that do not define
> KEXEC_CRASH_CONTROL_MEMORY_LIMIT.

Hi Michael,

Curious that why limit is different for kexec and kdump cases on s390
only.

Thanks
Vivek

> 
> Signed-off-by: Michael Holzheu <holzheu@...>
> ---
>  include/linux/kexec.h |    4 ++++
>  kernel/kexec.c        |    2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
>  <at>  <at>  -33,6 +33,10  <at>  <at> 
>  #error KEXEC_ARCH not defined
>  #endif
>  
(Continue reading)

Vivek Goyal | 1 Aug 2011 22:31
Picon
Favicon

Re: [patch v2 02/10] kdump: Make kimage_load_crash_segment() weak

On Wed, Jul 27, 2011 at 02:55:06PM +0200, Michael Holzheu wrote:
> From: Michael Holzheu <holzheu@...>
> 
> On s390 we do not create page tables at all for the crashkernel memory.
> This requires a s390 specific version for kimage_load_crash_segment().
> Therefore this patch declares this function as "__weak". The s390 version is
> very simple. It just copies the kexec segment to real memory without using
> page tables:
> 
> int kimage_load_crash_segment(struct kimage *image,
>                               struct kexec_segment *segment)
> {
>         return copy_from_user_real((void *) segment->mem, segment->buf,
>                                    segment->bufsz);
> }
> 
> There are two main advantages of not creating page tables for the
> crashkernel memory:
> 
> a) It saves memory. We have scenarios in mind, where crashkernel
>    memory can be very large and saving page table space is important.
> b) We protect the crashkernel memory from being overwritten.
> 
> Signed-off-by: Michael Holzheu <holzheu@...>
> ---
>  kernel/kexec.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
(Continue reading)

Vivek Goyal | 1 Aug 2011 22:36
Picon
Favicon

Re: [patch v2 03/10] kdump: Add size to elfcorehdr kernel parameter

On Wed, Jul 27, 2011 at 02:55:07PM +0200, Michael Holzheu wrote:
> From: Michael Holzheu <holzheu@...>
> 
> Currently only the address of the pre-allocated ELF header is passed with
> the elfcorehdr= kernel parameter. In order to reserve memory for the header
> in the 2nd kernel also the size is required. To pass the size with this
> patch the syntax of the kernel parameter is extended as follows:
> 
> elfcorehdr=[size[KMG] <at> ]offset[KMG]
> 
> This change is backward compatible because elfcorehdr=size is still allowed.

Generally vmcore parses elfcorehdr to figure out the sizes. kexec-tools
knows about it and relevant memory is excluded from second kernel's map
with the help of memmap= command line option.

Can you please mention that this is only s390 specific requirement as
there are no memmap= equivalent options and somehow dump tools wants
to know how big the elf header size is?

Thanks
Vivek

> 
> Signed-off-by: Michael Holzheu <holzheu@...>
> ---
>  Documentation/kernel-parameters.txt |    6 +++---
>  include/linux/crash_dump.h          |    1 +
>  kernel/crash_dump.c                 |   11 +++++++++++
>  3 files changed, 15 insertions(+), 3 deletions(-)
(Continue reading)

Vivek Goyal | 1 Aug 2011 22:41
Picon
Favicon

Re: [patch v2 04/10] kdump: Trigger kdump via panic notifier chain on s390

On Wed, Jul 27, 2011 at 02:55:08PM +0200, Michael Holzheu wrote:
> From: Michael Holzheu <holzheu@...>
> 
> On s390 we have the possibility to configure actions that are executed in
> case of a kernel panic. E.g. it is possible to automatically trigger an s390
> stand-alone dump. The actions are called via a panic notifier.  We also want
> to trigger kdump via the notifier call chain. Therefore this patch disables
> for s390 the direct kdump invocation in the panic() function.

Doesn't this reduce the reliability of the operation as you are assuming
that panic notifier list is fine and not corrupted.

There might be other generic notifiers registerd on panic notifier list
too. So in your case, are there multiple subsystem registering for panic
notifiers? If not, why not call crash_kexec() directly. Are there any
other actions you want to take on panic then calling crash_kexec()?

Thanks
Vivek

> 
> Signed-off-by: Michael Holzheu <holzheu@...>
> ---
>  kernel/panic.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
>  <at>  <at>  -84,9 +84,12  <at>  <at>  NORET_TYPE void panic(const char * fmt,
>  	/*
(Continue reading)

Vivek Goyal | 1 Aug 2011 22:54
Picon
Favicon

Re: [patch v2 05/10] s390: Add PSW restart shutdown trigger

On Wed, Jul 27, 2011 at 02:55:09PM +0200, Michael Holzheu wrote:
> From: Michael Holzheu <holzheu@...>
> 
> With this patch a new S390 shutdown trigger "restart" is added. If under
> z/VM "systerm restart" is entered or under the HMC the "PSW restart" button
> is pressed, the PSW located at 0 (31 bit) or 0x1a0 (64 bit) bit is loaded.
> Now we execute do_restart() that processes the restart action that is
> defined under /sys/firmware/shutdown_actions/on_restart. Currently the
> following actions are possible: reipl (default), stop, vmcmd, dump, and
> dump_reipl.

i know nothing about s390 but this sounds like a independent
functionality? I mean defining a restart trigger and associated actions
could be done even in current framework without kdump. IIUC, additional
kdump series only adds the ability to use kdump for second kernel instead
of using dump tools in case of "dump" or "dump_reipl" ?

If yes, then this patch probably does not have to be part of this series
and pushed in independetly?

Thanks
Vivek

> 
> Signed-off-by: Michael Holzheu <holzheu@...>
> ---
>  arch/s390/include/asm/lowcore.h |   11 +++++++++--
>  arch/s390/include/asm/system.h  |    1 +
>  arch/s390/kernel/asm-offsets.c  |    1 +
>  arch/s390/kernel/entry.S        |   28 ++++++++++++++++++++++++++++
(Continue reading)

Michael Holzheu | 2 Aug 2011 10:05
Picon

Re: [patch v2 05/10] s390: Add PSW restart shutdown trigger

Hello Vivek,

On Mon, 2011-08-01 at 16:54 -0400, Vivek Goyal wrote:
> On Wed, Jul 27, 2011 at 02:55:09PM +0200, Michael Holzheu wrote:
> > From: Michael Holzheu <holzheu@...>
> > 
> > With this patch a new S390 shutdown trigger "restart" is added. If under
> > z/VM "systerm restart" is entered or under the HMC the "PSW restart" button
> > is pressed, the PSW located at 0 (31 bit) or 0x1a0 (64 bit) bit is loaded.
> > Now we execute do_restart() that processes the restart action that is
> > defined under /sys/firmware/shutdown_actions/on_restart. Currently the
> > following actions are possible: reipl (default), stop, vmcmd, dump, and
> > dump_reipl.
> 
> i know nothing about s390 but this sounds like a independent
> functionality? I mean defining a restart trigger and associated actions
> could be done even in current framework without kdump. IIUC, additional
> kdump series only adds the ability to use kdump for second kernel instead
> of using dump tools in case of "dump" or "dump_reipl" ?
> 
> If yes, then this patch probably does not have to be part of this series
> and pushed in independetly?

Yes, this is correct. We implemented the PSW restart functionality for
kdump, but it can be used independently.

We will integrate this patch upstream independently from this series.

Michael
(Continue reading)

Michael Holzheu | 2 Aug 2011 11:08
Picon

Re: [patch v2 03/10] kdump: Add size to elfcorehdr kernel parameter

Hello Vivek,

#On Mon, 2011-08-01 at 16:36 -0400, Vivek Goyal wrote: 
> On Wed, Jul 27, 2011 at 02:55:07PM +0200, Michael Holzheu wrote:
> > From: Michael Holzheu <holzheu@...>
> > 
> > Currently only the address of the pre-allocated ELF header is passed with
> > the elfcorehdr= kernel parameter. In order to reserve memory for the header
> > in the 2nd kernel also the size is required. To pass the size with this
> > patch the syntax of the kernel parameter is extended as follows:
> > 
> > elfcorehdr=[size[KMG] <at> ]offset[KMG]
> > 
> > This change is backward compatible because elfcorehdr=size is still allowed.
> 
> Generally vmcore parses elfcorehdr to figure out the sizes. kexec-tools
> knows about it and relevant memory is excluded from second kernel's map
> with the help of memmap= command line option.
> 
> Can you please mention that this is only s390 specific requirement as
> there are no memmap= equivalent options and somehow dump tools wants
> to know how big the elf header size is?

I updated the description: 

Currently only the address of the pre-allocated ELF header is passed with
the elfcorehdr= kernel parameter. In order to reserve memory for the header
in the 2nd kernel also the size is required. Current kdump architecture
backends use different methods to do that, e.g. x86 uses the memmap= kernel
parameter. On s390 there is no easy way to transfer this information.
(Continue reading)

Michael Holzheu | 2 Aug 2011 11:51
Picon

Re: [patch v2 01/10] kdump: Add KEXEC_CRASH_CONTROL_MEMORY_LIMIT

Hello Vivek,

On Mon, 2011-08-01 at 16:16 -0400, Vivek Goyal wrote:
> On Wed, Jul 27, 2011 at 02:55:05PM +0200, Michael Holzheu wrote:
> > From: Michael Holzheu <holzheu@...>
> > 
> > On s390 there is a different KEXEC_CONTROL_MEMORY_LIMIT for the normal and
> > the kdump kexec case. Therefore this patch introduces a new macro
> > KEXEC_CRASH_CONTROL_MEMORY_LIMIT. This is set to
> > KEXEC_CONTROL_MEMORY_LIMIT for all architectures that do not define
> > KEXEC_CRASH_CONTROL_MEMORY_LIMIT.
> 
> Hi Michael,
> 
> Curious that why limit is different for kexec and kdump cases on s390
> only.

The standard kexec relocate_kernel code calls a machine instruction that
must run below 2 GiB. For kdump we currently do not use the control page
at all because no segments have to be moved in that case. Perhaps I am
still missing something here?

Michael
Vivek Goyal | 2 Aug 2011 20:55
Picon
Favicon

Re: [patch v2 03/10] kdump: Add size to elfcorehdr kernel parameter

On Tue, Aug 02, 2011 at 11:08:07AM +0200, Michael Holzheu wrote:
> Hello Vivek,
> 
> #On Mon, 2011-08-01 at 16:36 -0400, Vivek Goyal wrote: 
> > On Wed, Jul 27, 2011 at 02:55:07PM +0200, Michael Holzheu wrote:
> > > From: Michael Holzheu <holzheu@...>
> > > 
> > > Currently only the address of the pre-allocated ELF header is passed with
> > > the elfcorehdr= kernel parameter. In order to reserve memory for the header
> > > in the 2nd kernel also the size is required. To pass the size with this
> > > patch the syntax of the kernel parameter is extended as follows:
> > > 
> > > elfcorehdr=[size[KMG] <at> ]offset[KMG]
> > > 
> > > This change is backward compatible because elfcorehdr=size is still allowed.
> > 
> > Generally vmcore parses elfcorehdr to figure out the sizes. kexec-tools
> > knows about it and relevant memory is excluded from second kernel's map
> > with the help of memmap= command line option.
> > 
> > Can you please mention that this is only s390 specific requirement as
> > there are no memmap= equivalent options and somehow dump tools wants
> > to know how big the elf header size is?
> 
> I updated the description: 
> 
> Currently only the address of the pre-allocated ELF header is passed with
> the elfcorehdr= kernel parameter. In order to reserve memory for the header
> in the 2nd kernel also the size is required. Current kdump architecture
> backends use different methods to do that, e.g. x86 uses the memmap= kernel
(Continue reading)


Gmane