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. > ... > > vaddr_to_paddr_XXX() does VtoP translation *properly*, so it needs > several symbols. This design is OK since it's a general method. > OTOH, we need a kludge which can be used under any situations and > should use it for --mem-usage: > > VtoP_kludge_s390x(unsigned long vaddr) > { > /* s390 has 1:1 VtoP mapping that starts with zero. */ > return vaddr; > } > > also x86_64's can be implemented like below: > > VtoP_kludge_x86_64(unsigned long vaddr) > { > /* if the address is direct mapped, it's easy to translate. */ > unsigned long paddr = vaddr - PAGE_OFFSET; > return paddr; > } > > >Here the updated patch: > >--- > >[PATCH] makedumpfile: Enable --mem-usage for s390x > > > >Replace is_vmalloc_addr() by is_phys_addr() and implement is_phys_addr() > >using /proc/iommem parsing to enable the new makedumpfile option "--mem-usage" > >on s390x. > > > >Signed-off-by: Michael Holzheu <holzheu at linux.vnet.ibm.com> > >--- > > elf_info.c | 4 ++-- > > makedumpfile.c | 26 ++++++++++++++++++++++++++ > > makedumpfile.h | 15 ++++++++------- > > 3 files changed, 36 insertions(+), 9 deletions(-) > > > >--- a/elf_info.c > >+++ b/elf_info.c > >@@ -854,7 +854,7 @@ int get_kcore_dump_loads(void) > > > > for (i = 0; i < num_pt_loads; ++i) { > > struct pt_load_segment *p = &pt_loads[i]; > >- if (is_vmalloc_addr(p->virt_start)) > >+ if (!is_phys_addr(p->virt_start)) > > This part implicitly does VtoP translation. > Actually it works for s390x but it's not suitable as a general code, > so we should use VtoP_kludge(should be better name!) too. > Then we can use is_iomem_phys_addr() also for other architecture. > > > Thanks, > Atsushi Kumagai > > > continue; > > loads++; > > } > >@@ -874,7 +874,7 @@ int get_kcore_dump_loads(void) > > > > for (i = 0, j = 0; i < num_pt_loads; ++i) { > > struct pt_load_segment *p = &pt_loads[i]; > >- if (is_vmalloc_addr(p->virt_start)) > >+ if (!is_phys_addr(p->virt_start)) > > continue; > > if (j >= loads) > > return FALSE; > >--- a/makedumpfile.c > >+++ b/makedumpfile.c > >@@ -9227,6 +9227,32 @@ int is_crashkernel_mem_reserved(void) > > return !!crash_reserved_mem_nr; > > } > > > >+struct addr_check { > >+ unsigned long addr; > >+ int found; > >+}; > >+ > >+static int phys_addr_callback(void *data, int nr, char *str, > >+ unsigned long base, unsigned long length) > >+{ > >+ struct addr_check *addr_check = data; > >+ unsigned long addr = addr_check->addr; > >+ > >+ if (addr >= base && addr < base + length) { > >+ addr_check->found = 1; > >+ return -1; > >+ } > >+ return 0; > >+} > >+ > >+int is_iomem_phys_addr(unsigned long addr) > >+{ > >+ struct addr_check addr_check = {addr, 0}; > >+ > >+ iomem_for_each_line("System RAM\n", phys_addr_callback, &addr_check); > >+ return addr_check.found; > >+} > >+ > > static int get_page_offset(void) > > { > > struct utsname utsname; > >--- a/makedumpfile.h > >+++ b/makedumpfile.h > >@@ -765,7 +765,7 @@ unsigned long long vaddr_to_paddr_arm(un > > #define get_machdep_info() get_machdep_info_arm() > > #define get_versiondep_info() TRUE > > #define vaddr_to_paddr(X) vaddr_to_paddr_arm(X) > >-#define is_vmalloc_addr(X) TRUE > >+#define is_phys_addr(X) TRUE > > #endif /* arm */ > > > > #ifdef __x86__ > >@@ -776,7 +776,7 @@ unsigned long long vaddr_to_paddr_x86(un > > #define get_machdep_info() get_machdep_info_x86() > > #define get_versiondep_info() get_versiondep_info_x86() > > #define vaddr_to_paddr(X) vaddr_to_paddr_x86(X) > >-#define is_vmalloc_addr(X) TRUE > >+#define is_phys_addr(X) TRUE > > #endif /* x86 */ > > > > #ifdef __x86_64__ > >@@ -789,7 +789,7 @@ unsigned long long vaddr_to_paddr_x86_64 > > #define get_machdep_info() get_machdep_info_x86_64() > > #define get_versiondep_info() get_versiondep_info_x86_64() > > #define vaddr_to_paddr(X) vaddr_to_paddr_x86_64(X) > >-#define is_vmalloc_addr(X) is_vmalloc_addr_x86_64(X) > >+#define is_phys_addr(X) (!is_vmalloc_addr_x86_64(X) > > #endif /* x86_64 */ > > > > #ifdef __powerpc64__ /* powerpc64 */ > >@@ -800,7 +800,7 @@ unsigned long long vaddr_to_paddr_ppc64( > > #define get_machdep_info() get_machdep_info_ppc64() > > #define get_versiondep_info() get_versiondep_info_ppc64() > > #define vaddr_to_paddr(X) vaddr_to_paddr_ppc64(X) > >-#define is_vmalloc_addr(X) TRUE > >+#define is_phys_addr(X) TRUE > > #endif /* powerpc64 */ > > > > #ifdef __powerpc32__ /* powerpc32 */ > >@@ -810,7 +810,7 @@ unsigned long long vaddr_to_paddr_ppc(un > > #define get_machdep_info() get_machdep_info_ppc() > > #define get_versiondep_info() TRUE > > #define vaddr_to_paddr(X) vaddr_to_paddr_ppc(X) > >-#define is_vmalloc_addr(X) TRUE > >+#define is_phys_addr(X) TRUE > > #endif /* powerpc32 */ > > > > #ifdef __s390x__ /* s390x */ > >@@ -820,7 +820,7 @@ unsigned long long vaddr_to_paddr_s390x( > > #define get_machdep_info() get_machdep_info_s390x() > > #define get_versiondep_info() TRUE > > #define vaddr_to_paddr(X) vaddr_to_paddr_s390x(X) > >-#define is_vmalloc_addr(X) TRUE > >+#define is_phys_addr(X) is_iomem_phys_addr(X) > > #endif /* s390x */ > > > > #ifdef __ia64__ /* ia64 */ > >@@ -832,7 +832,7 @@ unsigned long long vaddr_to_paddr_ia64(u > > #define get_versiondep_info() TRUE > > #define vaddr_to_paddr(X) vaddr_to_paddr_ia64(X) > > #define VADDR_REGION(X) (((unsigned long)(X)) >> REGION_SHIFT) > >-#define is_vmalloc_addr(X) TRUE > >+#define is_phys_addr(X) TRUE > > #endif /* ia64 */ > > > > typedef unsigned long long mdf_pfn_t; > >@@ -1567,6 +1567,7 @@ int read_disk_dump_header(struct disk_du > > int read_kdump_sub_header(struct kdump_sub_header *kh, char *filename); > > void close_vmcoreinfo(void); > > int close_files_for_creating_dumpfile(void); > >+int is_iomem_phys_addr(unsigned long addr); > > > > > > /* >
