Hi Tao,
Thanks for your review.
> 
> Thanks for the fix, however there are several issues:
> 
> On Mon, Sep 2, 2024 at 2:30 AM <qiwu.chen(a)transsion.com&gt; wrote:
> 
> The new members for the offset_table should always be appended to the
> end of the struct.
> 
> 
> 1. The function naming better be page_slab() instead of PageSlab.
> 
> 2. For the 1st "if", the page_type is not used, only used in the
> "else" branch. But page_type will always be readmem() previously, also
> page_type may not exist for older kernels. Can you rewrite the
> function?
> 
> 
> There is a problem for readmem(page_page_type), the page_type member
> of struct page is not introduced for kernels such as v4.17 and prior.
> E.g for 3.10:
> 
> crash> kmem -s ffff880169b30000
> 
> kmem: invalid structure member offset: page_page_type
>       FILE: memory.c  LINE: 9780  FUNCTION: vaddr_to_kmem_cache()
> 
> [/root/crash-dev/crash] error trace: 530063 => 55e269 => 53f5d8 => 63740e
> 
>   63740e: OFFSET_verify+164
>   53f5d8: vaddr_to_kmem_cache+299
>   55e269: dump_kmem_cache_slub+688
>   530063: cmd_kmem+3017
> 
> kmem: invalid structure member offset: page_page_type
>       FILE: memory.c  LINE: 9780  FUNCTION: vaddr_to_kmem_cache()
> 
> Also see the comment for function PageSlab.
> 
I make a improvement in the following change as your comments.  The page_slab() 
function will resolve the problem for readmem(page_page_type), since it will  
test the page_type member of struct page exist before readmem(page_page_type).

diff --git a/defs.h b/defs.h
index 2231cb6..e2a9278 100644
--- a/defs.h
+++ b/defs.h
@@ -2243,6 +2243,7 @@ struct offset_table {                    /* stash of 
commonly-used offsets */
        long vmap_node_busy;
        long rb_list_head;
        long file_f_inode;
+       long page_page_type;
 };
 
 struct size_table {         /* stash of commonly-used sizes */
@@ -2651,6 +2652,7 @@ struct vm_table {                /* kernel VM-related 
data */
        ulong max_mem_section_nr;
        ulong zero_paddr;
        ulong huge_zero_paddr;
+       uint page_type_base;
 };
 
 #define NODES                       (0x1)
@@ -2684,6 +2686,11 @@ struct vm_table {                /* kernel VM-related 
data */
 #define SLAB_CPU_CACHE       (0x10000000)
 #define SLAB_ROOT_CACHES     (0x20000000)
 #define USE_VMAP_NODES       (0x40000000)
+/*
+ * The SLAB_PAGEFLAGS flag is introduced to detect the change of
+ * PG_slab's type from a page flag to a page type.
+ */
+#define SLAB_PAGEFLAGS       (0x80000000)
 
 #define IS_FLATMEM()           (vt->flags & FLATMEM)
 #define IS_DISCONTIGMEM()      (vt->flags & DISCONTIGMEM)
diff --git a/memory.c b/memory.c
index 967a9cf..a2885bd 100644
--- a/memory.c
+++ b/memory.c
@@ -351,6 +351,43 @@ static ulong handle_each_vm_area(struct 
handle_each_vm_area_args *);
 
 static ulong DISPLAY_DEFAULT;
 
+#define PAGE_TYPE_BASE 0xf0000000
+#define PageType(page_type, flag)                                              
\
+       ((page_type & (vt->page_type_base | flag)) == vt->page_type_base)
+
+static void page_type_init(void)
+{
+       /*
+        * The page type macros covert into enum since kernel commit 
ff202303c398e,
+        * use the default macro value if the vmcoreinfo without enum pagetype.
+        */
+       if (!enumerator_value("PAGE_TYPE_BASE", (long *)&vt->page_type_base))
+               vt->page_type_base = PAGE_TYPE_BASE;
+}
+
+/*
+ * The PG_slab's type has changed from a page flag to a page type
+ * since kernel commit 46df8e73a4a3.
+ */
+static bool page_slab(ulong pp, ulong flags)
+{
+       if (vt->flags & SLAB_PAGEFLAGS) {
+               if ((flags >> vt->PG_slab) & 1)
+                       return TRUE;
+       }
+
+       if (VALID_MEMBER(page_page_type)) {
+               uint page_type;
+
+               readmem(pp+OFFSET(page_page_type), KVADDR, &page_type,
+                       sizeof(page_type), "page_type", FAULT_ON_ERROR);
+               if (PageType(page_type, (uint)vt->PG_slab))
+                       return TRUE;
+       }
+
+       return FALSE;
+}
+
 /*
  *  Verify that the sizeof the primitive types are reasonable.
  */
@@ -504,6 +541,10 @@ vm_init(void)
                ANON_MEMBER_OFFSET_INIT(page_compound_head, "page", 
"compound_head");
        MEMBER_OFFSET_INIT(page_private, "page", "private");
        MEMBER_OFFSET_INIT(page_freelist, "page", "freelist");
+       if (MEMBER_EXISTS("page", "page_type")) {
+               MEMBER_OFFSET_INIT(page_page_type, "page", "page_type");
+               page_type_init();
+       }
 
        MEMBER_OFFSET_INIT(mm_struct_pgd, "mm_struct", "pgd");
 
@@ -5931,7 +5972,7 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
                                        if ((flags >> v22_PG_Slab) & 1) 
                                                slabs++;
                                } else if (vt->PG_slab) {
-                                       if ((flags >> vt->PG_slab) & 1) 
+                                       if (page_slab(pp, flags))
                                                slabs++;
                                } else {
                                        if ((flags >> v24_PG_slab) & 1) 
@@ -6381,7 +6422,7 @@ dump_mem_map(struct meminfo *mi)
                                        if ((flags >> v22_PG_Slab) & 1) 
                                                slabs++;
                                } else if (vt->PG_slab) {
-                                       if ((flags >> vt->PG_slab) & 1) 
+                                       if (page_slab(pp, flags))
                                                slabs++;
                                } else {
                                        if ((flags >> v24_PG_slab) & 1) 
@@ -6775,6 +6816,9 @@ page_flags_init_from_pageflag_names(void)
                vt->pageflags_data[i].name = nameptr;
                vt->pageflags_data[i].mask = mask;
 
+               if (!strncmp(nameptr, "slab", 4))
+                       vt->flags |= SLAB_PAGEFLAGS;
+
                if (CRASHDEBUG(1)) {
                        fprintf(fp, "  %08lx %s\n", 
                                vt->pageflags_data[i].mask,
@@ -6836,7 +6880,8 @@ page_flags_init_from_pageflags_enum(void)
                        strcpy(nameptr, arglist[0] + strlen("PG_"));
                        vt->pageflags_data[p].name = nameptr;
                        vt->pageflags_data[p].mask = 1 << atoi(arglist[2]); 
-
+                       if (!strncmp(nameptr, "slab", 4))
+                               vt->flags |= SLAB_PAGEFLAGS;
                        p++;
                }
        } else 
@@ -9736,14 +9781,14 @@ vaddr_to_kmem_cache(ulong vaddr, char *buf, int verbose)
                readmem(page+OFFSET(page_flags), KVADDR,
                        &page_flags, sizeof(ulong), "page.flags",
                        FAULT_ON_ERROR);
-               if (!(page_flags & (1 << vt->PG_slab))) {
+               if (!page_slab(page, page_flags)) {
                        if (((vt->flags & KMALLOC_SLUB) || 
VALID_MEMBER(page_compound_head)) ||
                            ((vt->flags & KMALLOC_COMMON) &&
                            VALID_MEMBER(page_slab) && 
VALID_MEMBER(page_first_page))) {
                                readmem(compound_head(page)+OFFSET(page_flags), 
KVADDR,
                                        &page_flags, sizeof(ulong), 
"page.flags",
                                        FAULT_ON_ERROR);
-                               if (!(page_flags & (1 << vt->PG_slab)))
+                               if (!page_slab(compound_head(page), page_flags))
                                        return NULL;
                        } else
                                return NULL;
@@ -20195,7 +20240,7 @@ char *
 is_slab_page(struct meminfo *si, char *buf)
 {
        int i, cnt;
-       ulong page_slab, page_flags, name;
+       ulong pg_slab, page_flags, name;
         ulong *cache_list;
         char *retval;
 
@@ -20210,11 +20255,11 @@ is_slab_page(struct meminfo *si, char *buf)
            RETURN_ON_ERROR|QUIET))
                return NULL;
 
-       if (!(page_flags & (1 << vt->PG_slab)))
+       if (!page_slab(si->spec_addr, page_flags))
                return NULL;
 
        if (!readmem(si->spec_addr + OFFSET(page_slab), KVADDR, 
-           &page_slab, sizeof(ulong), "page.slab", 
+           &pg_slab, sizeof(ulong), "page.slab", 
            RETURN_ON_ERROR|QUIET))
                return NULL;
 
@@ -20222,7 +20267,7 @@ is_slab_page(struct meminfo *si, char *buf)
         cnt = get_kmem_cache_list(&cache_list);
 
        for (i = 0; i < cnt; i++) {
-               if (page_slab == cache_list[i]) {
+               if (pg_slab == cache_list[i]) {
                        if (!readmem(cache_list[i] + OFFSET(kmem_cache_name), 
                            KVADDR, &name, sizeof(char *),
                            "kmem_cache.name", QUIET|RETURN_ON_ERROR))
--
Crash-utility mailing list -- devel@lists.crash-utility.osci.io
To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

Reply via email to