[Crash-utility] Re: [PATCH] kmem: fix the determination for slab page

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

 



Hi qiwu,

Thanks for the fix, however there are several issues:

On Mon, Sep 2, 2024 at 2:30 AM <qiwu.chen@xxxxxxxxxxxxx> wrote:
>
> Hi crash maintainers,
> The previous patch is a mistake that can be ignored. Please help review the following rework patch, thanks!
>
> 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@xxxxxxxxxxxxx>
> ---
>  defs.h   |  7 ++++++
>  memory.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++--------
>  2 files changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index dfbd241..d6cb2af 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -1416,6 +1416,7 @@ struct offset_table {                    /* stash of commonly-used offsets */
>         long page_buffers;
>         long page_lru;
>         long page_pte;
> +       long page_page_type;
>         long swap_info_struct_swap_file;
>         long swap_info_struct_swap_vfsmnt;
>         long swap_info_struct_flags;

The new members for the offset_table should always be appended to the
end of the struct.

> @@ -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 a74ebaf..f4905a2 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -323,6 +323,7 @@ static ulong kmem_cache_nodelists(ulong);
>  static void dump_hstates(void);
>  static ulong freelist_ptr(struct meminfo *, ulong, ulong);
>  static ulong handle_each_vm_area(struct handle_each_vm_area_args *);
> +static void page_type_init(void);
>
>  /*
>   *  Memory display modes specific to this file.
> @@ -504,6 +505,8 @@ 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");
>
>         MEMBER_OFFSET_INIT(mm_struct_pgd, "mm_struct", "pgd");
>
> @@ -1278,6 +1281,8 @@ vm_init(void)
>
>         page_flags_init();
>
> +       page_type_init();
> +
>         rss_page_types_init();
>
>         vt->flags |= VM_INIT;
> @@ -5645,6 +5650,36 @@ PG_slab_flag_init(void)
>
>  #define PGMM_CACHED (512)
>
> +#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 PageSlab(ulong flags, uint page_type)
> +{
> +       if (vt->flags & SLAB_PAGEFLAGS) {
> +               if ((flags >> vt->PG_slab) & 1)
> +                       return TRUE;
> +       } else if (PageType(page_type, (uint)vt->PG_slab))
> +               return TRUE;
> +       else
> +               return FALSE;
> +}

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?

> +
>  static void
>  dump_mem_map_SPARSEMEM(struct meminfo *mi)
>  {
> @@ -5657,7 +5692,7 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
>          ulong PG_reserved_flag;
>         long buffers;
>         ulong inode, offset, flags, mapping, index;
> -       uint count;
> +       uint count, page_type;
>         int print_hdr, pg_spec, phys_spec, done;
>         int v22;
>         char hdr[BUFSIZE];
> @@ -5904,6 +5939,7 @@ dump_mem_map_SPARSEMEM(struct meminfo *mi)
>                         if (SIZE(page_flags) == 4)
>                                 flags &= 0xffffffff;
>                         count = UINT(pcache + OFFSET(page_count));
> +                       page_type = UINT(pcache + OFFSET(page_page_type));
>
>                         switch (mi->flags)
>                         {
> @@ -5931,7 +5967,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 (PageSlab(flags, page_type))
>                                                 slabs++;
>                                 } else {
>                                         if ((flags >> v24_PG_slab) & 1)
> @@ -6142,7 +6178,7 @@ dump_mem_map(struct meminfo *mi)
>         long buffers;
>         ulong inode, offset, flags, mapping, index;
>         ulong node_size;
> -       uint count;
> +       uint count, page_type;
>         int print_hdr, pg_spec, phys_spec, done;
>         int v22;
>         struct node_table *nt;
> @@ -6354,6 +6390,7 @@ dump_mem_map(struct meminfo *mi)
>                         if (SIZE(page_flags) == 4)
>                                 flags &= 0xffffffff;
>                         count = UINT(pcache + OFFSET(page_count));
> +                       page_type = UINT(pcache + OFFSET(page_page_type));
>
>                         switch (mi->flags)
>                         {
> @@ -6381,7 +6418,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 (PageSlab(flags, page_type))
>                                                 slabs++;
>                                 } else {
>                                         if ((flags >> v24_PG_slab) & 1)
> @@ -6694,7 +6731,6 @@ dump_hstates()
>         FREEBUF(hstate);
>  }
>
> -
>  static void
>  page_flags_init(void)
>  {
> @@ -6775,6 +6811,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 +6874,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 (!strcmp(nameptr, "slab"))
> +                               vt->flags |= SLAB_PAGEFLAGS;
>                         p++;
>                 }
>         } else
> @@ -9715,6 +9755,7 @@ vaddr_to_kmem_cache(ulong vaddr, char *buf, int verbose)
>  {
>         physaddr_t paddr;
>         ulong page, cache, page_flags;
> +       uint page_type;
>
>          if (!kvtop(NULL, vaddr, &paddr, 0)) {
>                 if (verbose)
> @@ -9736,14 +9777,20 @@ 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))) {
> +               readmem(page+OFFSET(page_page_type), KVADDR,
> +                       &page_type, sizeof(page_type), "page_type",
> +                       FAULT_ON_ERROR);

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.

Thanks,
Tao Liu

> +               if (!PageSlab(page_flags, page_type)) {
>                         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)))
> +                               readmem(compound_head(page)+OFFSET(page_page_type), KVADDR,
> +                                       &page_type, sizeof(page_type), "page_type",
> +                                       FAULT_ON_ERROR);
> +                               if (!PageSlab(page_flags, page_type))
>                                         return NULL;
>                         } else
>                                 return NULL;
> @@ -20195,6 +20242,7 @@ is_slab_page(struct meminfo *si, char *buf)
>  {
>         int i, cnt;
>         ulong page_slab, page_flags, name;
> +       uint page_type;
>          ulong *cache_list;
>          char *retval;
>
> @@ -20209,7 +20257,12 @@ is_slab_page(struct meminfo *si, char *buf)
>             RETURN_ON_ERROR|QUIET))
>                 return NULL;
>
> -       if (!(page_flags & (1 << vt->PG_slab)))
> +       if (!readmem(si->spec_addr + OFFSET(page_page_type), KVADDR,
> +               &page_type, sizeof(ulong), "page.page_type",
> +               RETURN_ON_ERROR|QUIET))
> +               return NULL;
> +
> +       if (!PageSlab(page_flags, page_type))
>                 return NULL;
>
>         if (!readmem(si->spec_addr + OFFSET(page_slab), KVADDR,
> --
> 2.25.1
> --
> Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
> Contribution Guidelines: https://github.com/crash-utility/crash/wiki
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
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