[PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[Jingbai Ma]

This patch will fix a bug of makedumpfile doesn't work correctly on system
has over 44-bit addressing in compression dump mode.
This bug was posted here:
http://lists.infradead.org/pipermail/kexec/2013-September/009587.html

This patch will add a new field in struct kdump_sub_header.
unsigned long   max_mapnr;

And the old "unsigned int max_mapnr" in struct disk_dump_header will
not be used anymore. But still be there for compatibility purpose.

This patch will change the header_version to 6.

The corresponding patch for crash utility will be sent out separately.

This patch doesn't change sadump_header.
Because of in sadump file, there is no any sub-header, it has to change
the sadump_header itself.
And if do so, will cause backwards-compatibility issue.
So it could be a separate patch if needed.

Signed-off-by: Jingbai Ma <jingbai.ma@hp.com>
---
 IMPLEMENTATION |    1 +
 diskdump_mod.h |    5 ++++-
 makedumpfile.c |   28 ++++++++++++++++++++++------
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/IMPLEMENTATION b/IMPLEMENTATION
index f0f3135..d576811 100644
--- a/IMPLEMENTATION
+++ b/IMPLEMENTATION
@@ -77,6 +77,7 @@
 	unsigned long	size_note;        /* header_version 4 and later */
 	off_t		offset_eraseinfo; /* header_version 5 and later */
 	unsigned long	size_eraseinfo;   /* header_version 5 and later */
+	unsigned long	max_mapnr;        /* header_version 6 and later */
     };
 
   - 1st-bitmap
diff --git a/diskdump_mod.h b/diskdump_mod.h
index af060b6..30306a5 100644
--- a/diskdump_mod.h
+++ b/diskdump_mod.h
@@ -48,7 +48,9 @@ struct disk_dump_header {
 						   header in blocks */
 	unsigned int		bitmap_blocks;	/* Size of Memory bitmap in
 						   block */
-	unsigned int		max_mapnr;	/* = max_mapnr */
+	unsigned int		max_mapnr;	/* = max_mapnr, 32bit only,
+						   full 64bit in sub header.
+						   Do not use this anymore! */
 	unsigned int		total_ram_blocks;/* Number of blocks should be
 						   written */
 	unsigned int		device_blocks;	/* Number of total blocks in
@@ -75,6 +77,7 @@ struct kdump_sub_header {
 	unsigned long	size_note;        /* header_version 4 and later */
 	off_t		offset_eraseinfo; /* header_version 5 and later */
 	unsigned long	size_eraseinfo;   /* header_version 5 and later */
+	unsigned long	max_mapnr;        /* header_version 6 and later */
 };
 
 /* page flags */
diff --git a/makedumpfile.c b/makedumpfile.c
index b42565c..a444adf 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -125,7 +125,7 @@ get_max_mapnr(void)
 	unsigned long long max_paddr;
 
 	if (info->flag_refiltering) {
-		info->max_mapnr = info->dh_memory->max_mapnr;
+		info->max_mapnr = info->kh_memory->max_mapnr;
 		return TRUE;
 	}
 
@@ -783,6 +783,10 @@ get_kdump_compressed_header_info(char *filename)
 		ERRMSG("header does not have dump_level member\n");
 		return FALSE;
 	}
+
+	if (dh.header_version < 6)
+		kh.max_mapnr = dh.max_mapnr;
+
 	DEBUG_MSG("diskdump main header\n");
 	DEBUG_MSG("  signature        : %s\n", dh.signature);
 	DEBUG_MSG("  header_version   : %d\n", dh.header_version);
@@ -790,7 +794,7 @@ get_kdump_compressed_header_info(char *filename)
 	DEBUG_MSG("  block_size       : %d\n", dh.block_size);
 	DEBUG_MSG("  sub_hdr_size     : %d\n", dh.sub_hdr_size);
 	DEBUG_MSG("  bitmap_blocks    : %d\n", dh.bitmap_blocks);
-	DEBUG_MSG("  max_mapnr        : 0x%x\n", dh.max_mapnr);
+	DEBUG_MSG("  max_mapnr(32bit) : 0x%x\n", dh.max_mapnr);
 	DEBUG_MSG("  total_ram_blocks : %d\n", dh.total_ram_blocks);
 	DEBUG_MSG("  device_blocks    : %d\n", dh.device_blocks);
 	DEBUG_MSG("  written_blocks   : %d\n", dh.written_blocks);
@@ -802,6 +806,7 @@ get_kdump_compressed_header_info(char *filename)
 	DEBUG_MSG("  split            : %d\n", kh.split);
 	DEBUG_MSG("  start_pfn        : 0x%lx\n", kh.start_pfn);
 	DEBUG_MSG("  end_pfn          : 0x%lx\n", kh.end_pfn);
+	DEBUG_MSG("  max_mapnr(64bit) : 0x%lx\n", kh.max_mapnr);
 
 	info->dh_memory = malloc(sizeof(dh));
 	if (info->dh_memory == NULL) {
@@ -2766,6 +2771,7 @@ int
 initialize_bitmap_memory(void)
 {
 	struct disk_dump_header	*dh;
+	struct kdump_sub_header *kh;
 	struct dump_bitmap *bmp;
 	off_t bitmap_offset;
 	int bitmap_len, max_sect_len;
@@ -2774,6 +2780,7 @@ initialize_bitmap_memory(void)
 	long block_size;
 
 	dh = info->dh_memory;
+	kh = info->kh_memory;
 	block_size = dh->block_size;
 
 	bitmap_offset
@@ -2793,7 +2800,7 @@ initialize_bitmap_memory(void)
 	bmp->offset = bitmap_offset + bitmap_len / 2;
 	info->bitmap_memory = bmp;
 
-	max_sect_len = divideup(dh->max_mapnr, BITMAP_SECT_LEN);
+	max_sect_len = divideup(kh->max_mapnr, BITMAP_SECT_LEN);
 	info->valid_pages = calloc(sizeof(ulong), max_sect_len);
 	if (info->valid_pages == NULL) {
 		ERRMSG("Can't allocate memory for the valid_pages. %s\n",
@@ -5153,10 +5160,11 @@ write_kdump_header(void)
 	 * Write common header
 	 */
 	strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
-	dh->header_version = 5;
+	dh->header_version = 6;
 	dh->block_size     = info->page_size;
 	dh->sub_hdr_size   = sizeof(kh) + size_note;
 	dh->sub_hdr_size   = divideup(dh->sub_hdr_size, dh->block_size);
+	/* dh->max_mapnr may be truncated here, full 64bit in kh.max_mapnr */
 	dh->max_mapnr      = info->max_mapnr;
 	dh->nr_cpus        = get_nr_cpus();
 	dh->bitmap_blocks  = divideup(info->len_bitmap, dh->block_size);
@@ -5172,6 +5180,7 @@ write_kdump_header(void)
 	 */
 	size = sizeof(struct kdump_sub_header);
 	memset(&kh, 0, size);
+	kh.max_mapnr = info->max_mapnr;
 	kh.phys_base  = info->phys_base;
 	kh.dump_level = info->dump_level;
 	if (info->flag_split) {
@@ -7796,10 +7805,8 @@ store_splitting_info(void)
 
 		if (i == 0) {
 			memcpy(&dh, &tmp_dh, sizeof(tmp_dh));
-			info->max_mapnr = dh.max_mapnr;
 			if (!set_page_size(dh.block_size))
 				return FALSE;
-			DEBUG_MSG("max_mapnr    : %llx\n", info->max_mapnr);
 			DEBUG_MSG("page_size    : %ld\n", info->page_size);
 		}
 
@@ -7816,6 +7823,15 @@ store_splitting_info(void)
 			return FALSE;
 
 		if (i == 0) {
+			if (dh.header_version >= 6)
+				info->max_mapnr = kh.max_mapnr;
+			else
+				info->max_mapnr = dh.max_mapnr;
+
+			DEBUG_MSG("max_mapnr    : %llx\n", info->max_mapnr);
+		}
+
+		if (i == 0) {
 			info->dump_level = kh.dump_level;
 			DEBUG_MSG("dump_level   : %d\n", info->dump_level);
 		}


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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[HATAYAMA Daisuke]

(2013/09/24 21:49), Jingbai Ma wrote:
> This patch will fix a bug of makedumpfile doesn't work correctly on system
> has over 44-bit addressing in compression dump mode.
> This bug was posted here:
> http://lists.infradead.org/pipermail/kexec/2013-September/009587.html
>
> This patch will add a new field in struct kdump_sub_header.
> unsigned long   max_mapnr;
>
> And the old "unsigned int max_mapnr" in struct disk_dump_header will
> not be used anymore. But still be there for compatibility purpose.
>
> This patch will change the header_version to 6.
>
> The corresponding patch for crash utility will be sent out separately.
>
> This patch doesn't change sadump_header.
> Because of in sadump file, there is no any sub-header, it has to change
> the sadump_header itself.
> And if do so, will cause backwards-compatibility issue.
> So it could be a separate patch if needed.
>
> Signed-off-by: Jingbai Ma <jingbai.ma@hp.com>
> ---
>   IMPLEMENTATION |    1 +
>   diskdump_mod.h |    5 ++++-
>   makedumpfile.c |   28 ++++++++++++++++++++++------
>   3 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/IMPLEMENTATION b/IMPLEMENTATION
> index f0f3135..d576811 100644
> --- a/IMPLEMENTATION
> +++ b/IMPLEMENTATION
> @@ -77,6 +77,7 @@
>   	unsigned long	size_note;        /* header_version 4 and later */
>   	off_t		offset_eraseinfo; /* header_version 5 and later */
>   	unsigned long	size_eraseinfo;   /* header_version 5 and later */
> +	unsigned long	max_mapnr;        /* header_version 6 and later */

x86 PAE mode can represents 52-bit physical addresses and so 40-bit physical
page frames. On x86_32 unsigned long has 32-bit length only. So, you should
define max_mapnr as unsigned long long.

>       };
>
>     - 1st-bitmap
> diff --git a/diskdump_mod.h b/diskdump_mod.h
> index af060b6..30306a5 100644
> --- a/diskdump_mod.h
> +++ b/diskdump_mod.h
> @@ -48,7 +48,9 @@ struct disk_dump_header {
>   						   header in blocks */
>   	unsigned int		bitmap_blocks;	/* Size of Memory bitmap in
>   						   block */
> -	unsigned int		max_mapnr;	/* = max_mapnr */
> +	unsigned int		max_mapnr;	/* = max_mapnr, 32bit only,
> +						   full 64bit in sub header.
> +						   Do not use this anymore! */
>   	unsigned int		total_ram_blocks;/* Number of blocks should be
>   						   written */
>   	unsigned int		device_blocks;	/* Number of total blocks in
> @@ -75,6 +77,7 @@ struct kdump_sub_header {
>   	unsigned long	size_note;        /* header_version 4 and later */
>   	off_t		offset_eraseinfo; /* header_version 5 and later */
>   	unsigned long	size_eraseinfo;   /* header_version 5 and later */
> +	unsigned long	max_mapnr;        /* header_version 6 and later */

This, too.

> @@ -5153,10 +5160,11 @@ write_kdump_header(void)
>   	 * Write common header
>   	 */
>   	strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
> -	dh->header_version = 5;
> +	dh->header_version = 6;
>   	dh->block_size     = info->page_size;
>   	dh->sub_hdr_size   = sizeof(kh) + size_note;
>   	dh->sub_hdr_size   = divideup(dh->sub_hdr_size, dh->block_size);
> +	/* dh->max_mapnr may be truncated here, full 64bit in kh.max_mapnr */
>   	dh->max_mapnr      = info->max_mapnr;

dh->max_mapnr = MIN(info->max_mapnr, UINT_MAX) seems better for old versions of crash utitlity.

-- 
Thanks.
HATAYAMA, Daisuke


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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[Jingbai Ma]

On 09/25/2013 08:35 AM, HATAYAMA Daisuke wrote:
> (2013/09/24 21:49), Jingbai Ma wrote:
>> This patch will fix a bug of makedumpfile doesn't work correctly on
>> system
>> has over 44-bit addressing in compression dump mode.
>> This bug was posted here:
>> http://lists.infradead.org/pipermail/kexec/2013-September/009587.html
>>
>> This patch will add a new field in struct kdump_sub_header.
>> unsigned long max_mapnr;
>>
>> And the old "unsigned int max_mapnr" in struct disk_dump_header will
>> not be used anymore. But still be there for compatibility purpose.
>>
>> This patch will change the header_version to 6.
>>
>> The corresponding patch for crash utility will be sent out separately.
>>
>> This patch doesn't change sadump_header.
>> Because of in sadump file, there is no any sub-header, it has to change
>> the sadump_header itself.
>> And if do so, will cause backwards-compatibility issue.
>> So it could be a separate patch if needed.
>>
>> Signed-off-by: Jingbai Ma <jingbai.ma@hp.com>
>> ---
>> IMPLEMENTATION | 1 +
>> diskdump_mod.h | 5 ++++-
>> makedumpfile.c | 28 ++++++++++++++++++++++------
>> 3 files changed, 27 insertions(+), 7 deletions(-)
>>
>> diff --git a/IMPLEMENTATION b/IMPLEMENTATION
>> index f0f3135..d576811 100644
>> --- a/IMPLEMENTATION
>> +++ b/IMPLEMENTATION
>> @@ -77,6 +77,7 @@
>> unsigned long size_note; /* header_version 4 and later */
>> off_t offset_eraseinfo; /* header_version 5 and later */
>> unsigned long size_eraseinfo; /* header_version 5 and later */
>> + unsigned long max_mapnr; /* header_version 6 and later */
>
> x86 PAE mode can represents 52-bit physical addresses and so 40-bit
> physical
> page frames. On x86_32 unsigned long has 32-bit length only. So, you should
> define max_mapnr as unsigned long long.
>

Good catch, I forgot x86 PAE mode also may exceed 32-bit length.
Will fix it.

>> };
>>
>> - 1st-bitmap
>> diff --git a/diskdump_mod.h b/diskdump_mod.h
>> index af060b6..30306a5 100644
>> --- a/diskdump_mod.h
>> +++ b/diskdump_mod.h
>> @@ -48,7 +48,9 @@ struct disk_dump_header {
>> header in blocks */
>> unsigned int bitmap_blocks; /* Size of Memory bitmap in
>> block */
>> - unsigned int max_mapnr; /* = max_mapnr */
>> + unsigned int max_mapnr; /* = max_mapnr, 32bit only,
>> + full 64bit in sub header.
>> + Do not use this anymore! */
>> unsigned int total_ram_blocks;/* Number of blocks should be
>> written */
>> unsigned int device_blocks; /* Number of total blocks in
>> @@ -75,6 +77,7 @@ struct kdump_sub_header {
>> unsigned long size_note; /* header_version 4 and later */
>> off_t offset_eraseinfo; /* header_version 5 and later */
>> unsigned long size_eraseinfo; /* header_version 5 and later */
>> + unsigned long max_mapnr; /* header_version 6 and later */
>
> This, too.

Will fix it.

>
>> @@ -5153,10 +5160,11 @@ write_kdump_header(void)
>> * Write common header
>> */
>> strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
>> - dh->header_version = 5;
>> + dh->header_version = 6;
>> dh->block_size = info->page_size;
>> dh->sub_hdr_size = sizeof(kh) + size_note;
>> dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size);
>> + /* dh->max_mapnr may be truncated here, full 64bit in kh.max_mapnr */
>> dh->max_mapnr = info->max_mapnr;
>
> dh->max_mapnr = MIN(info->max_mapnr, UINT_MAX) seems better for old
> versions of crash utitlity.
>

Although change this value to UINT_MAX doesn't help the old crash 
utility very much. At least this special value will tell the user 
something happened.
Will fix it.


-- 
Thanks,
Jingbai Ma

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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[HATAYAMA Daisuke]

(2013/09/25 17:16), Jingbai Ma wrote:
<cut>
>>
>>> @@ -5153,10 +5160,11 @@ write_kdump_header(void)
>>> * Write common header
>>> */
>>> strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
>>> - dh->header_version = 5;
>>> + dh->header_version = 6;
>>> dh->block_size = info->page_size;
>>> dh->sub_hdr_size = sizeof(kh) + size_note;
>>> dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size);
>>> + /* dh->max_mapnr may be truncated here, full 64bit in kh.max_mapnr */
>>> dh->max_mapnr = info->max_mapnr;
>>
>> dh->max_mapnr = MIN(info->max_mapnr, UINT_MAX) seems better for old
>> versions of crash utitlity.
>>
>
> Although change this value to UINT_MAX doesn't help the old crash utility very much. At least this special value will tell the user something happened.
> Will fix it.
>
>

This is the largest page frame number old crash utilities can represent, not special value.
By this, old crash utilities can read at least the pages less than UINT_MAX,
which is better than making dh->max_mapnr overflow; then dh->max_mapnr would typically become
a small number.

-- 
Thanks.
HATAYAMA, Daisuke


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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[Jingbai Ma]

On 09/25/2013 04:39 PM, HATAYAMA Daisuke wrote:
> (2013/09/25 17:16), Jingbai Ma wrote:
> <cut>
>>>
>>>> @@ -5153,10 +5160,11 @@ write_kdump_header(void)
>>>> * Write common header
>>>> */
>>>> strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
>>>> - dh->header_version = 5;
>>>> + dh->header_version = 6;
>>>> dh->block_size = info->page_size;
>>>> dh->sub_hdr_size = sizeof(kh) + size_note;
>>>> dh->sub_hdr_size = divideup(dh->sub_hdr_size, dh->block_size);
>>>> + /* dh->max_mapnr may be truncated here, full 64bit in kh.max_mapnr */
>>>> dh->max_mapnr = info->max_mapnr;
>>>
>>> dh->max_mapnr = MIN(info->max_mapnr, UINT_MAX) seems better for old
>>> versions of crash utitlity.
>>>
>>
>> Although change this value to UINT_MAX doesn't help the old crash
>> utility very much. At least this special value will tell the user
>> something happened.
>> Will fix it.
>>
>>
>
> This is the largest page frame number old crash utilities can represent,
> not special value.
> By this, old crash utilities can read at least the pages less than
> UINT_MAX,
> which is better than making dh->max_mapnr overflow; then dh->max_mapnr
> would typically become
> a small number.
>

I knew, but crash utility may still report reading page error before its 
command prompt.
Anyway, UINT_MAX has a better chance to make old utility happy, so I 
will fix it. :)

-- 
Thanks,
Jingbai Ma

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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[Atsushi Kumagai]

Hello Jingbai,

I have a additional comment.

(2013/09/25 17:54), Jingbai Ma wrote:
> On 09/25/2013 04:39 PM, HATAYAMA Daisuke wrote:
>> (2013/09/25 17:16), Jingbai Ma wrote:
>> <cut>
> @@ -790,7 +794,7 @@ get_kdump_compressed_header_info(char *filename)
>       DEBUG_MSG("  block_size       : %d\n", dh.block_size);
>       DEBUG_MSG("  sub_hdr_size     : %d\n", dh.sub_hdr_size);
>       DEBUG_MSG("  bitmap_blocks    : %d\n", dh.bitmap_blocks);
> -     DEBUG_MSG("  max_mapnr        : 0x%x\n", dh.max_mapnr);
> +     DEBUG_MSG("  max_mapnr(32bit) : 0x%x\n", dh.max_mapnr);
>       DEBUG_MSG("  total_ram_blocks : %d\n", dh.total_ram_blocks);
>       DEBUG_MSG("  device_blocks    : %d\n", dh.device_blocks);
>       DEBUG_MSG("  written_blocks   : %d\n", dh.written_blocks);
> @@ -802,6 +806,7 @@ get_kdump_compressed_header_info(char *filename)
>       DEBUG_MSG("  split            : %d\n", kh.split);
>       DEBUG_MSG("  start_pfn        : 0x%lx\n", kh.start_pfn);
>       DEBUG_MSG("  end_pfn          : 0x%lx\n", kh.end_pfn);
> +     DEBUG_MSG("  max_mapnr(64bit) : 0x%lx\n", kh.max_mapnr);

max_mapnr(64bit) will be included only in header_version 6 and later,
but your code shows that in any version.
I think it's better to change the code like below:

diff --git a/makedumpfile.c b/makedumpfile.c
index 069a903..6c20e88 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -847,6 +847,10 @@ get_kdump_compressed_header_info(char *filename)
  				(unsigned long long)kh.offset_eraseinfo);
  		DEBUG_MSG("  size_eraseinfo   : 0x%ld\n", kh.size_eraseinfo);
  	}
+	if (dh.header_version >= 6) {
+		/* A dumpfile contains full 64bit max_mapnr. */
+		DEBUG_MSG("  max_mapnr(64bit) : 0x%llx\n", kh.max_mapnr);
+	}
  	return TRUE;
  error:
  	free(info->dh_memory);


Thanks
Atsushi Kumagai

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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[Ma, Jingbai (Kingboard)]

On 10/2/13 2:20 PM, "Atsushi Kumagai" <kumagai-atsushi@mxc.nes.nec.co.jp>
wrote:


>Hello Jingbai,
>
>I have a additional comment.
>
>(2013/09/25 17:54), Jingbai Ma wrote:
>> On 09/25/2013 04:39 PM, HATAYAMA Daisuke wrote:
>>> (2013/09/25 17:16), Jingbai Ma wrote:
>>> <cut>
>> @@ -790,7 +794,7 @@ get_kdump_compressed_header_info(char *filename)
>>       DEBUG_MSG("  block_size       : %d\n", dh.block_size);
>>       DEBUG_MSG("  sub_hdr_size     : %d\n", dh.sub_hdr_size);
>>       DEBUG_MSG("  bitmap_blocks    : %d\n", dh.bitmap_blocks);
>> -     DEBUG_MSG("  max_mapnr        : 0x%x\n", dh.max_mapnr);
>> +     DEBUG_MSG("  max_mapnr(32bit) : 0x%x\n", dh.max_mapnr);
>>       DEBUG_MSG("  total_ram_blocks : %d\n", dh.total_ram_blocks);
>>       DEBUG_MSG("  device_blocks    : %d\n", dh.device_blocks);
>>       DEBUG_MSG("  written_blocks   : %d\n", dh.written_blocks);
>> @@ -802,6 +806,7 @@ get_kdump_compressed_header_info(char *filename)
>>       DEBUG_MSG("  split            : %d\n", kh.split);
>>       DEBUG_MSG("  start_pfn        : 0x%lx\n", kh.start_pfn);
>>       DEBUG_MSG("  end_pfn          : 0x%lx\n", kh.end_pfn);
>> +     DEBUG_MSG("  max_mapnr(64bit) : 0x%lx\n", kh.max_mapnr);
>
>max_mapnr(64bit) will be included only in header_version 6 and later,
>but your code shows that in any version.
>I think it's better to change the code like below:
>
>diff --git a/makedumpfile.c b/makedumpfile.c
>index 069a903..6c20e88 100644
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -847,6 +847,10 @@ get_kdump_compressed_header_info(char *filename)
>  				(unsigned long long)kh.offset_eraseinfo);
>  		DEBUG_MSG("  size_eraseinfo   : 0x%ld\n", kh.size_eraseinfo);
>  	}
>+	if (dh.header_version >= 6) {
>+		/* A dumpfile contains full 64bit max_mapnr. */
>+		DEBUG_MSG("  max_mapnr(64bit) : 0x%llx\n", kh.max_mapnr);
>+	}
>  	return TRUE;
>  error:
>  	free(info->dh_memory);

OK, I will fix it in next version.
Thanks!

>
>
>Thanks
>Atsushi Kumagai

Thanks,
Jingbai Ma


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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[HATAYAMA Daisuke]

(2013/09/25 17:16), Jingbai Ma wrote:
> On 09/25/2013 08:35 AM, HATAYAMA Daisuke wrote:
>> (2013/09/24 21:49), Jingbai Ma wrote:
>>> This patch will fix a bug of makedumpfile doesn't work correctly on
>>> system
>>> has over 44-bit addressing in compression dump mode.
>>> This bug was posted here:
>>> http://lists.infradead.org/pipermail/kexec/2013-September/009587.html
>>>
>>> This patch will add a new field in struct kdump_sub_header.
>>> unsigned long max_mapnr;
>>>
>>> And the old "unsigned int max_mapnr" in struct disk_dump_header will
>>> not be used anymore. But still be there for compatibility purpose.
>>>
>>> This patch will change the header_version to 6.
>>>
>>> The corresponding patch for crash utility will be sent out separately.
>>>
>>> This patch doesn't change sadump_header.
>>> Because of in sadump file, there is no any sub-header, it has to change
>>> the sadump_header itself.
>>> And if do so, will cause backwards-compatibility issue.
>>> So it could be a separate patch if needed.
>>>
>>> Signed-off-by: Jingbai Ma <jingbai.ma@hp.com>
>>> ---
>>> IMPLEMENTATION | 1 +
>>> diskdump_mod.h | 5 ++++-
>>> makedumpfile.c | 28 ++++++++++++++++++++++------
>>> 3 files changed, 27 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/IMPLEMENTATION b/IMPLEMENTATION
>>> index f0f3135..d576811 100644
>>> --- a/IMPLEMENTATION
>>> +++ b/IMPLEMENTATION
>>> @@ -77,6 +77,7 @@
>>> unsigned long size_note; /* header_version 4 and later */
>>> off_t offset_eraseinfo; /* header_version 5 and later */
>>> unsigned long size_eraseinfo; /* header_version 5 and later */
>>> + unsigned long max_mapnr; /* header_version 6 and later */
>>
>> x86 PAE mode can represents 52-bit physical addresses and so 40-bit
>> physical
>> page frames. On x86_32 unsigned long has 32-bit length only. So, you should
>> define max_mapnr as unsigned long long.
>>
>
> Good catch, I forgot x86 PAE mode also may exceed 32-bit length.
> Will fix it.
>

Though perhaps you've already noticed, both start_pfn and end_pfn in kdump sub
header have 32-bit length on x86_32. These need to be extended to 64-bit length,
too.

/*
  * Sub header for KDUMP
  * But Common header of KDUMP is disk_dump_header of diskdump.
  */
struct kdump_sub_header {
         unsigned long   phys_base;
         int             dump_level;     /* header_version 1 and later */
         int             split;          /* header_version 2 and later */
         unsigned long   start_pfn;      /* header_version 2 and later */
         unsigned long   end_pfn;        /* header_version 2 and later */
         off_t           offset_vmcoreinfo;/* header_version 3 and later */
         unsigned long   size_vmcoreinfo;  /* header_version 3 and later */
         off_t           offset_note;      /* header_version 4 and later */
         unsigned long   size_note;        /* header_version 4 and later */
         off_t           offset_eraseinfo; /* header_version 5 and later */
         unsigned long   size_eraseinfo;   /* header_version 5 and later */
};

-- 
Thanks.
HATAYAMA, Daisuke


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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[Jingbai Ma]

On 10/08/2013 01:41 PM, HATAYAMA Daisuke wrote:
> (2013/09/25 17:16), Jingbai Ma wrote:
>> On 09/25/2013 08:35 AM, HATAYAMA Daisuke wrote:
>>> (2013/09/24 21:49), Jingbai Ma wrote:
>>>> This patch will fix a bug of makedumpfile doesn't work correctly on
>>>> system
>>>> has over 44-bit addressing in compression dump mode.
>>>> This bug was posted here:
>>>> http://lists.infradead.org/pipermail/kexec/2013-September/009587.html
>>>>
>>>> This patch will add a new field in struct kdump_sub_header.
>>>> unsigned long max_mapnr;
>>>>
>>>> And the old "unsigned int max_mapnr" in struct disk_dump_header will
>>>> not be used anymore. But still be there for compatibility purpose.
>>>>
>>>> This patch will change the header_version to 6.
>>>>
>>>> The corresponding patch for crash utility will be sent out separately.
>>>>
>>>> This patch doesn't change sadump_header.
>>>> Because of in sadump file, there is no any sub-header, it has to change
>>>> the sadump_header itself.
>>>> And if do so, will cause backwards-compatibility issue.
>>>> So it could be a separate patch if needed.
>>>>
>>>> Signed-off-by: Jingbai Ma <jingbai.ma@hp.com>
>>>> ---
>>>> IMPLEMENTATION | 1 +
>>>> diskdump_mod.h | 5 ++++-
>>>> makedumpfile.c | 28 ++++++++++++++++++++++------
>>>> 3 files changed, 27 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/IMPLEMENTATION b/IMPLEMENTATION
>>>> index f0f3135..d576811 100644
>>>> --- a/IMPLEMENTATION
>>>> +++ b/IMPLEMENTATION
>>>> @@ -77,6 +77,7 @@
>>>> unsigned long size_note; /* header_version 4 and later */
>>>> off_t offset_eraseinfo; /* header_version 5 and later */
>>>> unsigned long size_eraseinfo; /* header_version 5 and later */
>>>> + unsigned long max_mapnr; /* header_version 6 and later */
>>>
>>> x86 PAE mode can represents 52-bit physical addresses and so 40-bit
>>> physical
>>> page frames. On x86_32 unsigned long has 32-bit length only. So, you
>>> should
>>> define max_mapnr as unsigned long long.
>>>
>>
>> Good catch, I forgot x86 PAE mode also may exceed 32-bit length.
>> Will fix it.
>>
>
> Though perhaps you've already noticed, both start_pfn and end_pfn in
> kdump sub
> header have 32-bit length on x86_32. These need to be extended to 64-bit
> length,
> too.
>
> /*
> * Sub header for KDUMP
> * But Common header of KDUMP is disk_dump_header of diskdump.
> */
> struct kdump_sub_header {
> unsigned long phys_base;
> int dump_level; /* header_version 1 and later */
> int split; /* header_version 2 and later */
> unsigned long start_pfn; /* header_version 2 and later */
> unsigned long end_pfn; /* header_version 2 and later */
> off_t offset_vmcoreinfo;/* header_version 3 and later */
> unsigned long size_vmcoreinfo; /* header_version 3 and later */
> off_t offset_note; /* header_version 4 and later */
> unsigned long size_note; /* header_version 4 and later */
> off_t offset_eraseinfo; /* header_version 5 and later */
> unsigned long size_eraseinfo; /* header_version 5 and later */
> };
>

Thanks for your reminder.
Yes, I'm going to add another two new 64-bit values for them as well as 
max_mapnr.
How about these names?
unsigned long long start_pfn_64;
unsigned long long end_pfn_64;
unsigned long long max_mapnr_64;

BTW, Could you give me any suggestions about the the max_mapnr in sadump 
header.
Currently, it defined as below:
struct sadump_header {
...
         uint32_t max_mapnr;     /* = max_mapnr */
...
};
And there is no any sub-header in sadump format. How would you like to 
keep the backward compatibility?
I can add a new unit64_t max_mapnr_64 in this sadump_header but it will 
change the header size. It should work if I add some code to check 
header_version for sadump format, and for old format just assign the 
value of max_mapnr to max_mapnr_64.


-- 
Thanks,
Jingbai Ma

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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[HATAYAMA Daisuke]

(2013/10/08 16:26), Jingbai Ma wrote:
> On 10/08/2013 01:41 PM, HATAYAMA Daisuke wrote:
>> (2013/09/25 17:16), Jingbai Ma wrote:
>>> On 09/25/2013 08:35 AM, HATAYAMA Daisuke wrote:
>>>> (2013/09/24 21:49), Jingbai Ma wrote:
>>>>> This patch will fix a bug of makedumpfile doesn't work correctly on
>>>>> system
>>>>> has over 44-bit addressing in compression dump mode.
>>>>> This bug was posted here:
>>>>> http://lists.infradead.org/pipermail/kexec/2013-September/009587.html
>>>>>
>>>>> This patch will add a new field in struct kdump_sub_header.
>>>>> unsigned long max_mapnr;
>>>>>
>>>>> And the old "unsigned int max_mapnr" in struct disk_dump_header will
>>>>> not be used anymore. But still be there for compatibility purpose.
>>>>>
>>>>> This patch will change the header_version to 6.
>>>>>
>>>>> The corresponding patch for crash utility will be sent out separately.
>>>>>
>>>>> This patch doesn't change sadump_header.
>>>>> Because of in sadump file, there is no any sub-header, it has to change
>>>>> the sadump_header itself.
>>>>> And if do so, will cause backwards-compatibility issue.
>>>>> So it could be a separate patch if needed.
>>>>>
>>>>> Signed-off-by: Jingbai Ma <jingbai.ma@hp.com>
>>>>> ---
>>>>> IMPLEMENTATION | 1 +
>>>>> diskdump_mod.h | 5 ++++-
>>>>> makedumpfile.c | 28 ++++++++++++++++++++++------
>>>>> 3 files changed, 27 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/IMPLEMENTATION b/IMPLEMENTATION
>>>>> index f0f3135..d576811 100644
>>>>> --- a/IMPLEMENTATION
>>>>> +++ b/IMPLEMENTATION
>>>>> @@ -77,6 +77,7 @@
>>>>> unsigned long size_note; /* header_version 4 and later */
>>>>> off_t offset_eraseinfo; /* header_version 5 and later */
>>>>> unsigned long size_eraseinfo; /* header_version 5 and later */
>>>>> + unsigned long max_mapnr; /* header_version 6 and later */
>>>>
>>>> x86 PAE mode can represents 52-bit physical addresses and so 40-bit
>>>> physical
>>>> page frames. On x86_32 unsigned long has 32-bit length only. So, you
>>>> should
>>>> define max_mapnr as unsigned long long.
>>>>
>>>
>>> Good catch, I forgot x86 PAE mode also may exceed 32-bit length.
>>> Will fix it.
>>>
>>
>> Though perhaps you've already noticed, both start_pfn and end_pfn in
>> kdump sub
>> header have 32-bit length on x86_32. These need to be extended to 64-bit
>> length,
>> too.
>>
>> /*
>> * Sub header for KDUMP
>> * But Common header of KDUMP is disk_dump_header of diskdump.
>> */
>> struct kdump_sub_header {
>> unsigned long phys_base;
>> int dump_level; /* header_version 1 and later */
>> int split; /* header_version 2 and later */
>> unsigned long start_pfn; /* header_version 2 and later */
>> unsigned long end_pfn; /* header_version 2 and later */
>> off_t offset_vmcoreinfo;/* header_version 3 and later */
>> unsigned long size_vmcoreinfo; /* header_version 3 and later */
>> off_t offset_note; /* header_version 4 and later */
>> unsigned long size_note; /* header_version 4 and later */
>> off_t offset_eraseinfo; /* header_version 5 and later */
>> unsigned long size_eraseinfo; /* header_version 5 and later */
>> };
>>
>
> Thanks for your reminder.
> Yes, I'm going to add another two new 64-bit values for them as well as max_mapnr.
> How about these names?
> unsigned long long start_pfn_64;
> unsigned long long end_pfn_64;
> unsigned long long max_mapnr_64;
>
> BTW, Could you give me any suggestions about the the max_mapnr in sadump header.
> Currently, it defined as below:
> struct sadump_header {
> ...
>          uint32_t max_mapnr;     /* = max_mapnr */
> ...
> };
> And there is no any sub-header in sadump format. How would you like to keep the backward compatibility?
> I can add a new unit64_t max_mapnr_64 in this sadump_header but it will change the header size. It should work if I add some code to check header_version for sadump format, and for old format just assign the value of max_mapnr to max_mapnr_64.
>
>

Thanks. But you don't need to change sadump format. The format is used
by Fujitsu sadump that is firmware-based crash dump mechanism. Currently
we don't have system with more than 16TiB memory. So, there's no issue in
sadump format now.

Also, this format is maintained by firmware developers differnet from me.
The time I'll modify the format will be after they changes their firmware
implementation.

Anyway, you don't need to care about definition of sadump format now.

-- 
Thanks.
HATAYAMA, Daisuke


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

Re: [PATCH] makedumpfile: fix max_mapnr issue on system has over 44-bit addressing

[Jingbai Ma]

On 10/08/2013 04:34 PM, HATAYAMA Daisuke wrote:
> (2013/10/08 16:26), Jingbai Ma wrote:
>> On 10/08/2013 01:41 PM, HATAYAMA Daisuke wrote:
>>> (2013/09/25 17:16), Jingbai Ma wrote:
>>>> On 09/25/2013 08:35 AM, HATAYAMA Daisuke wrote:
>>>>> (2013/09/24 21:49), Jingbai Ma wrote:
>>>>>> This patch will fix a bug of makedumpfile doesn't work correctly on
>>>>>> system
>>>>>> has over 44-bit addressing in compression dump mode.
>>>>>> This bug was posted here:
>>>>>> http://lists.infradead.org/pipermail/kexec/2013-September/009587.html
>>>>>>
>>>>>> This patch will add a new field in struct kdump_sub_header.
>>>>>> unsigned long max_mapnr;
>>>>>>
>>>>>> And the old "unsigned int max_mapnr" in struct disk_dump_header will
>>>>>> not be used anymore. But still be there for compatibility purpose.
>>>>>>
>>>>>> This patch will change the header_version to 6.
>>>>>>
>>>>>> The corresponding patch for crash utility will be sent out
>>>>>> separately.
>>>>>>
>>>>>> This patch doesn't change sadump_header.
>>>>>> Because of in sadump file, there is no any sub-header, it has to
>>>>>> change
>>>>>> the sadump_header itself.
>>>>>> And if do so, will cause backwards-compatibility issue.
>>>>>> So it could be a separate patch if needed.
>>>>>>
>>>>>> Signed-off-by: Jingbai Ma <jingbai.ma@hp.com>
>>>>>> ---
>>>>>> IMPLEMENTATION | 1 +
>>>>>> diskdump_mod.h | 5 ++++-
>>>>>> makedumpfile.c | 28 ++++++++++++++++++++++------
>>>>>> 3 files changed, 27 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/IMPLEMENTATION b/IMPLEMENTATION
>>>>>> index f0f3135..d576811 100644
>>>>>> --- a/IMPLEMENTATION
>>>>>> +++ b/IMPLEMENTATION
>>>>>> @@ -77,6 +77,7 @@
>>>>>> unsigned long size_note; /* header_version 4 and later */
>>>>>> off_t offset_eraseinfo; /* header_version 5 and later */
>>>>>> unsigned long size_eraseinfo; /* header_version 5 and later */
>>>>>> + unsigned long max_mapnr; /* header_version 6 and later */
>>>>>
>>>>> x86 PAE mode can represents 52-bit physical addresses and so 40-bit
>>>>> physical
>>>>> page frames. On x86_32 unsigned long has 32-bit length only. So, you
>>>>> should
>>>>> define max_mapnr as unsigned long long.
>>>>>
>>>>
>>>> Good catch, I forgot x86 PAE mode also may exceed 32-bit length.
>>>> Will fix it.
>>>>
>>>
>>> Though perhaps you've already noticed, both start_pfn and end_pfn in
>>> kdump sub
>>> header have 32-bit length on x86_32. These need to be extended to 64-bit
>>> length,
>>> too.
>>>
>>> /*
>>> * Sub header for KDUMP
>>> * But Common header of KDUMP is disk_dump_header of diskdump.
>>> */
>>> struct kdump_sub_header {
>>> unsigned long phys_base;
>>> int dump_level; /* header_version 1 and later */
>>> int split; /* header_version 2 and later */
>>> unsigned long start_pfn; /* header_version 2 and later */
>>> unsigned long end_pfn; /* header_version 2 and later */
>>> off_t offset_vmcoreinfo;/* header_version 3 and later */
>>> unsigned long size_vmcoreinfo; /* header_version 3 and later */
>>> off_t offset_note; /* header_version 4 and later */
>>> unsigned long size_note; /* header_version 4 and later */
>>> off_t offset_eraseinfo; /* header_version 5 and later */
>>> unsigned long size_eraseinfo; /* header_version 5 and later */
>>> };
>>>
>>
>> Thanks for your reminder.
>> Yes, I'm going to add another two new 64-bit values for them as well
>> as max_mapnr.
>> How about these names?
>> unsigned long long start_pfn_64;
>> unsigned long long end_pfn_64;
>> unsigned long long max_mapnr_64;
>>
>> BTW, Could you give me any suggestions about the the max_mapnr in
>> sadump header.
>> Currently, it defined as below:
>> struct sadump_header {
>> ...
>> uint32_t max_mapnr; /* = max_mapnr */
>> ...
>> };
>> And there is no any sub-header in sadump format. How would you like to
>> keep the backward compatibility?
>> I can add a new unit64_t max_mapnr_64 in this sadump_header but it
>> will change the header size. It should work if I add some code to
>> check header_version for sadump format, and for old format just assign
>> the value of max_mapnr to max_mapnr_64.
>>
>>
>
> Thanks. But you don't need to change sadump format. The format is used
> by Fujitsu sadump that is firmware-based crash dump mechanism. Currently
> we don't have system with more than 16TiB memory. So, there's no issue in
> sadump format now.
>
> Also, this format is maintained by firmware developers differnet from me.
> The time I'll modify the format will be after they changes their firmware
> implementation.
>
> Anyway, you don't need to care about definition of sadump format now.
>

Got it. Thanks for your clarification!


-- 
Thanks,
Jingbai Ma

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