[Crash-utility] [PATCH] Add support for 'foreign' page sizes in kdump dumps
Dave Anderson
anderson at redhat.com
Wed Jan 14 21:32:12 UTC 2009
----- "Dave Anderson" <anderson at redhat.com> wrote:
>
> 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
Now I'm confused -- since the patches to diskdump.c and netdump.c properly set the
page size for ia64, why is there any need to change ia64_init()? If anything,
that code could only end up over-writing the pre-determined values returned by
diskdump_page_size() and kdump_page_size(), meaning that the diskdump header
info or the vmcoreinfo info is in fact invalid?
Dave
More information about the Crash-utility
mailing list