All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
@ 2014-03-25 16:14 Michael Holzheu
  2014-03-26  9:55 ` HATAYAMA Daisuke
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Holzheu @ 2014-03-25 16:14 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: d.hatayama, kexec

There are dump mechansims like s390 stand-alond dump or KVM virsh dump
that write the physical memory of a machine and are not aware of the
dumped operating system. For those dump mechanisms it can happen
that for the Linux kernel of the dumped system the "mem=" kernel
parameter has been specified. In this case max_mapnr that makedumpfile
gets from the ELF header can be bigger than the maximum page frame number
used by the dumped Linux kernel.

With this patch makedumpfile gets the maximum page frame number from
the mem_map array and adjusts info->max_mapnr if this value is smaller
than the value calculated from the ELF header.

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 makedumpfile.c |   14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2829,7 +2829,8 @@ get_mem_map_without_mm(void)
 int
 get_mem_map(void)
 {
-	int ret;
+	unsigned long max_pfn = 0;
+	int ret, i;
 
 	switch (get_mem_type()) {
 	case SPARSEMEM:
@@ -2861,6 +2862,17 @@ get_mem_map(void)
 		ret = FALSE;
 		break;
 	}
+	/*
+	 * Adjust "max_mapnr" for the case that Linux uses less memory
+	 * than is dumped. For example when "mem=" has been used for the
+	 * dumped system.
+	 */
+	for (i = 0; i < info->num_mem_map; i++) {
+		if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
+			continue;
+		max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
+	}
+	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
 	return ret;
 }
 


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-25 16:14 [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array Michael Holzheu
@ 2014-03-26  9:55 ` HATAYAMA Daisuke
  2014-03-26 17:54   ` Michael Holzheu
  0 siblings, 1 reply; 17+ messages in thread
From: HATAYAMA Daisuke @ 2014-03-26  9:55 UTC (permalink / raw)
  To: holzheu; +Cc: kumagai-atsushi, kexec

From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
Subject: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
Date: Tue, 25 Mar 2014 17:14:20 +0100

> There are dump mechansims like s390 stand-alond dump or KVM virsh dump
> that write the physical memory of a machine and are not aware of the
> dumped operating system. For those dump mechanisms it can happen
> that for the Linux kernel of the dumped system the "mem=" kernel
> parameter has been specified. In this case max_mapnr that makedumpfile
> gets from the ELF header can be bigger than the maximum page frame number
> used by the dumped Linux kernel.
> 
> With this patch makedumpfile gets the maximum page frame number from
> the mem_map array and adjusts info->max_mapnr if this value is smaller
> than the value calculated from the ELF header.
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---
>  makedumpfile.c |   14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -2829,7 +2829,8 @@ get_mem_map_without_mm(void)
>  int
>  get_mem_map(void)
>  {
> -	int ret;
> +	unsigned long max_pfn = 0;
> +	int ret, i;

Please define max_pfn as unsigned long long.

And for i,

>  
>  	switch (get_mem_type()) {
>  	case SPARSEMEM:
> @@ -2861,6 +2862,17 @@ get_mem_map(void)
>  		ret = FALSE;
>  		break;
>  	}
> +	/*
> +	 * Adjust "max_mapnr" for the case that Linux uses less memory
> +	 * than is dumped. For example when "mem=" has been used for the
> +	 * dumped system.
> +	 */
> +	for (i = 0; i < info->num_mem_map; i++) {

info->num_mem_map is defined as unsigned int. I guess some warning
about comparison with different signedness occurs.

> +		if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
> +			continue;
> +		max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
> +	}
> +	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
>  	return ret;
>  }
>  
> 

Thanks.
HATAYAMA, Daisuke


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-26  9:55 ` HATAYAMA Daisuke
@ 2014-03-26 17:54   ` Michael Holzheu
  2014-03-27  5:19     ` Atsushi Kumagai
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Holzheu @ 2014-03-26 17:54 UTC (permalink / raw)
  To: HATAYAMA Daisuke; +Cc: kexec, kumagai-atsushi

On Wed, 26 Mar 2014 10:55:07 +0100 (a/T)
HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:

> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> Subject: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
> Date: Tue, 25 Mar 2014 17:14:20 +0100

[snip]

> > With this patch makedumpfile gets the maximum page frame number from
> > the mem_map array and adjusts info->max_mapnr if this value is smaller
> > than the value calculated from the ELF header.
> > 
> > Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> > ---
> >  makedumpfile.c |   14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > --- a/makedumpfile.c
> > +++ b/makedumpfile.c
> > @@ -2829,7 +2829,8 @@ get_mem_map_without_mm(void)
> >  int
> >  get_mem_map(void)
> >  {
> > -	int ret;
> > +	unsigned long max_pfn = 0;
> > +	int ret, i;
> 
> Please define max_pfn as unsigned long long.

Ok done.

> 
> And for i,
> 
> >  
> >  	switch (get_mem_type()) {
> >  	case SPARSEMEM:
> > @@ -2861,6 +2862,17 @@ get_mem_map(void)
> >  		ret = FALSE;
> >  		break;
> >  	}
> > +	/*
> > +	 * Adjust "max_mapnr" for the case that Linux uses less memory
> > +	 * than is dumped. For example when "mem=" has been used for the
> > +	 * dumped system.
> > +	 */
> > +	for (i = 0; i < info->num_mem_map; i++) {
> 
> info->num_mem_map is defined as unsigned int. I guess some warning
> about comparison with different signedness occurs.

Ah ok...

With the default CFLAGS for makedumpfile-1.5.5 tarball I do not get
any warning. When I add "-W" to CFLAGS, I get lots of warnings
including the one you mentioned.

Here the fixed patch:
---
[PATCH 2/2] makedumpfile: Use max_pfn from mem_map array

There are dump mechansims like s390 stand-alond dump or KVM virsh dump
that write the physical memory of a machine and are not aware of the
dumped operating system. For those dump mechanisms it can happen
that for the Linux kernel of the dumped system the "mem=" kernel
parameter has been specified. In this case max_mapnr that makedumpfile
gets from the ELF header can be bigger than the maximum page frame number
used by the dumped Linux kernel.

With this patch makedumpfile gets the maximum page frame number from
the mem_map array and adjusts info->max_mapnr if this value is smaller
than the value calculated from the ELF header.

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 makedumpfile.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2829,6 +2829,8 @@ get_mem_map_without_mm(void)
 int
 get_mem_map(void)
 {
+	unsigned long long max_pfn = 0;
+	unsigned int i;
 	int ret;
 
 	switch (get_mem_type()) {
@@ -2861,6 +2863,17 @@ get_mem_map(void)
 		ret = FALSE;
 		break;
 	}
+	/*
+	 * Adjust "max_mapnr" for the case that Linux uses less memory
+	 * than is dumped. For example when "mem=" has been used for the
+	 * dumped system.
+	 */
+	for (i = 0; i < info->num_mem_map; i++) {
+		if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
+			continue;
+		max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
+	}
+	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
 	return ret;
 }
 


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-26 17:54   ` Michael Holzheu
@ 2014-03-27  5:19     ` Atsushi Kumagai
  2014-03-27 13:54       ` Michael Holzheu
  0 siblings, 1 reply; 17+ messages in thread
From: Atsushi Kumagai @ 2014-03-27  5:19 UTC (permalink / raw)
  To: holzheu; +Cc: d.hatayama, kexec

Hello Michael,

>On Wed, 26 Mar 2014 10:55:07 +0100 (a/T)
>HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:
>
>> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
>> Subject: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
>> Date: Tue, 25 Mar 2014 17:14:20 +0100
>
>[snip]
>
>> > With this patch makedumpfile gets the maximum page frame number from
>> > the mem_map array and adjusts info->max_mapnr if this value is smaller
>> > than the value calculated from the ELF header.
>> >
>> > Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
>> > ---
>> >  makedumpfile.c |   14 +++++++++++++-
>> >  1 file changed, 13 insertions(+), 1 deletion(-)
>> >
>> > --- a/makedumpfile.c
>> > +++ b/makedumpfile.c
>> > @@ -2829,7 +2829,8 @@ get_mem_map_without_mm(void)
>> >  int
>> >  get_mem_map(void)
>> >  {
>> > -	int ret;
>> > +	unsigned long max_pfn = 0;
>> > +	int ret, i;
>>
>> Please define max_pfn as unsigned long long.
>
>Ok done.
>
>>
>> And for i,
>>
>> >
>> >  	switch (get_mem_type()) {
>> >  	case SPARSEMEM:
>> > @@ -2861,6 +2862,17 @@ get_mem_map(void)
>> >  		ret = FALSE;
>> >  		break;
>> >  	}
>> > +	/*
>> > +	 * Adjust "max_mapnr" for the case that Linux uses less memory
>> > +	 * than is dumped. For example when "mem=" has been used for the
>> > +	 * dumped system.
>> > +	 */
>> > +	for (i = 0; i < info->num_mem_map; i++) {
>>
>> info->num_mem_map is defined as unsigned int. I guess some warning
>> about comparison with different signedness occurs.
>
>Ah ok...
>
>With the default CFLAGS for makedumpfile-1.5.5 tarball I do not get
>any warning. When I add "-W" to CFLAGS, I get lots of warnings
>including the one you mentioned.
>
>Here the fixed patch:

Thanks, I'll merge the fixed version into v1.5.6.


Atsushi Kumagai

>---
>[PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
>
>There are dump mechansims like s390 stand-alond dump or KVM virsh dump
>that write the physical memory of a machine and are not aware of the
>dumped operating system. For those dump mechanisms it can happen
>that for the Linux kernel of the dumped system the "mem=" kernel
>parameter has been specified. In this case max_mapnr that makedumpfile
>gets from the ELF header can be bigger than the maximum page frame number
>used by the dumped Linux kernel.
>
>With this patch makedumpfile gets the maximum page frame number from
>the mem_map array and adjusts info->max_mapnr if this value is smaller
>than the value calculated from the ELF header.
>
>Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
>---
> makedumpfile.c |   13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -2829,6 +2829,8 @@ get_mem_map_without_mm(void)
> int
> get_mem_map(void)
> {
>+	unsigned long long max_pfn = 0;
>+	unsigned int i;
> 	int ret;
>
> 	switch (get_mem_type()) {
>@@ -2861,6 +2863,17 @@ get_mem_map(void)
> 		ret = FALSE;
> 		break;
> 	}
>+	/*
>+	 * Adjust "max_mapnr" for the case that Linux uses less memory
>+	 * than is dumped. For example when "mem=" has been used for the
>+	 * dumped system.
>+	 */
>+	for (i = 0; i < info->num_mem_map; i++) {
>+		if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
>+			continue;
>+		max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
>+	}
>+	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
> 	return ret;
> }
>
>
>
>_______________________________________________
>kexec mailing list
>kexec@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/kexec

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-27  5:19     ` Atsushi Kumagai
@ 2014-03-27 13:54       ` Michael Holzheu
  2014-03-28 11:00         ` Petr Tesarik
  0 siblings, 1 reply; 17+ messages in thread
From: Michael Holzheu @ 2014-03-27 13:54 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: d.hatayama, kexec

On Thu, 27 Mar 2014 05:19:06 +0000
Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:

> Hello Michael,
> 
> >On Wed, 26 Mar 2014 10:55:07 +0100 (a/T)
> >HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:
> >
> >> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> >> Subject: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
> >> Date: Tue, 25 Mar 2014 17:14:20 +0100
> >
> >[snip]
> >
> >> > With this patch makedumpfile gets the maximum page frame number from
> >> > the mem_map array and adjusts info->max_mapnr if this value is smaller
> >> > than the value calculated from the ELF header.
> >> >
> >> > Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> >> > ---
> >> >  makedumpfile.c |   14 +++++++++++++-
> >> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >> >
> >> > --- a/makedumpfile.c
> >> > +++ b/makedumpfile.c
> >> > @@ -2829,7 +2829,8 @@ get_mem_map_without_mm(void)
> >> >  int
> >> >  get_mem_map(void)
> >> >  {
> >> > -	int ret;
> >> > +	unsigned long max_pfn = 0;
> >> > +	int ret, i;
> >>
> >> Please define max_pfn as unsigned long long.
> >
> >Ok done.
> >
> >>
> >> And for i,
> >>
> >> >
> >> >  	switch (get_mem_type()) {
> >> >  	case SPARSEMEM:
> >> > @@ -2861,6 +2862,17 @@ get_mem_map(void)
> >> >  		ret = FALSE;
> >> >  		break;
> >> >  	}
> >> > +	/*
> >> > +	 * Adjust "max_mapnr" for the case that Linux uses less memory
> >> > +	 * than is dumped. For example when "mem=" has been used for the
> >> > +	 * dumped system.
> >> > +	 */
> >> > +	for (i = 0; i < info->num_mem_map; i++) {
> >>
> >> info->num_mem_map is defined as unsigned int. I guess some warning
> >> about comparison with different signedness occurs.
> >
> >Ah ok...
> >
> >With the default CFLAGS for makedumpfile-1.5.5 tarball I do not get
> >any warning. When I add "-W" to CFLAGS, I get lots of warnings
> >including the one you mentioned.
> >
> >Here the fixed patch:
> 
> Thanks, I'll merge the fixed version into v1.5.6.

Great!

Thanks for your support!

Michael


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-27 13:54       ` Michael Holzheu
@ 2014-03-28 11:00         ` Petr Tesarik
  2014-03-28 15:54           ` Michael Holzheu
  2014-03-28 16:46           ` Michael Holzheu
  0 siblings, 2 replies; 17+ messages in thread
From: Petr Tesarik @ 2014-03-28 11:00 UTC (permalink / raw)
  To: Michael Holzheu; +Cc: kexec, d.hatayama, Atsushi Kumagai

On Thu, 27 Mar 2014 14:54:41 +0100
Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

> On Thu, 27 Mar 2014 05:19:06 +0000
> Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:
> 
> > Hello Michael,
> > 
> > >On Wed, 26 Mar 2014 10:55:07 +0100 (a/T)
> > >HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:
> > >
> > >> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> > >> Subject: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
> > >> Date: Tue, 25 Mar 2014 17:14:20 +0100
> > >
> > >[snip]
> > >
> > >> > With this patch makedumpfile gets the maximum page frame number from
> > >> > the mem_map array and adjusts info->max_mapnr if this value is smaller
> > >> > than the value calculated from the ELF header.
> > >> >
> > >> > Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> > >> > ---
> > >> >  makedumpfile.c |   14 +++++++++++++-
> > >> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > >> >
> > >> > --- a/makedumpfile.c
> > >> > +++ b/makedumpfile.c
> > >> > @@ -2829,7 +2829,8 @@ get_mem_map_without_mm(void)
> > >> >  int
> > >> >  get_mem_map(void)
> > >> >  {
> > >> > -	int ret;
> > >> > +	unsigned long max_pfn = 0;
> > >> > +	int ret, i;
> > >>
> > >> Please define max_pfn as unsigned long long.
> > >
> > >Ok done.
> > >
> > >>
> > >> And for i,
> > >>
> > >> >
> > >> >  	switch (get_mem_type()) {
> > >> >  	case SPARSEMEM:
> > >> > @@ -2861,6 +2862,17 @@ get_mem_map(void)
> > >> >  		ret = FALSE;
> > >> >  		break;
> > >> >  	}
> > >> > +	/*
> > >> > +	 * Adjust "max_mapnr" for the case that Linux uses less memory
> > >> > +	 * than is dumped. For example when "mem=" has been used for the
> > >> > +	 * dumped system.
> > >> > +	 */
> > >> > +	for (i = 0; i < info->num_mem_map; i++) {
> > >>
> > >> info->num_mem_map is defined as unsigned int. I guess some warning
> > >> about comparison with different signedness occurs.
> > >
> > >Ah ok...
> > >
> > >With the default CFLAGS for makedumpfile-1.5.5 tarball I do not get
> > >any warning. When I add "-W" to CFLAGS, I get lots of warnings
> > >including the one you mentioned.
> > >
> > >Here the fixed patch:
> > 
> > Thanks, I'll merge the fixed version into v1.5.6.
> 
> Great!

I'm sorry to spoil the party, but this patch broke Xen dumps for me.
I'm getting an long series of these messages:

set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
...

In fact, it most likely broke all non-cyclic dumps.

That's because the bitmap length is calculated in prepare_bitmap_buffer
using info->max_mapnr, but create_1st_bitmap() still loops over all
PT_LOAD segments, calling set_bit_on_1st_bitmap() for each PFN. The
offset may easily fall beyond the bitmap size.

Petr T

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-28 11:00         ` Petr Tesarik
@ 2014-03-28 15:54           ` Michael Holzheu
  2014-03-28 16:46           ` Michael Holzheu
  1 sibling, 0 replies; 17+ messages in thread
From: Michael Holzheu @ 2014-03-28 15:54 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: kexec, d.hatayama, Atsushi Kumagai

On Fri, 28 Mar 2014 12:00:47 +0100
Petr Tesarik <ptesarik@suse.cz> wrote:

> On Thu, 27 Mar 2014 14:54:41 +0100
> Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:
> 
> > On Thu, 27 Mar 2014 05:19:06 +0000
> > Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:
> > 
> > > Hello Michael,
> > > 
> > > >On Wed, 26 Mar 2014 10:55:07 +0100 (a/T)
> > > >HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com> wrote:
> > > >
> > > >> From: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> > > >> Subject: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
> > > >> Date: Tue, 25 Mar 2014 17:14:20 +0100
> > > >
> > > >[snip]
> > > >
> > > >> > With this patch makedumpfile gets the maximum page frame number from
> > > >> > the mem_map array and adjusts info->max_mapnr if this value is smaller
> > > >> > than the value calculated from the ELF header.
> > > >> >
> > > >> > Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> > > >> > ---
> > > >> >  makedumpfile.c |   14 +++++++++++++-
> > > >> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > > >> >
> > > >> > --- a/makedumpfile.c
> > > >> > +++ b/makedumpfile.c
> > > >> > @@ -2829,7 +2829,8 @@ get_mem_map_without_mm(void)
> > > >> >  int
> > > >> >  get_mem_map(void)
> > > >> >  {
> > > >> > -	int ret;
> > > >> > +	unsigned long max_pfn = 0;
> > > >> > +	int ret, i;
> > > >>
> > > >> Please define max_pfn as unsigned long long.
> > > >
> > > >Ok done.
> > > >
> > > >>
> > > >> And for i,
> > > >>
> > > >> >
> > > >> >  	switch (get_mem_type()) {
> > > >> >  	case SPARSEMEM:
> > > >> > @@ -2861,6 +2862,17 @@ get_mem_map(void)
> > > >> >  		ret = FALSE;
> > > >> >  		break;
> > > >> >  	}
> > > >> > +	/*
> > > >> > +	 * Adjust "max_mapnr" for the case that Linux uses less memory
> > > >> > +	 * than is dumped. For example when "mem=" has been used for the
> > > >> > +	 * dumped system.
> > > >> > +	 */
> > > >> > +	for (i = 0; i < info->num_mem_map; i++) {
> > > >>
> > > >> info->num_mem_map is defined as unsigned int. I guess some warning
> > > >> about comparison with different signedness occurs.
> > > >
> > > >Ah ok...
> > > >
> > > >With the default CFLAGS for makedumpfile-1.5.5 tarball I do not get
> > > >any warning. When I add "-W" to CFLAGS, I get lots of warnings
> > > >including the one you mentioned.
> > > >
> > > >Here the fixed patch:
> > > 
> > > Thanks, I'll merge the fixed version into v1.5.6.
> > 
> > Great!
> 
> I'm sorry to spoil the party, but this patch broke Xen dumps for me.
> I'm getting an long series of these messages:
> 
> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> ...
> 
> In fact, it most likely broke all non-cyclic dumps.
> 
> That's because the bitmap length is calculated in prepare_bitmap_buffer
> using info->max_mapnr, but create_1st_bitmap() still loops over all
> PT_LOAD segments, calling set_bit_on_1st_bitmap() for each PFN. The
> offset may easily fall beyond the bitmap size.

Hello Petr,

I can reproduce your issue with the --non-cyclic option and I will look
into this.

Thanks for testing this!
Michael


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-28 11:00         ` Petr Tesarik
  2014-03-28 15:54           ` Michael Holzheu
@ 2014-03-28 16:46           ` Michael Holzheu
  2014-03-28 16:53             ` Petr Tesarik
  2014-03-31 10:27             ` Petr Tesarik
  1 sibling, 2 replies; 17+ messages in thread
From: Michael Holzheu @ 2014-03-28 16:46 UTC (permalink / raw)
  To: Petr Tesarik; +Cc: kexec, d.hatayama, Atsushi Kumagai

On Fri, 28 Mar 2014 12:00:47 +0100
Petr Tesarik <ptesarik@suse.cz> wrote:

> On Thu, 27 Mar 2014 14:54:41 +0100
> Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

[snip]

> > > >Here the fixed patch:
> > > 
> > > Thanks, I'll merge the fixed version into v1.5.6.
> > 
> > Great!
> 
> I'm sorry to spoil the party, but this patch broke Xen dumps for me.
> I'm getting an long series of these messages:
> 
> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> ...
> 
> In fact, it most likely broke all non-cyclic dumps.
> 
> That's because the bitmap length is calculated in prepare_bitmap_buffer
> using info->max_mapnr, but create_1st_bitmap() still loops over all
> PT_LOAD segments, calling set_bit_on_1st_bitmap() for each PFN. The
> offset may easily fall beyond the bitmap size.

What about the following patch. It works for me when I specify
the "--non-cyclic" option.

Michael
---
[PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr

If info->max_mapnr has been adjusted, for example because the dumped
system has specified the "mem=" kernel parameter, makedumpfile writes
the following error messages for Xen dumps or when the "--non-cyclic"
option has been specified:

set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap96s9V8). Success

Fix this and consider "info->max_mapnr" in the create bitmap functions.

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 makedumpfile.c |    9 +++++++++
 1 file changed, 9 insertions(+)

--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -4402,6 +4402,9 @@ create_1st_bitmap(void)
 
 		pfn_start = paddr_to_pfn(phys_start);
 		pfn_end   = paddr_to_pfn(phys_end);
+		if (pfn_start > info->max_mapnr)
+			continue;
+		pfn_end = MIN(phys_end, info->max_mapnr);
 
 		for (pfn = pfn_start; pfn < pfn_end; pfn++) {
 			set_bit_on_1st_bitmap(pfn);
@@ -7511,6 +7514,9 @@ exclude_xen3_user_domain(void)
 		pfn     = paddr_to_pfn(phys_start);
 		pfn_end = paddr_to_pfn(phys_end);
 		size    = pfn_end - pfn;
+		if (pfn > info->max_mapnr)
+			continue;
+		pfn_end = MIN(phys_end, info->max_mapnr);
 
 		for (j = 0; pfn < pfn_end; pfn++, j++) {
 			print_progress(PROGRESS_XEN_DOMAIN, j + (size * i),
@@ -7575,6 +7581,9 @@ exclude_xen4_user_domain(void)
 		pfn     = paddr_to_pfn(phys_start);
 		pfn_end = paddr_to_pfn(phys_end);
 		size    = pfn_end - pfn;
+		if (pfn > info->max_mapnr)
+			continue;
+		pfn_end = MIN(phys_end, info->max_mapnr);
 
 		for (j = 0; pfn < pfn_end; pfn++, j++) {
 			print_progress(PROGRESS_XEN_DOMAIN, j + (size * i),


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-28 16:46           ` Michael Holzheu
@ 2014-03-28 16:53             ` Petr Tesarik
  2014-03-31  9:48               ` Atsushi Kumagai
  2014-03-31 10:27             ` Petr Tesarik
  1 sibling, 1 reply; 17+ messages in thread
From: Petr Tesarik @ 2014-03-28 16:53 UTC (permalink / raw)
  To: Michael Holzheu; +Cc: Atsushi Kumagai, d.hatayama, kexec

On Fri, 28 Mar 2014 17:46:22 +0100
Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

> On Fri, 28 Mar 2014 12:00:47 +0100
> Petr Tesarik <ptesarik@suse.cz> wrote:
> 

[snip]

> > That's because the bitmap length is calculated in prepare_bitmap_buffer
> > using info->max_mapnr, but create_1st_bitmap() still loops over all
> > PT_LOAD segments, calling set_bit_on_1st_bitmap() for each PFN. The
> > offset may easily fall beyond the bitmap size.
> 
> What about the following patch. It works for me when I specify
> the "--non-cyclic" option.
> 
> Michael
> ---
> [PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr
> 
> If info->max_mapnr has been adjusted, for example because the dumped
> system has specified the "mem=" kernel parameter, makedumpfile writes
> the following error messages for Xen dumps or when the "--non-cyclic"
> option has been specified:
> 
> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap96s9V8). Success
> 
> Fix this and consider "info->max_mapnr" in the create bitmap functions.
> 
> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> ---

[snip]

> @@ -7575,6 +7581,9 @@ exclude_xen4_user_domain(void)
>  		pfn     = paddr_to_pfn(phys_start);
>  		pfn_end = paddr_to_pfn(phys_end);
>  		size    = pfn_end - pfn;
> +		if (pfn > info->max_mapnr)
> +			continue;
> +		pfn_end = MIN(phys_end, info->max_mapnr);

Hmm, probably time for weekend. Of course this should be:

pfn_end = MIN(pfn_end, info->max_mapnr);

Here the updated patch:
---
[PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr

If info->max_mapnr has been adjusted, for example because the dumped
system has specified the "mem=" kernel parameter, makedumpfile writes
the following error messages for Xen dumps or when the "--non-cyclic"
option has been specified:

set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap96s9V8). Success

Fix this and consider "info->max_mapnr" in the create bitmap functions.

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 makedumpfile.c |    9 +++++++++
 1 file changed, 9 insertions(+)

--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -4402,6 +4402,9 @@ create_1st_bitmap(void)
 
 		pfn_start = paddr_to_pfn(phys_start);
 		pfn_end   = paddr_to_pfn(phys_end);
+		if (pfn_start > info->max_mapnr)
+			continue;
+		pfn_end = MIN(pfn_end, info->max_mapnr);
 
 		for (pfn = pfn_start; pfn < pfn_end; pfn++) {
 			set_bit_on_1st_bitmap(pfn);
@@ -7511,6 +7514,9 @@ exclude_xen3_user_domain(void)
 		pfn     = paddr_to_pfn(phys_start);
 		pfn_end = paddr_to_pfn(phys_end);
 		size    = pfn_end - pfn;
+		if (pfn > info->max_mapnr)
+			continue;
+		pfn_end = MIN(pfn_end, info->max_mapnr);
 
 		for (j = 0; pfn < pfn_end; pfn++, j++) {
 			print_progress(PROGRESS_XEN_DOMAIN, j + (size * i),
@@ -7575,6 +7581,9 @@ exclude_xen4_user_domain(void)
 		pfn     = paddr_to_pfn(phys_start);
 		pfn_end = paddr_to_pfn(phys_end);
 		size    = pfn_end - pfn;
+		if (pfn > info->max_mapnr)
+			continue;
+		pfn_end = MIN(pfn_end, info->max_mapnr);
 
 		for (j = 0; pfn < pfn_end; pfn++, j++) {
 			print_progress(PROGRESS_XEN_DOMAIN, j + (size * i),


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-28 16:53             ` Petr Tesarik
@ 2014-03-31  9:48               ` Atsushi Kumagai
  2014-03-31 10:37                 ` Petr Tesarik
  2014-03-31 12:59                 ` Michael Holzheu
  0 siblings, 2 replies; 17+ messages in thread
From: Atsushi Kumagai @ 2014-03-31  9:48 UTC (permalink / raw)
  To: ptesarik, holzheu; +Cc: d.hatayama, kexec

[snip]

>> > That's because the bitmap length is calculated in prepare_bitmap_buffer
>> > using info->max_mapnr, but create_1st_bitmap() still loops over all
>> > PT_LOAD segments, calling set_bit_on_1st_bitmap() for each PFN. The
>> > offset may easily fall beyond the bitmap size.
>>
>> What about the following patch. It works for me when I specify
>> the "--non-cyclic" option.
>>
>> Michael
>> ---
>> [PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr
>>
>> If info->max_mapnr has been adjusted, for example because the dumped
>> system has specified the "mem=" kernel parameter, makedumpfile writes
>> the following error messages for Xen dumps or when the "--non-cyclic"
>> option has been specified:
>>
>> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap96s9V8). Success
                                                               ^^^^^^^^
This looks confusing, is it an actual message ?
I suppose it must be "Invalid argument" like Petr's log.

>>
>> Fix this and consider "info->max_mapnr" in the create bitmap functions.
>>
>> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
>> ---
>
>[snip]
>
>> @@ -7575,6 +7581,9 @@ exclude_xen4_user_domain(void)
>>  		pfn     = paddr_to_pfn(phys_start);
>>  		pfn_end = paddr_to_pfn(phys_end);
>>  		size    = pfn_end - pfn;
>> +		if (pfn > info->max_mapnr)
>> +			continue;
>> +		pfn_end = MIN(phys_end, info->max_mapnr);
>
>Hmm, probably time for weekend. Of course this should be:
>
>pfn_end = MIN(pfn_end, info->max_mapnr);
>
>Here the updated patch:

I found another issue of truncating max_mapnr for Xen.

The bitmap manages MFN(machine frame number) for Xen
while __exclude_unnecessary_pages() treats PFN(guest-physical frame number).
__exclude_unnecessary_pages() expects that all bits of PFNs
are mapped in the bitmap even if it was reduced by truncated 
max_mapnr. However, PtoM mapping isn't linear(probably...),
there is no guarantee that a set of continuous PFNs is mapped
in a set of continuous MFNs.
So the actual I/O offset can exceed the bitmap size when the
bitmap size is reduced.

In the first place, we shouldn't truncate max_mapnr
based on dom0's mem_section since there are some domU's
memories on Xen dumps. Now, I think a better way for Xen
is just leaving max_mapnr as it is.

Do you agree with my view ?


Thanks
Atsushi Kumagai

>---
>[PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr
>
>If info->max_mapnr has been adjusted, for example because the dumped
>system has specified the "mem=" kernel parameter, makedumpfile writes
>the following error messages for Xen dumps or when the "--non-cyclic"
>option has been specified:
>
>set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap96s9V8). Success
>
>Fix this and consider "info->max_mapnr" in the create bitmap functions.
>
>Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
>---
> makedumpfile.c |    9 +++++++++
> 1 file changed, 9 insertions(+)
>
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -4402,6 +4402,9 @@ create_1st_bitmap(void)
>
> 		pfn_start = paddr_to_pfn(phys_start);
> 		pfn_end   = paddr_to_pfn(phys_end);
>+		if (pfn_start > info->max_mapnr)
>+			continue;
>+		pfn_end = MIN(pfn_end, info->max_mapnr);
>
> 		for (pfn = pfn_start; pfn < pfn_end; pfn++) {
> 			set_bit_on_1st_bitmap(pfn);
>@@ -7511,6 +7514,9 @@ exclude_xen3_user_domain(void)
> 		pfn     = paddr_to_pfn(phys_start);
> 		pfn_end = paddr_to_pfn(phys_end);
> 		size    = pfn_end - pfn;
>+		if (pfn > info->max_mapnr)
>+			continue;
>+		pfn_end = MIN(pfn_end, info->max_mapnr);
>
> 		for (j = 0; pfn < pfn_end; pfn++, j++) {
> 			print_progress(PROGRESS_XEN_DOMAIN, j + (size * i),
>@@ -7575,6 +7581,9 @@ exclude_xen4_user_domain(void)
> 		pfn     = paddr_to_pfn(phys_start);
> 		pfn_end = paddr_to_pfn(phys_end);
> 		size    = pfn_end - pfn;
>+		if (pfn > info->max_mapnr)
>+			continue;
>+		pfn_end = MIN(pfn_end, info->max_mapnr);
>
> 		for (j = 0; pfn < pfn_end; pfn++, j++) {
> 			print_progress(PROGRESS_XEN_DOMAIN, j + (size * i),
>
>
>_______________________________________________
>kexec mailing list
>kexec@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/kexec

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-28 16:46           ` Michael Holzheu
  2014-03-28 16:53             ` Petr Tesarik
@ 2014-03-31 10:27             ` Petr Tesarik
  1 sibling, 0 replies; 17+ messages in thread
From: Petr Tesarik @ 2014-03-31 10:27 UTC (permalink / raw)
  To: Michael Holzheu; +Cc: kexec, d.hatayama, Atsushi Kumagai

On Fri, 28 Mar 2014 17:46:22 +0100
Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:

> On Fri, 28 Mar 2014 12:00:47 +0100
> Petr Tesarik <ptesarik@suse.cz> wrote:
> 
> > On Thu, 27 Mar 2014 14:54:41 +0100
> > Michael Holzheu <holzheu@linux.vnet.ibm.com> wrote:
> 
> [snip]
> 
> > > > >Here the fixed patch:
> > > > 
> > > > Thanks, I'll merge the fixed version into v1.5.6.
> > > 
> > > Great!
> > 
> > I'm sorry to spoil the party, but this patch broke Xen dumps for me.
> > I'm getting an long series of these messages:
> > 
> > set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> > set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> > set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap91pbsO). Invalid argument
> > ...
> > 
> > In fact, it most likely broke all non-cyclic dumps.
> > 
> > That's because the bitmap length is calculated in prepare_bitmap_buffer
> > using info->max_mapnr, but create_1st_bitmap() still loops over all
> > PT_LOAD segments, calling set_bit_on_1st_bitmap() for each PFN. The
> > offset may easily fall beyond the bitmap size.
> 
> What about the following patch. It works for me when I specify
> the "--non-cyclic" option.

I'm still getting a bunch of these:

set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap7lSPrl). Invalid
argument

This time they come in like this:

create_2nd_bitmap
-> exclude_free_page
   -> _exclude_free_page
      -> reset_bitmap_of_free_pages
         -> clear_bit_on_2nd_bitmap_for_kernel
            (here, physical PFN is translated to machine PFN)
            -> clear_bit_on_2nd_bitmap

The resulting machine PFN is beyond the bitmap extents.

Petr T

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-31  9:48               ` Atsushi Kumagai
@ 2014-03-31 10:37                 ` Petr Tesarik
  2014-04-01  5:06                   ` Atsushi Kumagai
  2014-03-31 12:59                 ` Michael Holzheu
  1 sibling, 1 reply; 17+ messages in thread
From: Petr Tesarik @ 2014-03-31 10:37 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: d.hatayama, holzheu, kexec

On Mon, 31 Mar 2014 09:48:05 +0000
Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:

>[...]
> 
> In the first place, we shouldn't truncate max_mapnr
> based on dom0's mem_section since there are some domU's
> memories on Xen dumps. Now, I think a better way for Xen
> is just leaving max_mapnr as it is.
> 
> Do you agree with my view ?

Definitely! For Xen, info->max_mapnr gives the maximum machine PFN
(ie. it corresponds to total memory installed in the machine).

The data in mem_section describes Dom0 kernel memory map (and gets
initialized from info->dom0_mapnr). It may be substantially smaller
than info->max_mapnr...

A "clean" solution would be to change info->max_mapnr so that it always
gives the maximum physical PFN, but that doesn't fly very well in
practice, because memory bitmaps and other stuff must still be sized
according to the number of machine PFNs, so I would have to add special
cases all around the place...

OTOH, Michal's patch is still neded to fix non-Xen non-cyclic dumps.

Petr T

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-31  9:48               ` Atsushi Kumagai
  2014-03-31 10:37                 ` Petr Tesarik
@ 2014-03-31 12:59                 ` Michael Holzheu
  1 sibling, 0 replies; 17+ messages in thread
From: Michael Holzheu @ 2014-03-31 12:59 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: kexec, d.hatayama, ptesarik

On Mon, 31 Mar 2014 09:48:05 +0000
Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:

> [snip]
> 
> >> > That's because the bitmap length is calculated in prepare_bitmap_buffer
> >> > using info->max_mapnr, but create_1st_bitmap() still loops over all
> >> > PT_LOAD segments, calling set_bit_on_1st_bitmap() for each PFN. The
> >> > offset may easily fall beyond the bitmap size.
> >>
> >> What about the following patch. It works for me when I specify
> >> the "--non-cyclic" option.
> >>
> >> Michael
> >> ---
> >> [PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr
> >>
> >> If info->max_mapnr has been adjusted, for example because the dumped
> >> system has specified the "mem=" kernel parameter, makedumpfile writes
> >> the following error messages for Xen dumps or when the "--non-cyclic"
> >> option has been specified:
> >>
> >> set_bitmap: Can't read the bitmap(/tmp/kdump_bitmap96s9V8). Success
>                                                                ^^^^^^^^
> This looks confusing, is it an actual message ?
> I suppose it must be "Invalid argument" like Petr's log.

Right, I get "Invalid argument".

No idea from where I pasted "Success" here...

> >>
> >> Fix this and consider "info->max_mapnr" in the create bitmap functions.
> >>
> >> Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
> >> ---
> >

[snip]

> I found another issue of truncating max_mapnr for Xen.
> 
> The bitmap manages MFN(machine frame number) for Xen
> while __exclude_unnecessary_pages() treats PFN(guest-physical frame number).
> __exclude_unnecessary_pages() expects that all bits of PFNs
> are mapped in the bitmap even if it was reduced by truncated 
> max_mapnr. However, PtoM mapping isn't linear(probably...),
> there is no guarantee that a set of continuous PFNs is mapped
> in a set of continuous MFNs.
> So the actual I/O offset can exceed the bitmap size when the
> bitmap size is reduced.
> 
> In the first place, we shouldn't truncate max_mapnr
> based on dom0's mem_section since there are some domU's
> memories on Xen dumps. Now, I think a better way for Xen
> is just leaving max_mapnr as it is.
> 
> Do you agree with my view ?

I don't know the Xen details so I would leave it to Petr
to answer this question.

Michael


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-03-31 10:37                 ` Petr Tesarik
@ 2014-04-01  5:06                   ` Atsushi Kumagai
  2014-04-01  8:11                     ` Petr Tesarik
  2014-04-01  9:20                     ` Michael Holzheu
  0 siblings, 2 replies; 17+ messages in thread
From: Atsushi Kumagai @ 2014-04-01  5:06 UTC (permalink / raw)
  To: ptesarik, holzheu; +Cc: d.hatayama, kexec

[...]
>>
>> In the first place, we shouldn't truncate max_mapnr
>> based on dom0's mem_section since there are some domU's
>> memories on Xen dumps. Now, I think a better way for Xen
>> is just leaving max_mapnr as it is.
>>
>> Do you agree with my view ?
>
>Definitely! For Xen, info->max_mapnr gives the maximum machine PFN
>(ie. it corresponds to total memory installed in the machine).
>
>The data in mem_section describes Dom0 kernel memory map (and gets
>initialized from info->dom0_mapnr). It may be substantially smaller
>than info->max_mapnr...

Thanks for your confirming.

>A "clean" solution would be to change info->max_mapnr so that it always
>gives the maximum physical PFN, but that doesn't fly very well in
>practice, because memory bitmaps and other stuff must still be sized
>according to the number of machine PFNs, so I would have to add special
>cases all around the place...

I don't know how to capture a dump on Xen well, so do you have any idea
how to produce the difference between actual memory regions and the ELF
header like the s390 case ?
If it isn't possible, we don't need to change info->max_mapnr since the
value calculated from the ELF header must be correct.

>OTOH, Michal's patch is still neded to fix non-Xen non-cyclic dumps.

Yes, the fix for create_1st_bitmap() is still necessary.

Michael, could you fix your patch ? We need to add the conditional
check for Xen like below:

+	if (!is_xen_memory()) {
+	       for (i = 0; i < info->num_mem_map; i++) {
+   	            if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
+       	                continue;
+           	    max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
+       	}
+       	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
+	}


Thanks
Atsushi Kumagai

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-04-01  5:06                   ` Atsushi Kumagai
@ 2014-04-01  8:11                     ` Petr Tesarik
  2014-04-01  9:20                     ` Michael Holzheu
  1 sibling, 0 replies; 17+ messages in thread
From: Petr Tesarik @ 2014-04-01  8:11 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: d.hatayama, holzheu, kexec

On Tue, 1 Apr 2014 05:06:33 +0000
Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:

> [...]
> >>
> >> In the first place, we shouldn't truncate max_mapnr
> >> based on dom0's mem_section since there are some domU's
> >> memories on Xen dumps. Now, I think a better way for Xen
> >> is just leaving max_mapnr as it is.
> >>
> >> Do you agree with my view ?
> >
> >Definitely! For Xen, info->max_mapnr gives the maximum machine PFN
> >(ie. it corresponds to total memory installed in the machine).
> >
> >The data in mem_section describes Dom0 kernel memory map (and gets
> >initialized from info->dom0_mapnr). It may be substantially smaller
> >than info->max_mapnr...
> 
> Thanks for your confirming.
> 
> >A "clean" solution would be to change info->max_mapnr so that it always
> >gives the maximum physical PFN, but that doesn't fly very well in
> >practice, because memory bitmaps and other stuff must still be sized
> >according to the number of machine PFNs, so I would have to add special
> >cases all around the place...
> 
> I don't know how to capture a dump on Xen well, so do you have any idea
> how to produce the difference between actual memory regions and the ELF
> header like the s390 case ?

I don't quite see what would be the Xen equivalent. Like I said in a
previous mail, memory regions under Xen are sized by Dom0's max_pfn, so
it makes no sense to set this value back from the memory regions.

> If it isn't possible, we don't need to change info->max_mapnr since the
> value calculated from the ELF header must be correct.

Ah, if we're talking about the ELF headers, then the extents are
determined by kexec-tools using information passed on by the Xen
hypervisor on boot. If the available memory is reduced using Xen's mem=
or availmem= command line parameter, then these headers are correct.
AFAIK there is no mechanism to change the amount of RAM used by the
hypervisor at run-time.

In short, I agree that the adjustment should be simply skipped for Xen,
exactly as you proposed in your patch:

+	if (!is_xen_memory()) {
+	       for (i = 0; i < info->num_mem_map; i++) {
+   	            if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
+       	                continue;
+           	    max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
+       	}
+       	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
+	}

Petr T

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-04-01  5:06                   ` Atsushi Kumagai
  2014-04-01  8:11                     ` Petr Tesarik
@ 2014-04-01  9:20                     ` Michael Holzheu
  2014-04-03  2:38                       ` Atsushi Kumagai
  1 sibling, 1 reply; 17+ messages in thread
From: Michael Holzheu @ 2014-04-01  9:20 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: kexec, d.hatayama, ptesarik

On Tue, 1 Apr 2014 05:06:33 +0000
Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:

[...]

> >OTOH, Michal's patch is still neded to fix non-Xen non-cyclic dumps.
> 
> Yes, the fix for create_1st_bitmap() is still necessary.
> 
> Michael, could you fix your patch ? We need to add the conditional
> check for Xen like below:
> 
> +	if (!is_xen_memory()) {
> +	       for (i = 0; i < info->num_mem_map; i++) {
> +   	            if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
> +       	                continue;
> +           	    max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
> +       	}
> +       	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
> +	}

Hello Atsushi and Petr,

Based on the discussion I removed the checks in exclude_xen3_user_domain()
and exclude_xen4_user_domain() and added the is_xen_memory() check
int get_mem_map().

Here the updated patch:
---
[PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr

If info->max_mapnr has been adjusted, for example because the dumped
system has specified the "mem=" kernel parameter, makedumpfile writes
the following error messages for Xen dumps or when the "--non-cyclic"
option has been specified:

set_bitmap: Can't read the bitmap(/tmp/kdump_bitmapBsKAUe). Invalid argument

Fix this and consider "info->max_mapnr" in the create_1st_bitmap() function.

In addition to this, do not adjust max_mapnr for Xen dumps.
For Xen info->max_mapnr gives the maximum machine PFN and the data
in mem_section describes the Dom0 kernel memory map that gets
initialized from info->dom0_mapnr. It may be substantially smaller
than info->max_mapnr.

Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
---
 makedumpfile.c |   15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -2868,12 +2868,14 @@ get_mem_map(void)
 	 * than is dumped. For example when "mem=" has been used for the
 	 * dumped system.
 	 */
-	for (i = 0; i < info->num_mem_map; i++) {
-		if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
-			continue;
-		max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
+	if (!is_xen_memory()) {
+		for (i = 0; i < info->num_mem_map; i++) {
+			if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
+				continue;
+			max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
+		}
+		info->max_mapnr = MIN(info->max_mapnr, max_pfn);
 	}
-	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
 	return ret;
 }
 
@@ -4402,6 +4404,9 @@ create_1st_bitmap(void)
 
 		pfn_start = paddr_to_pfn(phys_start);
 		pfn_end   = paddr_to_pfn(phys_end);
+		if (pfn_start > info->max_mapnr)
+			continue;
+		pfn_end = MIN(pfn_end, info->max_mapnr);
 
 		for (pfn = pfn_start; pfn < pfn_end; pfn++) {
 			set_bit_on_1st_bitmap(pfn);


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array
  2014-04-01  9:20                     ` Michael Holzheu
@ 2014-04-03  2:38                       ` Atsushi Kumagai
  0 siblings, 0 replies; 17+ messages in thread
From: Atsushi Kumagai @ 2014-04-03  2:38 UTC (permalink / raw)
  To: holzheu; +Cc: kexec, d.hatayama, ptesarik

>On Tue, 1 Apr 2014 05:06:33 +0000
>Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp> wrote:
>
>[...]
>
>> >OTOH, Michal's patch is still neded to fix non-Xen non-cyclic dumps.
>>
>> Yes, the fix for create_1st_bitmap() is still necessary.
>>
>> Michael, could you fix your patch ? We need to add the conditional
>> check for Xen like below:
>>
>> +	if (!is_xen_memory()) {
>> +	       for (i = 0; i < info->num_mem_map; i++) {
>> +   	            if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
>> +       	                continue;
>> +           	    max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
>> +       	}
>> +       	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
>> +	}
>
>Hello Atsushi and Petr,
>
>Based on the discussion I removed the checks in exclude_xen3_user_domain()
>and exclude_xen4_user_domain() and added the is_xen_memory() check
>int get_mem_map().
>
>Here the updated patch:

Good, I'll merge this into v1.5.6.
Thanks for your work, Michael and Petr !


Atsushi Kumagai

>---
>[PATCH] makedumpfile: Fix bitmap create for adjusted info->max_mapnr
>
>If info->max_mapnr has been adjusted, for example because the dumped
>system has specified the "mem=" kernel parameter, makedumpfile writes
>the following error messages for Xen dumps or when the "--non-cyclic"
>option has been specified:
>
>set_bitmap: Can't read the bitmap(/tmp/kdump_bitmapBsKAUe). Invalid argument
>
>Fix this and consider "info->max_mapnr" in the create_1st_bitmap() function.
>
>In addition to this, do not adjust max_mapnr for Xen dumps.
>For Xen info->max_mapnr gives the maximum machine PFN and the data
>in mem_section describes the Dom0 kernel memory map that gets
>initialized from info->dom0_mapnr. It may be substantially smaller
>than info->max_mapnr.
>
>Signed-off-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
>---
> makedumpfile.c |   15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -2868,12 +2868,14 @@ get_mem_map(void)
> 	 * than is dumped. For example when "mem=" has been used for the
> 	 * dumped system.
> 	 */
>-	for (i = 0; i < info->num_mem_map; i++) {
>-		if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
>-			continue;
>-		max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
>+	if (!is_xen_memory()) {
>+		for (i = 0; i < info->num_mem_map; i++) {
>+			if (info->mem_map_data[i].mem_map == NOT_MEMMAP_ADDR)
>+				continue;
>+			max_pfn = MAX(max_pfn, info->mem_map_data[i].pfn_end);
>+		}
>+		info->max_mapnr = MIN(info->max_mapnr, max_pfn);
> 	}
>-	info->max_mapnr = MIN(info->max_mapnr, max_pfn);
> 	return ret;
> }
>
>@@ -4402,6 +4404,9 @@ create_1st_bitmap(void)
>
> 		pfn_start = paddr_to_pfn(phys_start);
> 		pfn_end   = paddr_to_pfn(phys_end);
>+		if (pfn_start > info->max_mapnr)
>+			continue;
>+		pfn_end = MIN(pfn_end, info->max_mapnr);
>
> 		for (pfn = pfn_start; pfn < pfn_end; pfn++) {
> 			set_bit_on_1st_bitmap(pfn);

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-04-03  2:43 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-25 16:14 [PATCH 2/2] makedumpfile: Use max_pfn from mem_map array Michael Holzheu
2014-03-26  9:55 ` HATAYAMA Daisuke
2014-03-26 17:54   ` Michael Holzheu
2014-03-27  5:19     ` Atsushi Kumagai
2014-03-27 13:54       ` Michael Holzheu
2014-03-28 11:00         ` Petr Tesarik
2014-03-28 15:54           ` Michael Holzheu
2014-03-28 16:46           ` Michael Holzheu
2014-03-28 16:53             ` Petr Tesarik
2014-03-31  9:48               ` Atsushi Kumagai
2014-03-31 10:37                 ` Petr Tesarik
2014-04-01  5:06                   ` Atsushi Kumagai
2014-04-01  8:11                     ` Petr Tesarik
2014-04-01  9:20                     ` Michael Holzheu
2014-04-03  2:38                       ` Atsushi Kumagai
2014-03-31 12:59                 ` Michael Holzheu
2014-03-31 10:27             ` Petr Tesarik

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.