* [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@linux.vnet.ibm.com Cc: d.hatayama@jp.fujitsu.com, kexec@lists.infradead.org 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@jp.fujitsu.com, kexec@lists.infradead.org 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@lists.infradead.org, d.hatayama@jp.fujitsu.com, 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@lists.infradead.org, d.hatayama@jp.fujitsu.com, 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@lists.infradead.org, d.hatayama@jp.fujitsu.com, 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@jp.fujitsu.com, kexec@lists.infradead.org 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@suse.cz, holzheu@linux.vnet.ibm.com Cc: d.hatayama@jp.fujitsu.com, kexec@lists.infradead.org [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-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@jp.fujitsu.com, holzheu@linux.vnet.ibm.com, kexec@lists.infradead.org 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 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@suse.cz, holzheu@linux.vnet.ibm.com Cc: d.hatayama@jp.fujitsu.com, kexec@lists.infradead.org [...] >> >> 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@jp.fujitsu.com, holzheu@linux.vnet.ibm.com, kexec@lists.infradead.org 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@lists.infradead.org, d.hatayama@jp.fujitsu.com, ptesarik@suse.cz 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@linux.vnet.ibm.com Cc: kexec@lists.infradead.org, d.hatayama@jp.fujitsu.com, ptesarik@suse.cz >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
* 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@lists.infradead.org, d.hatayama@jp.fujitsu.com, ptesarik@suse.cz 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-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@lists.infradead.org, d.hatayama@jp.fujitsu.com, 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox