Ken'ichi Ohmichi wrote: > Hi Dave, > > 2007/04/02 08:54:38 -0400, Dave Anderson <anderson@xxxxxxxxxx> 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 -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
