Re: [Crash-utility] [PATCH] Add support for 'foreign' page sizes in kdump dumps
----- "Bernhard Walle" <bwalle(a)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(a)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