Re: [PATCH v2] arm64/x86_64: show zero pfn information when using vtop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2023/5/15 10:31, HAGIO KAZUHITO(萩尾 一仁) wrote:
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 ':'?
Oh, that's my fault.

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();

OK, will do.


Thanks for your time!

-wrw


+    }
+
        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




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux