On 10/27/14 at 03:57pm, Baoquan He wrote: > On 10/23/14 at 06:56am, Atsushi Kumagai wrote: > > >On Tue, 14 Oct 2014 07:19:13 +0000 > > >Atsushi Kumagai <kumagai-atsushi at mxc.nes.nec.co.jp> wrote: > > > > > >[snip] > > > > > >> > > >> I understand why your patch works on s390x, so how about this way ? > > >> > > >> 1. Define "is_phys_addr()" for --mem-usage. > > >> 2. Implement is_phys_addr() using is_iomem_phys_addr() for s390x > > >> while x86_64 uses is_vmalloc_addr_x86_64(). > > >> 3. Use is_phys_addr() instead of is_vmalloc_addr() in get_kcore_dump_loads(). > > > > > >Hello Atsushi, > > > > > >Great, so let's do that. > > > > > >@Baoquan: > > >If you want to use the is_iomem_phys_addr() function also for x86, > > >you could perhaps add an additional patch. > > > > I noticed that is_vmalloc_addr_x86_64() can't be used as is_phys_addr() > > due to the "kaslr issue". I fixed it in the common path as below, but > > --mem-usage still has the same issue since initial() will be invoked > > after get_kcore_dump_loads(). > > > > http://lists.infradead.org/pipermail/kexec/2014-October/012743.html > > > > I know it's so late, but I began to think the current implementation > > that invokes vaddr_to_paddr_XXX() before initial() is bad: > > > > show_mem_usage() > > + get_kcore_dump_loads() > > + process_dump_load() > > + vaddr_to_paddr_XXX() > > + initial() > > This is a bug. Seems that get_kcore_dump_loads() has to be called in > initial(). Since dumped vmcore need contains kernel text segment. > Without setting correct value for MODULES_VADDR, it will be equal to > __START_KERNEL_map, then calling is_vmalloc_addr() will filter kernel > text mapping segment too. > > Now it seems only one way to solve this, that is moving > get_kcore_dump_loads() into initial() just after > get_value_for_old_linux() is called. Hi Atsushi, Checking code again, I found with kaslr support we need move vmcoreinfo reading earlier to initialize the MODULES_VADDR, otherwise get_phys_base_x86_64() will be wrong. Since not initializing MODULES_VADDR explicitly its value is equal to __START_KERNEL_map, that means kernel text mapping segment is filtered by is_vmalloc_addr_x86_64(). So could we move below code earlier? diff --git a/makedumpfile.c b/makedumpfile.c index b27ea1e..1dac19f 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -3092,6 +3092,23 @@ initial(void) return FALSE; } + /* + * Get the debug information from /proc/vmcore. + * NOTE: Don't move this code to the above, because the debugging + * information token by -x/-i option is overwritten by vmcoreinfo + * in /proc/vmcore. vmcoreinfo in /proc/vmcore is more reliable + * than -x/-i option. + */ + if (has_vmcoreinfo()) { + get_vmcoreinfo(&offset, &size); + if (!read_vmcoreinfo_from_vmcore(offset, size, FALSE)) + return FALSE; + debug_info = TRUE; + } + + if (!get_value_for_old_linux()) + return FALSE; + if (info->flag_refiltering) { if (info->flag_elf_dumpfile) { MSG("'-E' option is disable, "); @@ -3189,23 +3206,6 @@ initial(void) } } - /* - * Get the debug information from /proc/vmcore. - * NOTE: Don't move this code to the above, because the debugging - * information token by -x/-i option is overwritten by vmcoreinfo - * in /proc/vmcore. vmcoreinfo in /proc/vmcore is more reliable - * than -x/-i option. - */ - if (has_vmcoreinfo()) { - get_vmcoreinfo(&offset, &size); - if (!read_vmcoreinfo_from_vmcore(offset, size, FALSE)) - return FALSE; - debug_info = TRUE; - } - - if (!get_value_for_old_linux()) - return FALSE; - out: if (!info->page_size) { /*
