[Crash-utility] handling missing kdump pages in diskdump format

[Ken'ichi Ohmichi]

Hi Dave,

2007/04/05 09:04:53 -0400, Dave Anderson <anderson at redhat.com> wrote:
>> >If this were to be restricted to kdump only, then it would be
>> >safe to add a field to the kdump_sub_header since it contains
>> >only one field.
>> >
>> >Whether it is necessary for diskdump is up to the diskdump
>> >maintainers to decide.  But if they want to do it, they would
>> >have to come up with different manner of adding it to the
>> >disk_dump_sub_header structure, because of the way
>> >that it's declared, i.e., using "long elf_regs" as a placeholder
>> >to describe the start of the processor-dependent register
>> >set.  They'd need to define a new disk_dump_sub_header
>> >version that alternatively would be used depending upon
>> >the header->version number.
>>
>> We asked the diskdump developers (Indoh-san, Akiyama-san) how
>> to display diskdump's dump_level, and they said that changing
>> of file format is not necessary for displaying the dump_level.
>> And they gave us a good advice that the crash utility can get
>> diskdump's dump_level by using a symbol "dump_level" like the
>> following:
>>
>> int get_dump_level(void)
>> {
>>         if (symbol_exists("dump_level")) {
>>                 int dump_level;
>>                 readmem(symbol_value("dump_level"), KVADDR, &dump_level,
>>                         sizeof(dump_level), "dump_level",
>>                         FAULT_ON_ERROR);
>>                 return dump_level;
>>
>>         }
>>         else
>>         ...
>>
>> I think it is a good way because the crash utility can get diskdump's
>> dump_level of the existing dumpfile. I propose the following way for it.
>> Please let me know your opinion.
>>
>> - If a symbol "dump_level" is present (diskdump), the dump_level is
>>   taken from a symbol "dump_level".
>> - Else if a dumpfile is kdump's and header_version is 1 or more, the
>>   dump_level taken from a new member "kdump_sub_header->dump_level".
>
>Both proposals sound good to me.
>
>Just so that I can have it ready-to-go for you in the next crash release,
>do you want to make the additional "dump_level" member an int or
>a long?

I would like to add a member "dump_level" as int, because diskdump's
dump_level is defined as int. If it is defined as long, the crash
utility gets the dump_level with something else on 64bit system.
On x86_64 RHEL4 (2.6.9-42.EL), I tried getting the dump_level (15):

crash> rd dump_level
ffffffffa0140ba4:  fffffffb0000000f                    ........
crash> rd -32 dump_level
ffffffffa0140ba4:  0000000f                              ....
crash>


And I think Bob's proposal (the excluded pages should not be accessed)
is worthy, too. I will add the following changes into makedumpfile.

makedumpfile:
- disk_dump_header->header_version is changed into 1 from 0.
- kdump_sub_header->dump_level is added as int, and makedumpfile sets
  the value into it.
- The page descriptors (struct page_desc) of all the excluded zero-pages
  point same zero-page. The crash utility can read zero-pages, even if
  it cannot accesss the excluded pages.


Could you change the crash utility as the following ?

The crash utility:
- If disk_dump_header->signature is "KDUMP   ", it checks its header_version.
  If it is 1 or more, the dump_level is taken from kdump_sub_header->dump_level,
  and the excluded pages cannot be accessed (by default, or option ?).


Thanks
Ken'ichi Ohmichi