On 2023/05/12 0:32, Rongwei Wang wrote:
>> btw, how can I create this situation seeing zero page easily?
>> I also would like to test the patch.
>
> Here a testcase that I used to allocate zero page:
Thanks! I could see it on x86_64.
crash> vtop -c 638970 7f018fb11000
VIRTUAL PHYSICAL
7f018fb11000 3af648000
PGD: 4646ca7f0 => 5b1ba7067
PUD: 5b1ba7030 => 5f9e63067
PMD: 5f9e633e8 => b45b7d067
PTE: b45b7d888 => 80000003af648225
PAGE: 3af648000 (ZERO PAGE)
>
> z.c:
>
> #include <string.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <errno.h>
> #include <ctype.h>
>
> unsigned long *zero_data;
>
> void read_only(void)
> {
> unsigned long i, val, count = 0;
>
> printf("hello\n");
> while (count++ < 10) {
> for (i=0; i<(4 * 1024 * 1024) / sizeof(unsigned long); i+=1024) {
> val = zero_data[i];
> }
> }
>
> return;
> }
>
> int main(int argc, char *argv[])
> {
> char c;
>
> zero_data = (unsigned long *)malloc(1024 * 1024 * 4);
> if (!zero_data) {
> printf("malloc fail: exit\n");
> return -1;
> }
> printf("zero: %lx\n", zero_data);
>
> read_only();
>
> c = getchar();
>
> free(zero_data);
> exit(EXIT_SUCCESS);
> }
>
> When you test in arm64 with 64k base pagesize, the size of allocating memory is needed to set more than 512MB.
I see.
>
>>
>>>> PTE PHYSICAL FLAGS
>>>> 160000836e71fc3 836e71000 (VALID|USER|RDONLY|SHARED|AF|NG|PXN|UXN|SPECIAL)
>>>>
>>>> VMA START END FLAGS FILE
>>>> ffff000844f51860 ffff8917c000 ffff8957d000 100073
>>>>
>>>> PAGE PHYSICAL MAPPING INDEX CNT FLAGS
>>>> fffffe001fbb9c40 836e71000 0 0 1 2ffffc000001000 reserved
>> a bit verbose, please replace these with "..."
>
> Your mean comment as below is OK?
Yes.
>
> crash> vtop -c 13674 ffff8917e000
> VIRTUAL PHYSICAL
> ffff8917e000 836e71000
>
> PAGE DIRECTORY: ffff000802f8d000
> PGD: ffff000802f8dff8 => 884e29003
> PUD: ffff000844e29ff0 => 884e93003
> PMD: ffff000844e93240 => 840413003
> PTE: ffff000800413bf0 => 160000836e71fc3
> PAGE: 836e71000 (ZERO PAGE)
>
> ...
>
>>>> + if (readmem(symbol_value("huge_zero_pfn"), KVADDR,
>>>> + &huge_zero_pfn, sizeof(huge_zero_pfn),
>>>> + "read huge_zero_pfn", QUIET|RETURN_ON_ERROR))
>>>> + vt->huge_zero_paddr = huge_zero_pfn << PAGESHIFT();
>> In kernel, huge_zero_pfn is initialized as ~0UL and set to a pfn when
>> get_huge_zero_page() is called.
>>
>> crash> p -x huge_zero_pfn
>> huge_zero_pfn = $4 = 0xffffffffffffffff
>>
>> so if huge_zero_pfn is ~0UL, vt->huge_zero_paddr should not be set.
>
> Good catch.
>
> How about following modification:
>
> + if (readmem(symbol_value("huge_zero_pfn"), KVADDR,
> + &huge_zero_pfn, sizeof(huge_zero_pfn),
> + "read huge_zero_pfn", QUIET|RETURN_ON_ERROR))
> + vt->huge_zero_paddr = (huge_zero_pfn != ~0UL) ? (huge_zero_pfn << PAGESHIFT());
hmm, this looks wrong, needs ':'?
I would prefer this simply:
if (readmem(symbol_value("huge_zero_pfn"), KVADDR,
&huge_zero_pfn, sizeof(huge_zero_pfn),
- "read huge_zero_pfn", QUIET|RETURN_ON_ERROR))
+ "read huge_zero_pfn", QUIET|RETURN_ON_ERROR) &&
+ huge_zero_pfn != ~0UL)
vt->huge_zero_paddr = huge_zero_pfn << PAGESHIFT();
>
>>
>>>> + }
>>>> +
>>>> if (vt->flags & PERCPU_KMALLOC_V1)
>>>> vt->dump_kmem_cache = dump_kmem_cache_percpu_v1;
>>>> else if (vt->flags & PERCPU_KMALLOC_V2)
>>>> diff --git a/x86_64.c b/x86_64.c
>>>> index 5019c69..693a08b 100644
>>>> --- a/x86_64.c
>>>> +++ b/x86_64.c
>>>> @@ -2114,8 +2114,9 @@ x86_64_uvtop_level4(struct task_context *tc, ulong uvaddr, physaddr_t *paddr, in
>>>> goto no_upage;
>>>> if (pmd_pte & _PAGE_PSE) {
>>>> if (verbose) {
>>>> - fprintf(fp, " PAGE: %lx (2MB)\n\n",
>>>> - PAGEBASE(pmd_pte) & PHYSICAL_PAGE_MASK);
>>>> + fprintf(fp, " PAGE: %lx (2MB%s)\n\n",
>>>> + PAGEBASE(pmd_pte) & PHYSICAL_PAGE_MASK,
>>>> + IS_ZEROPAGE(PAGEBASE(pmd_pte) & PHYSICAL_PAGE_MASK) ? ", ZERO PAGE" : "");
>>>> x86_64_translate_pte(pmd_pte, 0, 0);
>>>> }
>>>> @@ -2143,8 +2144,8 @@ x86_64_uvtop_level4(struct task_context *tc, ulong uvaddr, physaddr_t *paddr, in
>>>> *paddr = (PAGEBASE(pte) & PHYSICAL_PAGE_MASK) + PAGEOFFSET(uvaddr);
>>>> if (verbose) {
>>>> - fprintf(fp, " PAGE: %lx\n\n",
>>>> - PAGEBASE(*paddr) & PHYSICAL_PAGE_MASK);
>>>> + fprintf(fp, " PAGE: %lx %s\n\n", PAGEBASE(*paddr) & PHYSICAL_PAGE_MASK,
>>>> + IS_ZEROPAGE(PAGEBASE(*paddr) & PHYSICAL_PAGE_MASK) ? "(ZERO PAGE)" : "");
>>>> x86_64_translate_pte(pte, 0, 0);
>>>> }
>> why only for uvtop_level4?
>
> That because I have no idea about xyz_xen_wpt and similar target. In addition, I have no environment to test that, so...
>
> How about doing this when someone need it in these environment? Or I can do it if you think just complete it directly is ok.
Ah ok, I see. Your patch is good enough for now.
Thanks,
Kazu
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
