On Tue, Sep 24, 2024 at 11:42 AM <devel-request@lists.crash-utility.osci.io> wrote:
Date: Tue, 24 Sep 2024 15:30:00 +1200
From: Tao Liu <ltao@redhat.com>
Subject: [Crash-utility] Re: [PATCH v3] kmem: fix the determination
        for slab page
To: lijiang <lijiang@redhat.com>
Cc: devel@lists.crash-utility.osci.io
Message-ID:
        <CAO7dBbVf-GeGZXs011x3LEf_c=0L7jGq8RRv5QRgsN_MEF2UqQ@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"

Hi lianbo & qiwu,

For the v3, LGTM, so ack.

Applied:
https://github.com/crash-utility/crash/commit/9babe985a7eb001ec398a3734c1aee853a12668f

Thanks
Lianbo 

Thanks,
Tao Liu


On Fri, Sep 20, 2024 at 6:55 PM lijiang <lijiang@redhat.com> wrote:
>
> On Fri, Sep 20, 2024 at 9:30 AM <devel-request@lists.crash-utility.osci.io> wrote:
>>
>> Date: Fri, 20 Sep 2024 01:28:32 -0000
>> From: qiwu.chen@transsion.com
>> Subject: [Crash-utility] [PATCH v3] kmem: fix the determination for
>>         slab page
>> To: devel@lists.crash-utility.osci.io
>> Message-ID: <20240920012832.29184.28326@lists.crash-utility.osci.io>
>> Content-Type: text/plain; charset="utf-8"
>>
>> The determination for a slab page has changed due to changing
>> PG_slab from a page flag to a page type since kernel commit
>> 46df8e73a4a3.
>>
>> Before apply this patch:
>> crash> kmem -s ffff000002aa4100
>> kmem: address is not allocated in slab subsystem: ffff000002aa4100
>>
>> After apply this patch:
>> crash> kmem -s ffff000002aa4100
>> CACHE             OBJSIZE  ALLOCATED     TOTAL  SLABS  SSIZE  NAME
>> ffff00000140f900     4096         94       126     18    32k  task_struct
>>   SLAB              MEMORY            NODE  TOTAL  ALLOCATED  FREE
>>   fffffdffc00aa800  ffff000002aa0000     0      7          5     2
>>   FREE / [ALLOCATED]
>>   [ffff000002aa4100]
>>
>> Signed-off-by: qiwu.chen <qiwu.chen@transsion.com>
>> ---
>>  defs.h    |  7 ++++++
>>  memory.c  | 68 ++++++++++++++++++++++++++++++++++++++++++++++---------
>>  symbols.c |  2 ++
>>  3 files changed, 66 insertions(+), 11 deletions(-)
>>
>
> Thank you for the update, qiwu.
>
> I have no other issues, for the v3: Ack
>
> Thanks
> Lianbo
>
>>
>> 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..8befe8c 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;
>>
>> +/*
>> + * Before kernel commit ff202303c398e, the value is defined as a macro, so copy it here;
>> + * After this commit, the value is defined as an enum, which can be evaluated at runtime.
>> + */
>> +#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)
>> +{
>> +       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 page, 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(page+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,7 @@ 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");
>> +       MEMBER_OFFSET_INIT(page_page_type, "page", "page_type");
>>
>>         MEMBER_OFFSET_INIT(mm_struct_pgd, "mm_struct", "pgd");
>>
>> @@ -1278,6 +1316,8 @@ vm_init(void)
>>
>>         page_flags_init();
>>
>> +       page_type_init();
>> +
>>         rss_page_types_init();
>>
>>         vt->flags |= VM_INIT;
>> @@ -5931,7 +5971,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 +6421,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)
>> @@ -6694,7 +6734,6 @@ dump_hstates()
>>         FREEBUF(hstate);
>>  }
>>
>> -
>>  static void
>>  page_flags_init(void)
>>  {
>> @@ -6775,6 +6814,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,
>> @@ -6835,8 +6877,9 @@ 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]);
>> -
>> +                       vt->pageflags_data[p].mask = 1 << atoi(arglist[2]);
>> +                       if (!strncmp(nameptr, "slab", 4))
>> +                               vt->flags |= SLAB_PAGEFLAGS;
>>                         p++;
>>                 }
>>         } else
>> @@ -9736,14 +9779,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;
>> @@ -14108,6 +14151,8 @@ dump_vm_table(int verbose)
>>                 fprintf(fp, "%sNODELISTS_IS_PTR", others++ ? "|" : "");\
>>         if (vt->flags & VM_INIT)
>>                 fprintf(fp, "%sVM_INIT", others++ ? "|" : "");\
>> +       if (vt->flags & SLAB_PAGEFLAGS)
>> +               fprintf(fp, "%sSLAB_PAGEFLAGS", others++ ? "|" : "");\
>>
>>         fprintf(fp, ")\n");
>>         if (vt->kernel_pgd[0] == vt->kernel_pgd[1])
>> @@ -14237,6 +14282,7 @@ dump_vm_table(int verbose)
>>                         vt->pageflags_data[i].mask,
>>                         vt->pageflags_data[i].name);
>>         }
>> +       fprintf(fp, "     page_type_base: %x\n", vt->page_type_base);
>>
>>         dump_vma_cache(VERBOSE);
>>  }
>> @@ -20195,7 +20241,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 +20256,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 +20268,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))
>> diff --git a/symbols.c b/symbols.c
>> index 69a1fbb..014cd29 100644
>> --- a/symbols.c
>> +++ b/symbols.c
>> @@ -10339,6 +10339,8 @@ dump_offset_table(char *spec, ulong makestruct)
>>          fprintf(fp, "            page_compound_head: %ld\n",
>>                  OFFSET(page_compound_head));
>>          fprintf(fp, "                  page_private: %ld\n", OFFSET(page_private));
>> +       fprintf(fp, "                page_page_type: %ld\n",
>> +               OFFSET(page_page_type));
>>
>>         fprintf(fp, "        trace_print_flags_mask: %ld\n",
>>                 OFFSET(trace_print_flags_mask));
>> --
>> 2.25.1