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

[Dave Anderson]

Ken'ichi Ohmichi wrote:

> Hi Dave,
>
> 2007/04/02 08:54:38 -0400, Dave Anderson <anderson at redhat.com> wrote:
> >> I think it is good that the crash utility can know the dump_level
> >> of a dumpfile. But it is not good that dump_level is merged into
> >> diskdump->header_version, because one member should represent one
> >> meaning. I checked the code of crash-4.0-3.21, and I noticed
> >> diskdump->total_ram_blocks is not referred by the crash utility
> >> though the diskdump module sets the value into it. And makedumpfile
> >> sets 0 into it. It does not have any meaning.
> >>
> >> Can this member be changed for dump_level ?
> >> I think the following methods for it:
> >>   - If the crash utility reads a dumpfile, it checks diskdump->
> >>     header_version.
> >>   - If diskdump->header_version is 1 or more, the crash utility
> >>     considers diskdump->total_ram_blocks as dump_level.
> >>
> >> By the way, the dump_level of diskdump is different from the one
> >> of kdump (makedumpfile). If the crash utility will be able to
> >> display the excluded page-type, we should note it.
> >>
> >> The dump_level of diskdump
> >>   1: Excluding cache pages with private pages
> >>   2: Excluding zero-filled pages
> >>   4: Excluding free pages
> >>   8: Excluding user process data pages
> >>  16: Saving private pages
> >>
> >> The dump_level of kdump (makedumpfile)
> >>   1: Excluding zero-filled pages
> >>   2: Excluding cache pages without private pages
> >>   4: Excluding cache pages with private pages
> >>   8: Excluding user process data pages
> >>  16: Excluding free pages
> >>
> >> Thanks
> >> Ken'ichi Ohmichi
> >
> >Sounds good to me.  I don't have any plans for the translation
> >of the dump_level, but only a display of its value in a new "help -n"
> >function that I've written for diskdump dumpfiles.  "help -n" shows
> >netdump, ELF-diskdump, kdump, LKCD and xendump dumpfile
> >information, but functions for diskdump and s390 dumps were
> >never written.
>
> It is a good idea that "help -n" shows dump_level.
>
> I checked the code of diskdumputils-1.3.25, and I found the dumpfilter
> command refers to diskdump->total_ram_blocks. In struct disk_dump_header,
> there is no member not referred to by neither the crash utility or diskdumputils.
> My proposal (changing total_ram_blocks for dump_level) was not good.
>
> I think it is enough that the member for dump_level is added into the
> sub header of kdump (struct kdump_sub_header) only for kdump.
> Is the change for diskdump necessary, too ?
>
>

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.

Or again, since header->version has plenty of space,
I still think it makes just as much sense to utilize the
upper bits for something useful:

> But it is not good that dump_level is merged into
> diskdump->header_version, because one member should represent one
> meaning

I agree with the general statement, but until now, the version
field been completely useless, without any "meaning".

But that's just my opinion, and I understand your hesitation at
doing it that way.  Just let me know what you're planning to do...

Thanks,
   Dave