[Crash-utility] Re: [PATCH 2/2] Add vmemmap support
On 2023/11/30 17:26, Shijie Huang wrote:
Hi Kazu,
在 2023/11/30 15:29, HAGIO KAZUHITO(萩尾 一仁) 写道:
> 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.
Maybe your vmlinux is not so big as mine(441M)...
It's not so small :)
crash> files | grep -m1 vmlinux
5 ffff97c21fa2cf00 ffff97c2400e5200 ffff97c0f5cf5ec8 REG
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> !ls -lh /usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
-rwxr-xr-x. 3 root root 826M Apr 29 2021
/usr/lib/debug/usr/lib/modules/4.18.0-305.el8.x86_64/vmlinux
crash> files -n ffff97c0f5cf5ec8 | ts.py
16:42:30.902 START
16:42:30.902 INODE NRPAGES
16:42:30.902 ffff97c0f5cf5ec8 211283
16:42:30.902
16:42:31.106 NODE PAGES
16:42:31.106 0 211283
16:42:31.106 END (ELAPS 0:00:00.204)
(This crash has your "files -n" v2 patch and is_page_ptr() check at the
beginning of summary_inode_page)
>> [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?
yes. your concern are right. I found some arch does not support the
VMEMMAP_VADDR/END.
So it is better to add this into ARM64 first. (I use the arm64 machine
all the time..)
>>
>> 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?
>>
I will add arm64 version code. I am not familar with x86, and I do not
have x86 machine for the testing.
so it's better to not change it.
OK, I see.
>> 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;
The code is converted by the linux code:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/i...
#define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
So it's correct.
>
> but can VMEMMAP_VADDR and phys_offset can be used to get vmemmap value
> on arm64?
On arm64, I can get vmemmap by parsing "memstart_addr".
But I cannot know if CONFIG_SPARSEMEM_VMEMMAP is enabled.
on x86_64, vmemmap_populate is used:
if (symbol_exists("vmemmap_populate"))
machdep->flags |= VMEMMAP;
on arm64, it's set unconditionally, is it ok?
arm64_init(int when)
{
...
case PRE_GDB:
...
machdep->stacksize = ARM64_STACK_SIZE;
machdep->flags |= VMEMMAP;
So if the kernel can export "vmemmap", everything become easy.
All the archs supporting the VMEMMAP can benifit from it, such as arm64,
ppc,loongarch,riscv..
Yes but we're not sure whether the users of the other archs have the
same issue (e.g. x86_64 is fast enough) and it's hard to test all archs.
Also, crash cannot always access vmcoreinfo (e.g. ramdumps), basically
it's better to implement without depending on vmcoreinfo first if
possible. If there is a case where crash cannot work without
vmcoreinfo, then we will add a entry though.
Thanks,
Kazu