Re: [PATCH v2 2/6] Introduce maple tree vma iteration to memory.c

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

 



On Tue, Oct 25, 2022 at 8:39 PM <crash-utility-request@xxxxxxxxxx> wrote:
> Date: Tue, 25 Oct 2022 20:38:21 +0800
> From: Tao Liu <ltao@xxxxxxxxxx>
> To: crash-utility@xxxxxxxxxx
> Subject:  [PATCH v2 2/6] Introduce maple tree vma
>         iteration to memory.c
> Message-ID: <20221025123825.36421-3-ltao@xxxxxxxxxx>
> Content-Type: text/plain; charset="US-ASCII"; x-default=true
>
> Since memory.c:vm_area_dump will iterate all vma, this patch mainly
> introduces maple tree vma iteration to it.
>
> We extract the code which handles each vma into a function. If
> mm_struct_mmap exist, aka the linked list of vma iteration available,
> we goto the original way; if not and mm_struct_mm_mt exist, aka
> maple tree is available, then we goto the maple tree vma iteration.
>
> Signed-off-by: Tao Liu <ltao@xxxxxxxxxx>
> ---
>  Makefile |   2 +-
>  memory.c | 319 ++++++++++++++++++++++++++++++++-----------------------
>  2 files changed, 190 insertions(+), 131 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 6f19b77..ad8e9c4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -355,7 +355,7 @@ filesys.o: ${GENERIC_HFILES} filesys.c
>  help.o: ${GENERIC_HFILES} help.c
>         ${CC} -c ${CRASH_CFLAGS} help.c ${WARNING_OPTIONS} ${WARNING_ERROR}
>
> -memory.o: ${GENERIC_HFILES} memory.c
> +memory.o: ${GENERIC_HFILES} ${MAPLE_TREE_HFILES} memory.c
>         ${CC} -c ${CRASH_CFLAGS} memory.c ${WARNING_OPTIONS} ${WARNING_ERROR}
>
>  test.o: ${GENERIC_HFILES} test.c
> diff --git a/memory.c b/memory.c
> index c80ef61..8e043d0 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -21,6 +21,7 @@
>  #include <ctype.h>
>  #include <netinet/in.h>
>  #include <byteswap.h>
> +#include "maple_tree.h"
>
>  struct meminfo {           /* general purpose memory information structure */
>          ulong cache;       /* used by the various memory searching/dumping */
> @@ -136,6 +137,27 @@ struct searchinfo {
>         char buf[BUFSIZE];
>  };
>
> +struct handle_each_vm_area_args {
> +       ulong task;
> +       ulong flag;
> +       ulong vaddr;
> +       struct reference *ref;
> +       char *vma_header;
> +       char *buf1;
> +       char *buf2;
> +       char *buf3;
> +       char *buf4;
> +       char *buf5;
> +       ulong vma;
> +       char **vma_buf;
> +       struct task_mem_usage *tm;
> +       int *found;
> +       int *single_vma_found;
> +       unsigned int radix;
> +       struct task_context *tc;
> +       ulong *single_vma;
> +};
> +
>  static char *memtype_string(int, int);
>  static char *error_handle_string(ulong);
>  static void collect_page_member_data(char *, struct meminfo *);
> @@ -298,6 +320,7 @@ static void dump_page_flags(ulonglong);
>  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 *);
>
>  /*
>   *  Memory display modes specific to this file.
> @@ -362,6 +385,10 @@ vm_init(void)
>
>          MEMBER_OFFSET_INIT(task_struct_mm, "task_struct", "mm");
>          MEMBER_OFFSET_INIT(mm_struct_mmap, "mm_struct", "mmap");
> +        MEMBER_OFFSET_INIT(mm_struct_mm_mt, "mm_struct", "mm_mt");
> +       if (VALID_MEMBER(mm_struct_mm_mt)) {
> +               maple_init();
> +       }
>          MEMBER_OFFSET_INIT(mm_struct_pgd, "mm_struct", "pgd");
>         MEMBER_OFFSET_INIT(mm_struct_rss, "mm_struct", "rss");
>         if (!VALID_MEMBER(mm_struct_rss))
> @@ -3866,7 +3893,7 @@ bailout:
>   *                        for references -- and only then does a display
>   */
>
> -#define PRINT_VM_DATA()                                                  \
> +#define PRINT_VM_DATA(buf4, buf5, tm)                                    \
>                  {                                                        \
>                  fprintf(fp, "%s  %s  ",                                  \
>                      mkstring(buf4, VADDR_PRLEN, CENTER|LJUST, "MM"),     \
> @@ -3888,9 +3915,9 @@ bailout:
>                      mkstring(buf5, 8, CENTER|LJUST, NULL));              \
>                 }
>
> -#define PRINT_VMA_DATA()                                                       \
> +#define PRINT_VMA_DATA(buf1, buf2, buf3, buf4, vma)                            \
>         fprintf(fp, "%s%s%s%s%s %6llx%s%s\n",                                  \
> -                mkstring(buf4, VADDR_PRLEN, CENTER|LJUST|LONG_HEX, MKSTR(vma)),       \
> +                mkstring(buf4, VADDR_PRLEN, CENTER|LJUST|LONG_HEX, MKSTR(vma)),\
>                 space(MINSPACE),                                               \
>                  mkstring(buf2, UVADDR_PRLEN, RJUST|LONG_HEX, MKSTR(vm_start)), \
>                  space(MINSPACE),                                               \
> @@ -3917,18 +3944,143 @@ bailout:
>     (DO_REF_SEARCH(X) && (string_exists(S)) && FILENAME_COMPONENT((S),(X)->str))
>  #define VM_REF_FOUND(X)    ((X) && ((X)->cmdflags & VM_REF_HEADER))
>
> -ulong
> -vm_area_dump(ulong task, ulong flag, ulong vaddr, struct reference *ref)
> +static ulong handle_each_vm_area(struct handle_each_vm_area_args *args)
>  {
> -        struct task_context *tc;
> -       ulong vma;
> +       char *dentry_buf, *file_buf;
>         ulong vm_start;
>         ulong vm_end;
> -       ulong vm_next, vm_mm;
> -       char *dentry_buf, *vma_buf, *file_buf;
> +       ulong vm_mm;
>         ulonglong vm_flags;
>         ulong vm_file, inode;
>         ulong dentry, vfsmnt;
> +
> +       if ((args->flag & PHYSADDR) && !DO_REF_SEARCH(args->ref))
> +               fprintf(fp, "%s", args->vma_header);
> +
> +       inode = 0;
> +       BZERO(args->buf1, BUFSIZE);
> +       *(args->vma_buf) = fill_vma_cache(args->vma);
> +
> +       vm_mm = ULONG(*(args->vma_buf) + OFFSET(vm_area_struct_vm_mm));
> +       vm_end = ULONG(*(args->vma_buf) + OFFSET(vm_area_struct_vm_end));
> +       vm_start = ULONG(*(args->vma_buf) + OFFSET(vm_area_struct_vm_start));
> +       vm_flags = get_vm_flags(*(args->vma_buf));
> +       vm_file = ULONG(*(args->vma_buf) + OFFSET(vm_area_struct_vm_file));
> +
> +       if (args->flag & PRINT_SINGLE_VMA) {
> +               if (args->vma != *(args->single_vma))
> +                       return 0;
> +               fprintf(fp, "%s", args->vma_header);
> +               *(args->single_vma_found) = TRUE;
> +       }
> +
> +       if (args->flag & PRINT_VMA_STRUCTS) {
> +               dump_struct("vm_area_struct", args->vma, args->radix);
> +               return 0;
> +       }
> +
> +       if (vm_file && !(args->flag & VERIFY_ADDR)) {
> +               file_buf = fill_file_cache(vm_file);
> +               dentry = ULONG(file_buf + OFFSET(file_f_dentry));
> +               dentry_buf = NULL;
> +               if (dentry) {
> +                       dentry_buf = fill_dentry_cache(dentry);
> +                       if (VALID_MEMBER(file_f_vfsmnt)) {
> +                               vfsmnt = ULONG(file_buf +
> +                                       OFFSET(file_f_vfsmnt));
> +                               get_pathname(dentry, args->buf1, BUFSIZE,
> +                                       1, vfsmnt);
> +                       } else {
> +                               get_pathname(dentry, args->buf1, BUFSIZE,
> +                                       1, 0);
> +                       }
> +               }
> +               if ((args->flag & PRINT_INODES) && dentry) {
> +                       inode = ULONG(dentry_buf +
> +                               OFFSET(dentry_d_inode));
> +               }
> +       }
> +
> +       if (!(args->flag & UVADDR) || ((args->flag & UVADDR) &&
> +               ((args->vaddr >= vm_start) && (args->vaddr < vm_end)))) {
> +               *(args->found) = TRUE;
> +
> +               if (args->flag & VERIFY_ADDR)
> +                       return args->vma;
> +
> +               if (DO_REF_SEARCH(args->ref)) {
> +                       if (VM_REF_CHECK_HEXVAL(args->ref, args->vma) ||
> +                               VM_REF_CHECK_HEXVAL(args->ref, (ulong)vm_flags) ||
> +                               VM_REF_CHECK_STRING(args->ref, args->buf1)) {
> +                               if (!(args->ref->cmdflags & VM_REF_HEADER)) {
> +                                       print_task_header(fp, args->tc, 0);
> +                                       PRINT_VM_DATA(args->buf4, args->buf5, args->tm);
> +                                       args->ref->cmdflags |= VM_REF_HEADER;
> +                               }
> +                               if (!(args->ref->cmdflags & VM_REF_VMA) ||
> +                                       (args->ref->cmdflags & VM_REF_PAGE)) {
> +                                       fprintf(fp, "%s", args->vma_header);
> +                                       args->ref->cmdflags |= VM_REF_VMA;
> +                                       args->ref->cmdflags &= ~VM_REF_PAGE;
> +                                       args->ref->ref1 = args->vma;
> +                               }
> +                               PRINT_VMA_DATA(args->buf1, args->buf2,
> +                                               args->buf3, args->buf4, args->vma);
> +                       }
> +
> +                       if (vm_area_page_dump(args->vma, args->task,
> +                               vm_start, vm_end, vm_mm, args->ref)) {
> +                               if (!(args->ref->cmdflags & VM_REF_HEADER)) {
> +                                       print_task_header(fp, args->tc, 0);
> +                                       PRINT_VM_DATA(args->buf4, args->buf5, args->tm);
> +                                       args->ref->cmdflags |= VM_REF_HEADER;
> +                               }
> +                               if (!(args->ref->cmdflags & VM_REF_VMA) ||
> +                                       (args->ref->ref1 != args->vma)) {
> +                                       fprintf(fp, "%s", args->vma_header);
> +                                       PRINT_VMA_DATA(args->buf1, args->buf2,
> +                                                       args->buf3, args->buf4, args->vma);
> +                                       args->ref->cmdflags |= VM_REF_VMA;
> +                                       args->ref->ref1 = args->vma;
> +                               }
> +
> +                               args->ref->cmdflags |= VM_REF_DISPLAY;
> +                               vm_area_page_dump(args->vma, args->task,
> +                                       vm_start, vm_end, vm_mm, args->ref);
> +                               args->ref->cmdflags &= ~VM_REF_DISPLAY;
> +                       }
> +
> +                       return 0;
> +               }
> +
> +               if (inode) {
> +                       fprintf(fp, "%lx%s%s%s%s%s%6llx%s%lx %s\n",
> +                               args->vma, space(MINSPACE),
> +                               mkstring(args->buf2, UVADDR_PRLEN, RJUST|LONG_HEX,
> +                               MKSTR(vm_start)), space(MINSPACE),
> +                               mkstring(args->buf3, UVADDR_PRLEN, RJUST|LONG_HEX,
> +                               MKSTR(vm_end)), space(MINSPACE),
> +                               vm_flags, space(MINSPACE), inode, args->buf1);
> +               } else {
> +                       PRINT_VMA_DATA(args->buf1, args->buf2,
> +                                       args->buf3, args->buf4, args->vma);
> +
> +                       if (args->flag & (PHYSADDR|PRINT_SINGLE_VMA))
> +                               vm_area_page_dump(args->vma, args->task,
> +                                       vm_start, vm_end, vm_mm, args->ref);
> +               }
> +
> +               if (args->flag & UVADDR)
> +                       return args->vma;
> +       }
> +       return 0;
> +}
> +
> +ulong
> +vm_area_dump(ulong task, ulong flag, ulong vaddr, struct reference *ref)
> +{
> +        struct task_context *tc;
> +       ulong vma;
>         ulong single_vma;
>         unsigned int radix;
>         int single_vma_found;
> @@ -3940,6 +4092,7 @@ vm_area_dump(ulong task, ulong flag, ulong vaddr, struct reference *ref)
>         char buf4[BUFSIZE];
>         char buf5[BUFSIZE];
>         char vma_header[BUFSIZE];
> +       char *vma_buf;
>
>          tc = task_to_context(task);
>         tm = &task_mem_usage;
> @@ -3973,14 +4126,14 @@ vm_area_dump(ulong task, ulong flag, ulong vaddr, struct reference *ref)
>          if (VM_REF_CHECK_HEXVAL(ref, tm->mm_struct_addr) ||
>              VM_REF_CHECK_HEXVAL(ref, tm->pgd_addr)) {
>                 print_task_header(fp, tc, 0);
> -               PRINT_VM_DATA();
> +               PRINT_VM_DATA(buf4, buf5, tm);
>                 fprintf(fp, "\n");
>                  return (ulong)NULL;
>          }
>
>          if (!(flag & (UVADDR|PRINT_MM_STRUCT|PRINT_VMA_STRUCTS|PRINT_SINGLE_VMA)) &&
>             !DO_REF_SEARCH(ref))
> -               PRINT_VM_DATA();
> +               PRINT_VM_DATA(buf4, buf5, tm);
>
>          if (!tm->mm_struct_addr) {
>                 if (pc->curcmd_flags & MM_STRUCT_FORCE) {
> @@ -4004,9 +4157,6 @@ vm_area_dump(ulong task, ulong flag, ulong vaddr, struct reference *ref)
>                  return (ulong)NULL;
>         }
>
> -       readmem(tm->mm_struct_addr + OFFSET(mm_struct_mmap), KVADDR,
> -               &vma, sizeof(void *), "mm_struct mmap", FAULT_ON_ERROR);
> -
>                 sprintf(vma_header, "%s%s%s%s%s  FLAGS%sFILE\n",
>                  mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "VMA"),
>                  space(MINSPACE),
> @@ -4019,125 +4169,34 @@ vm_area_dump(ulong task, ulong flag, ulong vaddr, struct reference *ref)
>             !DO_REF_SEARCH(ref))
>                 fprintf(fp, "%s", vma_header);
>
> -       for (found = FALSE; vma; vma = vm_next) {
> -
> -               if ((flag & PHYSADDR) && !DO_REF_SEARCH(ref))
> -                       fprintf(fp, "%s", vma_header);
> -
> -               inode = 0;
> -               BZERO(buf1, BUFSIZE);
> -               vma_buf = fill_vma_cache(vma);
> -
> -               vm_mm = ULONG(vma_buf + OFFSET(vm_area_struct_vm_mm));
> -               vm_end = ULONG(vma_buf + OFFSET(vm_area_struct_vm_end));
> -               vm_next = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
> -               vm_start = ULONG(vma_buf + OFFSET(vm_area_struct_vm_start));
> -               vm_flags = get_vm_flags(vma_buf);
> -               vm_file = ULONG(vma_buf + OFFSET(vm_area_struct_vm_file));
> -
> -               if (flag & PRINT_SINGLE_VMA) {
> -                       if (vma != single_vma)
> -                               continue;
> -                       fprintf(fp, "%s", vma_header);
> -                       single_vma_found = TRUE;
> -               }
> -
> -               if (flag & PRINT_VMA_STRUCTS) {
> -                       dump_struct("vm_area_struct", vma, radix);
> -                       continue;
> -               }
> -
> -               if (vm_file && !(flag & VERIFY_ADDR)) {
> -                       file_buf = fill_file_cache(vm_file);
> -                       dentry = ULONG(file_buf + OFFSET(file_f_dentry));
> -                       dentry_buf = NULL;
> -                       if (dentry) {
> -                               dentry_buf = fill_dentry_cache(dentry);
> -                               if (VALID_MEMBER(file_f_vfsmnt)) {
> -                                       vfsmnt = ULONG(file_buf +
> -                                               OFFSET(file_f_vfsmnt));
> -                                       get_pathname(dentry, buf1, BUFSIZE,
> -                                               1, vfsmnt);
> -                               } else {
> -                                       get_pathname(dentry, buf1, BUFSIZE,
> -                                               1, 0);
> -                               }
> -                       }
> -                       if ((flag & PRINT_INODES) && dentry) {
> -                               inode = ULONG(dentry_buf +
> -                                       OFFSET(dentry_d_inode));
> -                       }
> -               }
> -
> -               if (!(flag & UVADDR) || ((flag & UVADDR) &&
> -                   ((vaddr >= vm_start) && (vaddr < vm_end)))) {
> -                       found = TRUE;
> +       found = FALSE;
>
> -                       if (flag & VERIFY_ADDR)
> +       struct handle_each_vm_area_args args = {
> +               .task = task,    .flag = flag,             .vaddr = vaddr,
> +               .ref = ref,      .vma_header = vma_header, .buf1 = buf1,
> +               .buf2 = buf2,    .buf3 = buf3,             .buf4 = buf4,
> +               .buf5 = buf5,    .vma_buf = &vma_buf,      .tm = tm,
> +               .found = &found, .single_vma_found = &single_vma_found,
> +               .radix = radix,  .tc = tc,           .single_vma = &single_vma,
> +       };

The refactored code looks good, but the above structure seems to lack
a little readability. Is it possible to rearrange/split them according
to some rules? Such as parameter dependencies or input/output
parameters, etc... Just an idea.

Thanks.
Lianbo

> +
> +       if (INVALID_MEMBER(mm_struct_mmap) && VALID_MEMBER(mm_struct_mm_mt)) {
> +               VMA_ITERATOR(vmi, (struct maple_tree *)
> +                                  (tm->mm_struct_addr + OFFSET(mm_struct_mm_mt)), 0);
> +               for_each_vma(vmi, vma) {
> +                       args.vma = vma;
> +                       if (handle_each_vm_area(&args))
>                                 return vma;
> -
> -                       if (DO_REF_SEARCH(ref)) {
> -                               if (VM_REF_CHECK_HEXVAL(ref, vma) ||
> -                                   VM_REF_CHECK_HEXVAL(ref, (ulong)vm_flags) ||
> -                                   VM_REF_CHECK_STRING(ref, buf1)) {
> -                                       if (!(ref->cmdflags & VM_REF_HEADER)) {
> -                                               print_task_header(fp, tc, 0);
> -                                               PRINT_VM_DATA();
> -                                               ref->cmdflags |= VM_REF_HEADER;
> -                                       }
> -                                       if (!(ref->cmdflags & VM_REF_VMA) ||
> -                                           (ref->cmdflags & VM_REF_PAGE)) {
> -                                               fprintf(fp, "%s", vma_header);
> -                                               ref->cmdflags |= VM_REF_VMA;
> -                                               ref->cmdflags &= ~VM_REF_PAGE;
> -                                               ref->ref1 = vma;
> -                                       }
> -                                       PRINT_VMA_DATA();
> -                               }
> -
> -                               if (vm_area_page_dump(vma, task,
> -                                   vm_start, vm_end, vm_mm, ref)) {
> -                                       if (!(ref->cmdflags & VM_REF_HEADER)) {
> -                                               print_task_header(fp, tc, 0);
> -                                               PRINT_VM_DATA();
> -                                               ref->cmdflags |= VM_REF_HEADER;
> -                                       }
> -                                        if (!(ref->cmdflags & VM_REF_VMA) ||
> -                                            (ref->ref1 != vma)) {
> -                                                fprintf(fp, "%s", vma_header);
> -                                               PRINT_VMA_DATA();
> -                                                ref->cmdflags |= VM_REF_VMA;
> -                                                ref->ref1 = vma;
> -                                       }
> -
> -                                       ref->cmdflags |= VM_REF_DISPLAY;
> -                                       vm_area_page_dump(vma, task,
> -                                               vm_start, vm_end, vm_mm, ref);
> -                                       ref->cmdflags &= ~VM_REF_DISPLAY;
> -                               }
> -
> -                               continue;
> -                       }
> -
> -                       if (inode) {
> -                                fprintf(fp, "%lx%s%s%s%s%s%6llx%s%lx %s\n",
> -                                    vma, space(MINSPACE),
> -                                    mkstring(buf2, UVADDR_PRLEN, RJUST|LONG_HEX,
> -                                        MKSTR(vm_start)), space(MINSPACE),
> -                                    mkstring(buf3, UVADDR_PRLEN, RJUST|LONG_HEX,
> -                                        MKSTR(vm_end)), space(MINSPACE),
> -                                   vm_flags, space(MINSPACE), inode, buf1);
> -                       } else {
> -                               PRINT_VMA_DATA();
> -
> -                               if (flag & (PHYSADDR|PRINT_SINGLE_VMA))
> -                                       vm_area_page_dump(vma, task,
> -                                               vm_start, vm_end, vm_mm, ref);
> -                       }
> -
> -                       if (flag & UVADDR)
> +               }
> +       } else {
> +               readmem(tm->mm_struct_addr + OFFSET(mm_struct_mmap), KVADDR,
> +                       &vma, sizeof(void *), "mm_struct mmap", FAULT_ON_ERROR);
> +               while (vma) {
> +                       args.vma = vma;
> +                       if (handle_each_vm_area(&args))
>                                 return vma;
> -               }
> +                       vma = ULONG(vma_buf + OFFSET(vm_area_struct_vm_next));
> +               }
>         }
>
>         if (flag & VERIFY_ADDR)
> --
> 2.33.1

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