[PATCH v2 0/7] makedumpfile: Xen fixes and minor cleanups

[PATCH v2 0/7] makedumpfile: Xen fixes and minor cleanups

[Daniel Kiper]

Hi,

This patch series contains following makedumpfile Xen fixes and minor cleanups:
  - xen: Improve calculation of beginning of virtual address space,
  - elf: Properly check buf size in get_pt_note_info(),
  - xen: Take into account new frame table address since Xen 4.3,
  - xen: Enforce page size only when xen-syms file is used,
  - Do not break progress messages,
  - Mute some compiler warnings,
  - Use elf_getshdrstrndx() instead of elf_getshstrndx().

All patches were tested with Xen versions up to 4.3.

Please apply.

Daniel

[PATCH v2 1/7] xen: Improve calculation of beginning of virtual address space

[Daniel Kiper]

Xen commit 4b28bf6ae90bd83fd1113d8bdc53c3266ffeb328 (x86: re-introduce
map_domain_page() et al) once again altered virtual address space.
Current algorithm calculating its start could not cope with that change.
New version establishes this value on the base of domain_list placement
and is more generic.

Similar patch was applied to crash tool.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 arch/x86_64.c  |   16 +++++++++++-----
 makedumpfile.h |    2 --
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index 8e61b20..d864a06 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -372,14 +372,20 @@ int get_xen_basic_info_x86_64(void)
 		info->xen_phys_start = info->xen_crash_info.v2->xen_phys_start;
 	}
 
+	info->xen_virt_start = SYMBOL(domain_list);
+
+	/*
+	 * Xen virtual mapping is aligned to 1 GiB boundary.
+	 * domain_list lives in bss which sits no more than
+	 * 1 GiB below beginning of virtual address space.
+	 */
+	info->xen_virt_start &= 0xffffffffc0000000;
+
 	if (info->xen_crash_info.com &&
-	    info->xen_crash_info.com->xen_major_version >= 4) {
-		info->xen_virt_start = XEN_VIRT_START_V4;
+	    info->xen_crash_info.com->xen_major_version >= 4)
 		info->directmap_virt_end = DIRECTMAP_VIRT_END_V4;
-	} else {
-		info->xen_virt_start = XEN_VIRT_START_V3;
+	else
 		info->directmap_virt_end = DIRECTMAP_VIRT_END_V3;
-	}
 
 	if (SYMBOL(pgd_l4) == NOT_FOUND_SYMBOL) {
 		ERRMSG("Can't get pml4.\n");
diff --git a/makedumpfile.h b/makedumpfile.h
index a5826e0..1a87500 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1473,8 +1473,6 @@ int get_xen_info_x86(void);
 #define DIRECTMAP_VIRT_END_V3 (0xffff840000000000)
 #define DIRECTMAP_VIRT_END_V4 (0xffff880000000000)
 #define DIRECTMAP_VIRT_END    (info->directmap_virt_end)
-#define XEN_VIRT_START_V3     (0xffff828c80000000)
-#define XEN_VIRT_START_V4     (0xffff82c480000000)
 #define XEN_VIRT_START        (info->xen_virt_start)
 #define XEN_VIRT_END          (XEN_VIRT_START + (1UL << 30))
 #define FRAMETABLE_VIRT_START 0xffff82f600000000
-- 
1.7.10.4

Re: [PATCH v2 1/7] xen: Improve calculation of beginning of virtual address space

[Atsushi Kumagai]

On Tue, 16 Jul 2013 14:32:03 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> Xen commit 4b28bf6ae90bd83fd1113d8bdc53c3266ffeb328 (x86: re-introduce
> map_domain_page() et al) once again altered virtual address space.
> Current algorithm calculating its start could not cope with that change.
> New version establishes this value on the base of domain_list placement
> and is more generic.
> 
> Similar patch was applied to crash tool.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

OK, I'll merge this patch into v1.5.5.


Thanks
Atsushi Kumagai

> ---
>  arch/x86_64.c  |   16 +++++++++++-----
>  makedumpfile.h |    2 --
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86_64.c b/arch/x86_64.c
> index 8e61b20..d864a06 100644
> --- a/arch/x86_64.c
> +++ b/arch/x86_64.c
> @@ -372,14 +372,20 @@ int get_xen_basic_info_x86_64(void)
>  		info->xen_phys_start = info->xen_crash_info.v2->xen_phys_start;
>  	}
>  
> +	info->xen_virt_start = SYMBOL(domain_list);
> +
> +	/*
> +	 * Xen virtual mapping is aligned to 1 GiB boundary.
> +	 * domain_list lives in bss which sits no more than
> +	 * 1 GiB below beginning of virtual address space.
> +	 */
> +	info->xen_virt_start &= 0xffffffffc0000000;
> +
>  	if (info->xen_crash_info.com &&
> -	    info->xen_crash_info.com->xen_major_version >= 4) {
> -		info->xen_virt_start = XEN_VIRT_START_V4;
> +	    info->xen_crash_info.com->xen_major_version >= 4)
>  		info->directmap_virt_end = DIRECTMAP_VIRT_END_V4;
> -	} else {
> -		info->xen_virt_start = XEN_VIRT_START_V3;
> +	else
>  		info->directmap_virt_end = DIRECTMAP_VIRT_END_V3;
> -	}
>  
>  	if (SYMBOL(pgd_l4) == NOT_FOUND_SYMBOL) {
>  		ERRMSG("Can't get pml4.\n");
> diff --git a/makedumpfile.h b/makedumpfile.h
> index a5826e0..1a87500 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -1473,8 +1473,6 @@ int get_xen_info_x86(void);
>  #define DIRECTMAP_VIRT_END_V3 (0xffff840000000000)
>  #define DIRECTMAP_VIRT_END_V4 (0xffff880000000000)
>  #define DIRECTMAP_VIRT_END    (info->directmap_virt_end)
> -#define XEN_VIRT_START_V3     (0xffff828c80000000)
> -#define XEN_VIRT_START_V4     (0xffff82c480000000)
>  #define XEN_VIRT_START        (info->xen_virt_start)
>  #define XEN_VIRT_END          (XEN_VIRT_START + (1UL << 30))
>  #define FRAMETABLE_VIRT_START 0xffff82f600000000
> -- 
> 1.7.10.4

[PATCH v2 2/7] elf: Properly check buf size in get_pt_note_info()

[Daniel Kiper]

get_pt_note_info() always ignores VMCOREINFO_XEN note because
buf size must be smaller than note size including \0 char.
Usualy this condition is true but when VMCOREINFO_XEN note
is encountered it does not work due to buf size defined
as sizeof("VMCOREINFO_XEN"). This patch fixes that bug
and VMCOREINFO_XEN note living in /proc/vmcore file
could be properly detected now.

v2 - suggestions/fixes:
   - properly check buf size instead of increasing buf size
     (suggested by Andrew Cooper).

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 elf_info.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/elf_info.c b/elf_info.c
index 0c1e36a..e350b99 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -332,7 +332,7 @@ get_pt_note_info(void)
 		size_desc   = note_descsz(note);
 		offset_desc = offset + offset_note_desc(note);
 
-		if (!size_name || size_name >= sizeof(buf))
+		if (!size_name || size_name > sizeof(buf))
 			goto next_note;
 
 		if (read(fd_memory, &buf, sizeof(buf)) != sizeof(buf)) {
-- 
1.7.10.4

Re: [PATCH v2 2/7] elf: Properly check buf size in get_pt_note_info()

[Atsushi Kumagai]

On Tue, 16 Jul 2013 14:32:04 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> get_pt_note_info() always ignores VMCOREINFO_XEN note because
> buf size must be smaller than note size including \0 char.
> Usualy this condition is true but when VMCOREINFO_XEN note
> is encountered it does not work due to buf size defined
> as sizeof("VMCOREINFO_XEN"). This patch fixes that bug
> and VMCOREINFO_XEN note living in /proc/vmcore file
> could be properly detected now.
> 
> v2 - suggestions/fixes:
>    - properly check buf size instead of increasing buf size
>      (suggested by Andrew Cooper).
> 
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Good, I'll merge this patch into v1.5.5.


Thanks
Atsushi Kumagai

> ---
>  elf_info.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/elf_info.c b/elf_info.c
> index 0c1e36a..e350b99 100644
> --- a/elf_info.c
> +++ b/elf_info.c
> @@ -332,7 +332,7 @@ get_pt_note_info(void)
>  		size_desc   = note_descsz(note);
>  		offset_desc = offset + offset_note_desc(note);
>  
> -		if (!size_name || size_name >= sizeof(buf))
> +		if (!size_name || size_name > sizeof(buf))
>  			goto next_note;
>  
>  		if (read(fd_memory, &buf, sizeof(buf)) != sizeof(buf)) {
> -- 
> 1.7.10.4
> 

[PATCH v2 3/7] xen: Take into account new frame table address since Xen 4.3

[Daniel Kiper]

Since Xen commit a8d2b06db7826063df9d04be9d6f928bf2189bd0
(x86: extend frame table virtual space) frame table has
new address. Take into account that thing.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 arch/x86_64.c  |   11 +++++++++--
 makedumpfile.h |   21 +++++++++++----------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index d864a06..771d457 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -401,8 +401,15 @@ int get_xen_basic_info_x86_64(void)
 			return FALSE;
 		}
 		info->frame_table_vaddr = frame_table_vaddr;
-	} else
-		info->frame_table_vaddr = FRAMETABLE_VIRT_START;
+	} else {
+		if (info->xen_crash_info.com &&
+		    ((info->xen_crash_info.com->xen_major_version == 4 &&
+		      info->xen_crash_info.com->xen_minor_version >= 3) ||
+		      info->xen_crash_info.com->xen_major_version > 4))
+			info->frame_table_vaddr = FRAMETABLE_VIRT_START_V4_3;
+		else
+			info->frame_table_vaddr = FRAMETABLE_VIRT_START_V3;
+	}
 
 	if (!info->xen_crash_info.com ||
 	    info->xen_crash_info.com->xen_major_version < 4) {
diff --git a/makedumpfile.h b/makedumpfile.h
index 1a87500..1789a11 100644
--- a/makedumpfile.h
+++ b/makedumpfile.h
@@ -1466,16 +1466,17 @@ int get_xen_info_x86(void);
 #define ENTRY_MASK		(~0xfff0000000000fffULL)
 #define MAX_X86_64_FRAMES	(info->page_size / sizeof(unsigned long))
 
-#define PAGE_OFFSET_XEN_DOM0  (0xffff880000000000) /* different from linux */
-#define HYPERVISOR_VIRT_START (0xffff800000000000)
-#define HYPERVISOR_VIRT_END   (0xffff880000000000)
-#define DIRECTMAP_VIRT_START  (0xffff830000000000)
-#define DIRECTMAP_VIRT_END_V3 (0xffff840000000000)
-#define DIRECTMAP_VIRT_END_V4 (0xffff880000000000)
-#define DIRECTMAP_VIRT_END    (info->directmap_virt_end)
-#define XEN_VIRT_START        (info->xen_virt_start)
-#define XEN_VIRT_END          (XEN_VIRT_START + (1UL << 30))
-#define FRAMETABLE_VIRT_START 0xffff82f600000000
+#define PAGE_OFFSET_XEN_DOM0		(0xffff880000000000) /* different from linux */
+#define HYPERVISOR_VIRT_START		(0xffff800000000000)
+#define HYPERVISOR_VIRT_END		(0xffff880000000000)
+#define DIRECTMAP_VIRT_START		(0xffff830000000000)
+#define DIRECTMAP_VIRT_END_V3		(0xffff840000000000)
+#define DIRECTMAP_VIRT_END_V4		(0xffff880000000000)
+#define DIRECTMAP_VIRT_END		(info->directmap_virt_end)
+#define XEN_VIRT_START			(info->xen_virt_start)
+#define XEN_VIRT_END			(XEN_VIRT_START + (1UL << 30))
+#define FRAMETABLE_VIRT_START_V3	0xffff82f600000000
+#define FRAMETABLE_VIRT_START_V4_3	0xffff82e000000000
 
 #define is_xen_vaddr(x) \
 	((x) >= HYPERVISOR_VIRT_START && (x) < HYPERVISOR_VIRT_END)
-- 
1.7.10.4

Re: [PATCH v2 3/7] xen: Take into account new frame table address since Xen 4.3

[Atsushi Kumagai]

On Tue, 16 Jul 2013 14:32:05 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> Since Xen commit a8d2b06db7826063df9d04be9d6f928bf2189bd0
> (x86: extend frame table virtual space) frame table has
> new address. Take into account that thing.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

OK, I'll merge this patch into v1.5.5.


Thanks
Atsushi Kumagai

> ---
>  arch/x86_64.c  |   11 +++++++++--
>  makedumpfile.h |   21 +++++++++++----------
>  2 files changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86_64.c b/arch/x86_64.c
> index d864a06..771d457 100644
> --- a/arch/x86_64.c
> +++ b/arch/x86_64.c
> @@ -401,8 +401,15 @@ int get_xen_basic_info_x86_64(void)
>  			return FALSE;
>  		}
>  		info->frame_table_vaddr = frame_table_vaddr;
> -	} else
> -		info->frame_table_vaddr = FRAMETABLE_VIRT_START;
> +	} else {
> +		if (info->xen_crash_info.com &&
> +		    ((info->xen_crash_info.com->xen_major_version == 4 &&
> +		      info->xen_crash_info.com->xen_minor_version >= 3) ||
> +		      info->xen_crash_info.com->xen_major_version > 4))
> +			info->frame_table_vaddr = FRAMETABLE_VIRT_START_V4_3;
> +		else
> +			info->frame_table_vaddr = FRAMETABLE_VIRT_START_V3;
> +	}
>  
>  	if (!info->xen_crash_info.com ||
>  	    info->xen_crash_info.com->xen_major_version < 4) {
> diff --git a/makedumpfile.h b/makedumpfile.h
> index 1a87500..1789a11 100644
> --- a/makedumpfile.h
> +++ b/makedumpfile.h
> @@ -1466,16 +1466,17 @@ int get_xen_info_x86(void);
>  #define ENTRY_MASK		(~0xfff0000000000fffULL)
>  #define MAX_X86_64_FRAMES	(info->page_size / sizeof(unsigned long))
>  
> -#define PAGE_OFFSET_XEN_DOM0  (0xffff880000000000) /* different from linux */
> -#define HYPERVISOR_VIRT_START (0xffff800000000000)
> -#define HYPERVISOR_VIRT_END   (0xffff880000000000)
> -#define DIRECTMAP_VIRT_START  (0xffff830000000000)
> -#define DIRECTMAP_VIRT_END_V3 (0xffff840000000000)
> -#define DIRECTMAP_VIRT_END_V4 (0xffff880000000000)
> -#define DIRECTMAP_VIRT_END    (info->directmap_virt_end)
> -#define XEN_VIRT_START        (info->xen_virt_start)
> -#define XEN_VIRT_END          (XEN_VIRT_START + (1UL << 30))
> -#define FRAMETABLE_VIRT_START 0xffff82f600000000
> +#define PAGE_OFFSET_XEN_DOM0		(0xffff880000000000) /* different from linux */
> +#define HYPERVISOR_VIRT_START		(0xffff800000000000)
> +#define HYPERVISOR_VIRT_END		(0xffff880000000000)
> +#define DIRECTMAP_VIRT_START		(0xffff830000000000)
> +#define DIRECTMAP_VIRT_END_V3		(0xffff840000000000)
> +#define DIRECTMAP_VIRT_END_V4		(0xffff880000000000)
> +#define DIRECTMAP_VIRT_END		(info->directmap_virt_end)
> +#define XEN_VIRT_START			(info->xen_virt_start)
> +#define XEN_VIRT_END			(XEN_VIRT_START + (1UL << 30))
> +#define FRAMETABLE_VIRT_START_V3	0xffff82f600000000
> +#define FRAMETABLE_VIRT_START_V4_3	0xffff82e000000000
>  
>  #define is_xen_vaddr(x) \
>  	((x) >= HYPERVISOR_VIRT_START && (x) < HYPERVISOR_VIRT_END)
> -- 
> 1.7.10.4
>

[PATCH v2 4/7] xen: Enforce page size only when xen-syms file is used

[Daniel Kiper]

Enforce page size only when xen-syms file is used.
Otherwise its size could be read from VMCOREINFO
file or /proc/vmcore file.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 makedumpfile.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index b42565c..b4abbe5 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -7324,8 +7324,6 @@ initial_xen(void)
 #endif
 	if (!init_xen_crash_info())
 		return FALSE;
-	if (!fallback_to_current_page_size())
-		return FALSE;
 	/*
 	 * Get the debug information for analysis from the vmcoreinfo file
 	 */
@@ -7340,6 +7338,8 @@ initial_xen(void)
 		set_dwarf_debuginfo("xen-syms", NULL,
 				    info->name_xen_syms, info->fd_xen_syms);
 
+		if (!fallback_to_current_page_size())
+			return FALSE;
 		if (!get_symbol_info_xen())
 			return FALSE;
 		if (!get_structure_info_xen())
-- 
1.7.10.4

Re: [PATCH v2 4/7] xen: Enforce page size only when xen-syms file is used

[Atsushi Kumagai]

On Tue, 16 Jul 2013 14:32:06 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> Enforce page size only when xen-syms file is used.
> Otherwise its size could be read from VMCOREINFO
> file or /proc/vmcore file.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
>  makedumpfile.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index b42565c..b4abbe5 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -7324,8 +7324,6 @@ initial_xen(void)
>  #endif
>  	if (!init_xen_crash_info())
>  		return FALSE;
> -	if (!fallback_to_current_page_size())
> -		return FALSE;
>  	/*
>  	 * Get the debug information for analysis from the vmcoreinfo file
>  	 */
> @@ -7340,6 +7338,8 @@ initial_xen(void)
>  		set_dwarf_debuginfo("xen-syms", NULL,
>  				    info->name_xen_syms, info->fd_xen_syms);
>  
> +		if (!fallback_to_current_page_size())
> +			return FALSE;
>  		if (!get_symbol_info_xen())
>  			return FALSE;
>  		if (!get_structure_info_xen())

This patch has no problem, but I would like to fix a bit for readability.
So could you move the position of fallback_to_current_page_size() to after
this block like initial() ?

makedumpfile.c::initial()

   2972         if (!info->page_size) {
   2973                 /*
   2974                  * If we cannot get page_size from a vmcoreinfo file,
   2975                  * fall back to the current kernel page size.
   2976                  */
   2977                 if (!fallback_to_current_page_size())
   2978                         return FALSE;
   2979         }


Thanks
Atsushi Kumagai

Re: [PATCH v2 4/7] xen: Enforce page size only when xen-syms file is used

[Daniel Kiper]

On Fri, Jul 19, 2013 at 06:20:24PM +0900, Atsushi Kumagai wrote:
> On Tue, 16 Jul 2013 14:32:06 +0200
> Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>
> > Enforce page size only when xen-syms file is used.
> > Otherwise its size could be read from VMCOREINFO
> > file or /proc/vmcore file.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > ---
> >  makedumpfile.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/makedumpfile.c b/makedumpfile.c
> > index b42565c..b4abbe5 100644
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -7324,8 +7324,6 @@ initial_xen(void)
> >  #endif
> >  	if (!init_xen_crash_info())
> >  		return FALSE;
> > -	if (!fallback_to_current_page_size())
> > -		return FALSE;
> >  	/*
> >  	 * Get the debug information for analysis from the vmcoreinfo file
> >  	 */
> > @@ -7340,6 +7338,8 @@ initial_xen(void)
> >  		set_dwarf_debuginfo("xen-syms", NULL,
> >  				    info->name_xen_syms, info->fd_xen_syms);
> >
> > +		if (!fallback_to_current_page_size())
> > +			return FALSE;
> >  		if (!get_symbol_info_xen())
> >  			return FALSE;
> >  		if (!get_structure_info_xen())
>
> This patch has no problem, but I would like to fix a bit for readability.
> So could you move the position of fallback_to_current_page_size() to after
> this block like initial() ?
>
> makedumpfile.c::initial()
>
>    2972         if (!info->page_size) {
>    2973                 /*
>    2974                  * If we cannot get page_size from a vmcoreinfo file,
>    2975                  * fall back to the current kernel page size.
>    2976                  */
>    2977                 if (!fallback_to_current_page_size())
>    2978                         return FALSE;
>    2979         }

Sure. Should I repost whole series or just only this patch?

Daniel

Re: [PATCH v2 4/7] xen: Enforce page size only when xen-syms file is used

[Atsushi Kumagai]

On Fri, 19 Jul 2013 15:44:51 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> On Fri, Jul 19, 2013 at 06:20:24PM +0900, Atsushi Kumagai wrote:
> > On Tue, 16 Jul 2013 14:32:06 +0200
> > Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > > Enforce page size only when xen-syms file is used.
> > > Otherwise its size could be read from VMCOREINFO
> > > file or /proc/vmcore file.
> > >
> > > Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  makedumpfile.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/makedumpfile.c b/makedumpfile.c
> > > index b42565c..b4abbe5 100644
> > > --- a/makedumpfile.c
> > > +++ b/makedumpfile.c
> > > @@ -7324,8 +7324,6 @@ initial_xen(void)
> > >  #endif
> > >  	if (!init_xen_crash_info())
> > >  		return FALSE;
> > > -	if (!fallback_to_current_page_size())
> > > -		return FALSE;
> > >  	/*
> > >  	 * Get the debug information for analysis from the vmcoreinfo file
> > >  	 */
> > > @@ -7340,6 +7338,8 @@ initial_xen(void)
> > >  		set_dwarf_debuginfo("xen-syms", NULL,
> > >  				    info->name_xen_syms, info->fd_xen_syms);
> > >
> > > +		if (!fallback_to_current_page_size())
> > > +			return FALSE;
> > >  		if (!get_symbol_info_xen())
> > >  			return FALSE;
> > >  		if (!get_structure_info_xen())
> >
> > This patch has no problem, but I would like to fix a bit for readability.
> > So could you move the position of fallback_to_current_page_size() to after
> > this block like initial() ?
> >
> > makedumpfile.c::initial()
> >
> >    2972         if (!info->page_size) {
> >    2973                 /*
> >    2974                  * If we cannot get page_size from a vmcoreinfo file,
> >    2975                  * fall back to the current kernel page size.
> >    2976                  */
> >    2977                 if (!fallback_to_current_page_size())
> >    2978                         return FALSE;
> >    2979         }
> 
> Sure. Should I repost whole series or just only this patch?
> 
> Daniel

Please repost only this patch.


Thanks
Atsushi Kumagai

Re: [PATCH v2 4/7] xen: Enforce page size only when xen-syms file is used

[Daniel Kiper]

On Mon, Jul 22, 2013 at 04:35:29PM +0900, Atsushi Kumagai wrote:
> On Fri, 19 Jul 2013 15:44:51 +0200
> Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>
> > On Fri, Jul 19, 2013 at 06:20:24PM +0900, Atsushi Kumagai wrote:

[...]

> > > makedumpfile.c::initial()
> > >
> > >    2972         if (!info->page_size) {
> > >    2973                 /*
> > >    2974                  * If we cannot get page_size from a vmcoreinfo file,
> > >    2975                  * fall back to the current kernel page size.
> > >    2976                  */
> > >    2977                 if (!fallback_to_current_page_size())
> > >    2978                         return FALSE;
> > >    2979         }
> >
> > Sure. Should I repost whole series or just only this patch?
> >
> > Daniel
>
> Please repost only this patch.

Done.

Daniel

[PATCH v2 5/7] Do not break progress messages

[Daniel Kiper]

DEBUG_MSG() displaying erase info size breaks progress messages.
Fix this by moving relevant DEBUG_MSG() to write_elf_eraseinfo().

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 makedumpfile.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index b4abbe5..a3bfc80 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -5045,7 +5045,6 @@ write_elf_header(struct cache_data *cd_header)
 	 * function.
 	 */
 	info->size_elf_eraseinfo = size_eraseinfo;
-	DEBUG_MSG("erase info size: %lu\n", info->size_elf_eraseinfo);
 
 	/*
 	 * Write a PT_NOTE header.
@@ -6326,6 +6325,8 @@ write_elf_eraseinfo(struct cache_data *cd_header)
 	off_t offset_eraseinfo;
 	unsigned long note_header_size, size_written, size_note;
 
+	DEBUG_MSG("erase info size: %lu\n", info->size_elf_eraseinfo);
+
 	if (!info->size_elf_eraseinfo)
 		return TRUE;
 
-- 
1.7.10.4

Re: [PATCH v2 5/7] Do not break progress messages

[Atsushi Kumagai]

On Tue, 16 Jul 2013 14:32:07 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> DEBUG_MSG() displaying erase info size breaks progress messages.
> Fix this by moving relevant DEBUG_MSG() to write_elf_eraseinfo().
> 
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Good, I'll merge this patch into v1.5.5.


Thanks
Atsushi Kumagai

> ---
>  makedumpfile.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index b4abbe5..a3bfc80 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -5045,7 +5045,6 @@ write_elf_header(struct cache_data *cd_header)
>  	 * function.
>  	 */
>  	info->size_elf_eraseinfo = size_eraseinfo;
> -	DEBUG_MSG("erase info size: %lu\n", info->size_elf_eraseinfo);
>  
>  	/*
>  	 * Write a PT_NOTE header.
> @@ -6326,6 +6325,8 @@ write_elf_eraseinfo(struct cache_data *cd_header)
>  	off_t offset_eraseinfo;
>  	unsigned long note_header_size, size_written, size_note;
>  
> +	DEBUG_MSG("erase info size: %lu\n", info->size_elf_eraseinfo);
> +
>  	if (!info->size_elf_eraseinfo)
>  		return TRUE;
>  
> -- 
> 1.7.10.4

[PATCH v2 6/7] Mute some compiler warnings

[Daniel Kiper]

This patch mutes follwing compiler warnings:
  - warning: assignment discards ‘const’ qualifier from
    pointer target type [enabled by default],
  - warning: variable ‘page_offset’ set but not used
    [-Wunused-but-set-variable].

v2 - suggestions/fixes:
   - do not discard a const qualifier
     (suggested by Andrew Cooper).

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 dwarf_info.c  |   14 ++++++++------
 sadump_info.c |    2 --
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/dwarf_info.c b/dwarf_info.c
index 6e21b8a..85c116e 100644
--- a/dwarf_info.c
+++ b/dwarf_info.c
@@ -1381,6 +1381,7 @@ int
 get_die_member(unsigned long long die_off, int index, long *offset,
 		char **name, int *nbits, int *fbits, unsigned long long *m_die)
 {
+	const char *diename;
 	int tag, size, nfields = 0;
 	Dwarf_Die result, child, die_base, *die;
 
@@ -1427,14 +1428,14 @@ get_die_member(unsigned long long die_off, int index, long *offset,
 	if (!get_data_member_location(die, offset))
 		*offset = 0;
 
-	*name = dwarf_diename(die);
+	diename = dwarf_diename(die);
 	/*
 	 * Duplicate the string before we pass it to eppic layer. The
 	 * original string returned by dwarf layer will become invalid
 	 * after clean_dwfl_info() call.
 	 */
-	if (*name)
-		*name = strdup(*name);
+	if (diename)
+		*name = strdup(diename);
 	*m_die = dwarf_dieoffset(die);
 
 	get_die_type(die, &die_base);
@@ -1504,6 +1505,7 @@ get_die_name(unsigned long long die_off)
 {
 	Dwarf_Die result;
 	char *name = NULL;
+	const char *diename;
 
 	if (!die_off)
 		return NULL;
@@ -1513,9 +1515,9 @@ get_die_name(unsigned long long die_off)
 		return NULL;
 	}
 
-	name = dwarf_diename(&result);
-	if (name)
-		name = strdup(name);
+	diename = dwarf_diename(&result);
+	if (diename)
+		name = strdup(diename);
 	clean_dwfl_info();
 	return name;
 }
diff --git a/sadump_info.c b/sadump_info.c
index be6cf55..01cf5eb 100644
--- a/sadump_info.c
+++ b/sadump_info.c
@@ -948,7 +948,6 @@ int
 readpage_sadump(unsigned long long paddr, void *bufptr)
 {
 	unsigned long long pfn, block, whole_offset, perdisk_offset;
-	ulong page_offset;
 	int fd_memory;
 
 	if (si->kdump_backed_up &&
@@ -957,7 +956,6 @@ readpage_sadump(unsigned long long paddr, void *bufptr)
 		paddr += si->backup_offset - si->backup_src_start;
 
 	pfn = paddr_to_pfn(paddr);
-	page_offset = paddr % info->page_size;
 
 	if (pfn >= si->sh_memory->max_mapnr)
 		return FALSE;
-- 
1.7.10.4


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2 6/7] Mute some compiler warnings

[HATAYAMA Daisuke]

(2013/07/16 21:32), Daniel Kiper wrote:
<cut>
> diff --git a/sadump_info.c b/sadump_info.c
> index be6cf55..01cf5eb 100644
> --- a/sadump_info.c
> +++ b/sadump_info.c
> @@ -948,7 +948,6 @@ int
>   readpage_sadump(unsigned long long paddr, void *bufptr)
>   {
>   	unsigned long long pfn, block, whole_offset, perdisk_offset;
> -	ulong page_offset;
>   	int fd_memory;
>
>   	if (si->kdump_backed_up &&
> @@ -957,7 +956,6 @@ readpage_sadump(unsigned long long paddr, void *bufptr)
>   		paddr += si->backup_offset - si->backup_src_start;
>
>   	pfn = paddr_to_pfn(paddr);
> -	page_offset = paddr % info->page_size;
>
>   	if (pfn >= si->sh_memory->max_mapnr)
>   		return FALSE;
>

It seems good to me. Thanks.

-- 
Thanks.
HATAYAMA, Daisuke

Re: [PATCH v2 6/7] Mute some compiler warnings

[Atsushi Kumagai]

On Tue, 16 Jul 2013 14:32:08 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> This patch mutes follwing compiler warnings:
>   - warning: assignment discards ‘const’ qualifier from
>     pointer target type [enabled by default],
>   - warning: variable ‘page_offset’ set but not used
>     [-Wunused-but-set-variable].
> 
> v2 - suggestions/fixes:
>    - do not discard a const qualifier
>      (suggested by Andrew Cooper).
> 
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Good, I'll merge this patch into v1.5.5.
And thanks for your review, HATAYAMA-san.


Thanks
Atsushi Kumagai

On Fri, 19 Jul 2013 09:03:49 +0900
HATAYAMA Daisuke <d.hatayama-+CUm20s59erQFUHtdCDX3A@public.gmane.org> wrote:

> (2013/07/16 21:32), Daniel Kiper wrote:
> <cut>
> > diff --git a/sadump_info.c b/sadump_info.c
> > index be6cf55..01cf5eb 100644
> > --- a/sadump_info.c
> > +++ b/sadump_info.c
> > @@ -948,7 +948,6 @@ int
> >   readpage_sadump(unsigned long long paddr, void *bufptr)
> >   {
> >   	unsigned long long pfn, block, whole_offset, perdisk_offset;
> > -	ulong page_offset;
> >   	int fd_memory;
> >
> >   	if (si->kdump_backed_up &&
> > @@ -957,7 +956,6 @@ readpage_sadump(unsigned long long paddr, void *bufptr)
> >   		paddr += si->backup_offset - si->backup_src_start;
> >
> >   	pfn = paddr_to_pfn(paddr);
> > -	page_offset = paddr % info->page_size;
> >
> >   	if (pfn >= si->sh_memory->max_mapnr)
> >   		return FALSE;
> >
> 
> It seems good to me. Thanks.

[PATCH v2 7/7] Use elf_getshdrstrndx() instead of elf_getshstrndx()

[Daniel Kiper]

Use elf_getshdrstrndx() instead of elf_getshstrndx()
because later is marked as deprecated.

Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
 dwarf_info.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dwarf_info.c b/dwarf_info.c
index 85c116e..c5752c9 100644
--- a/dwarf_info.c
+++ b/dwarf_info.c
@@ -944,7 +944,7 @@ get_debug_info(void)
 	elfd = dwarf_info.elfd;
 	dwarfd = dwarf_info.dwarfd;
 
-	if (elf_getshstrndx(elfd, &shstrndx) < 0) {
+	if (elf_getshdrstrndx(elfd, &shstrndx) < 0) {
 		ERRMSG("Can't get the section index of the string table.\n");
 		goto out;
 	}
-- 
1.7.10.4

Re: [PATCH v2 7/7] Use elf_getshdrstrndx() instead of elf_getshstrndx()

[Atsushi Kumagai]

On Tue, 16 Jul 2013 14:32:09 +0200
Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:

> Use elf_getshdrstrndx() instead of elf_getshstrndx()
> because later is marked as deprecated.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Good, I'll merge this patch into v1.5.5.


Thanks
Atsushi Kumagai

> ---
>  dwarf_info.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/dwarf_info.c b/dwarf_info.c
> index 85c116e..c5752c9 100644
> --- a/dwarf_info.c
> +++ b/dwarf_info.c
> @@ -944,7 +944,7 @@ get_debug_info(void)
>  	elfd = dwarf_info.elfd;
>  	dwarfd = dwarf_info.dwarfd;
>  
> -	if (elf_getshstrndx(elfd, &shstrndx) < 0) {
> +	if (elf_getshdrstrndx(elfd, &shstrndx) < 0) {
>  		ERRMSG("Can't get the section index of the string table.\n");
>  		goto out;
>  	}
> -- 
> 1.7.10.4
> 

Re: [PATCH v2 7/7] Use elf_getshdrstrndx() instead of elf_getshstrndx()

[Daniel Kiper]

On Thu, Jul 18, 2013 at 05:03:22PM +0900, Atsushi Kumagai wrote:
> On Tue, 16 Jul 2013 14:32:09 +0200
> Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
>
> > Use elf_getshdrstrndx() instead of elf_getshstrndx()
> > because later is marked as deprecated.
> >
> > Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>
> Good, I'll merge this patch into v1.5.5.

Thanks. What about patches 4, 5 and 6.
Do you have any comments about them?
Should I fix something?

Daniel