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

Jingbai Ma
2013-10-08T07:27:26Z

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