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

[Jingbai Ma <jingbai.ma@hp.com>]

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