[Crash-utility] Re: [PATCH 2/2] Add vmemmap support

thanks for the patch. On 2023/11/28 12:41, Huang Shijie wrote: I recalled that I also did a similar thing for x86_64 [1] in the past.. so it's not so slow on x86_64 and it has valid section check too. [1] https://github.com/crash-utility/crash/commit/4141373d9de3fea29f5d2b58f60... I don't object to add an entry to vmcoreinfo as a way of getting the vmemmap value. Seems like arm64 has another value from VMEMMAP_START. but there are some concerns in crash: - machdep->flags already has VMEMMAP flag. It's used like "if (IS_SPARSEMEM() && (machdep->flags & VMEMMAP)" e.g. [1]. Adding the new flag is very confusing and inconsistent. - Do all architectures have VMEMMAP_VADDR/END values in crash with CONFIG_SPARSEMEM_VMEMMAP configured? we cannot test all archs, so this kind of arch-independent change makes me nervous. I would like to suggest an arch-specific use first. e.g. how about making x86_64_is_page_ptr more generic and use it? What happens if you use it on arm64? maybe the valid section can check it with VMEMMAP_VADDR without the vmemmap value. Just an idea though. Thanks, Kazu > > If we have vmemmap then we can implement fast page_to_pfn code in > vmemmap_is_page_ptr > > Test result: > Test the patch: "[PATCH v3] add "files -n" command for an inode" > https://lists.crash-utility.osci.io/archives/list/ > devel(a)lists.crash-utility.osci.io/thread/CRLOQP534YKEQPTCXIWVDYK4NA7BOSUK/ > > Without the this patch: > #files -n xxx (xxx is the inode of vmlinux which is 441M) > This costed about 162 seconds. > > With the this patch: > #files -n xxx (xxx is the inode of vmlinux which is 441M) > This costed less then 1 second. > > Signed-off-by: Huang Shijie <shijie(a)os.amperecomputing.com&gt; > --- > defs.h | 3 +++ > memory.c | 35 +++++++++++++++++++++++++++++++++++ > 2 files changed, 38 insertions(+) > > diff --git a/defs.h b/defs.h > index eec7b3e..25bb455 100644 > --- a/defs.h > +++ b/defs.h > @@ -2615,6 +2615,7 @@ struct vm_table { /* kernel VM-related data */ > ulong vma_cache_fills; > void *mem_sec; > char *mem_section; > + ulong vmemmap; > int ZONE_HIGHMEM; > ulong *node_online_map; > int node_online_map_len; > @@ -2665,10 +2666,12 @@ struct vm_table { /* kernel VM-related data */ > #define SLAB_OVERLOAD_PAGE (0x8000000) > #define SLAB_CPU_CACHE (0x10000000) > #define SLAB_ROOT_CACHES (0x20000000) > +#define SPARSEMEM_VMEMMAP (0x40000000) > > #define IS_FLATMEM() (vt->flags & FLATMEM) > #define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM) > #define IS_SPARSEMEM() (vt->flags & SPARSEMEM) > +#define IS_SPARSEMEM_VMEMMAP() (vt->flags & SPARSEMEM_VMEMMAP) > #define IS_SPARSEMEM_EX() (vt->flags & SPARSEMEM_EX) > > #define COMMON_VADDR_SPACE() (vt->flags & COMMON_VADDR) > diff --git a/memory.c b/memory.c > index ed1a4fb..668567f 100644 > --- a/memory.c > +++ b/memory.c > @@ -17462,6 +17462,39 @@ sparse_decode_mem_map(ulong coded_mem_map, ulong section_nr) > (section_nr_to_pfn(section_nr) * SIZE(page)); > } > > +static int > +vmemmap_is_page_ptr(ulong addr, physaddr_t *phys) > +{ > + ulong pfn; > + > + if (IS_SPARSEMEM_VMEMMAP() && > + (VMEMMAP_VADDR <= addr && addr < VMEMMAP_END)) { > + ulong size = SIZE(page); > + > + /* Not aligned with the page structure */ > + if ((addr - vt->vmemmap) % size) > + return FALSE; > + > + pfn = (addr - vt->vmemmap) / size; > + if (phys) > + *phys = PTOB(pfn); > + return TRUE; > + } > + return FALSE; > +} > + > +static bool > +check_sparsemem_vmemmap(void) > +{ > + if (!get_value_vmcore("SYMBOL(vmemmap)", &vt->vmemmap)) > + return FALSE; > + > + vt->flags |= SPARSEMEM_VMEMMAP; > + if (machdep->is_page_ptr == generic_is_page_ptr) > + machdep->is_page_ptr = vmemmap_is_page_ptr; > + return TRUE; > +} > + > void > sparse_mem_init(void) > { > @@ -17472,6 +17505,8 @@ sparse_mem_init(void) > if (!IS_SPARSEMEM()) > return; > > + check_sparsemem_vmemmap(); > + > MEMBER_OFFSET_INIT(mem_section_section_mem_map, "mem_section", > "section_mem_map"); >