On 2023/11/30 16:21, HAGIO KAZUHITO(萩尾 一仁) wrote: > thanks for the patch. > > On 2023/11/28 12:41, Huang Shijie wrote: >> If the kernel exports the vmmemap then we can use that symbol in >> crash to optimize access. vmmemap is just an array of page structs >> after all. >> >> This patch tries to get the "vmemmap" from the vmcore file. >> If we can use the "vmemmap", use the vmemmap_is_page_ptr to replace >> the generic_is_page_ptr(). > > 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/4141373d9de3fea29f5d2b58f60e44bc132726c0 > > 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. hmm sorry, probably this does not work, I missed this. + pfn = (addr - vt->vmemmap) / size; but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value on arm64? Thanks, Kazu > > 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@xxxxxxxxxxxxxxxxxxxxxxxxxxx/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@xxxxxxxxxxxxxxxxxxxxxx> >> --- >> 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"); >> > -- > Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx > To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx > https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ > Contribution Guidelines: https://github.com/crash-utility/crash/wiki -- Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki
