[Crash-utility] [PATCH] Add support for 'foreign' page sizes in kdump dumps

Dave Anderson anderson at redhat.com
Wed Jan 14 21:17:26 UTC 2009


----- "Bernhard Walle" <bwalle at suse.de> wrote:

> This affects only ppc64 and ia64 since that are the only architectures
> that have a page size that is configurable at runtime by the system (which means
> at boot by the kernel).
> 
> It also only affects dumps of the formats diskdump (which includes kdump
> compressed dumps created by makedumpfile) and netdump (which includes kdump
> ELF dumps copied from /proc/vmcore without any filtering applied and created
> by makedumpfile with the -E option).
> 
> The patch reads the page size from the diskdump header or from the VMCOREINFO
> in case of netdump (if it's there). For ia64 it also evaluates the page size
> of the zero page to *change* the page size. In the past id didn't change the
> page size, it only printed an error.
> 
> The patch has been tested on ppc64 (4k vs. 64k), ia64 (16k vs. 64k) and x86-64
> (always 4k). It has been tested for compilation on i386, x86-64, ia64, PPC,
> ppc64, s390 and s390x. Everything on a SLES 11/openSUSE 11.1 code base.
> 
> 
> Signed-off-by: Bernhard Walle <bwalle at suse.de>

For starters, I've got a few issues with this patch (in addition to the earlier
diskdump.c question):

(1) Your patch would only work with 32-bit ELF kdumps since the vmcoreinfo_read_integer()
    function is only called by dump_Elf32_Nhdr() -- and not by dump_Elf64_Nhdr().
    I wouldn't think that ppc64 and ia64 could even create 32-bit ELF kdumps since
    they would by definition have to truncate certain fields?  I presume you either
    forgot the 64-bit part of the patch, or maybe incorrectly put the code in the
    wrong function?

(2) Even so, in the dump_Elf32_Nhdr() switch statement, you're taking over the
    "0" case, whereas it used to be picked up by the "default" case -- which
    currently dumps the vmcoreinfo data.  Now that code can never be run, and 
    I don't want to give up that capability.

(3) I think I'd like to keep it forcibly segregated to ia64 and ppc64, instead of
    letting the other architectures even call vmcoreinfo_read_integer(PAGESIZE).
    I don't want to even allow a *remote* possibility of breaking things on the 
    other arches. 

(4) vmcore_info_read_integer() shouldn't be called if the "store" paramter isn't set.

(5) Minor nit -- I'd just as soon leave any changes to ia64_init_hyper() to its
    maintainer, especially since it makes no sense to make a follow-up call to
    ia64_check_adjust_pagesize().     

Thanks,
  Dave




More information about the Crash-utility mailing list