On Fri, Nov 4, 2022 at 5:07 PM lijiang <liji...@redhat.com> wrote: > > On Tue, Oct 25, 2022 at 8:39 PM <crash-utility-requ...@redhat.com> wrote: > > Date: Tue, 25 Oct 2022 20:38:21 +0800 > > From: Tao Liu <l...@redhat.com> > > To: crash-utility@redhat.com > > Subject: [Crash-utility] [PATCH v2 2/6] Introduce maple tree vma > > iteration to memory.c > > Message-ID: <20221025123825.36421-3-l...@redhat.com> > > 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 <l...@redhat.com> > > --- > > 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 for the suggestion, I have rearranged them a little bit, but it doesn't change much. I didn't think of a better solution for that. Please check out it in v3, we can get it improved then. Thanks, Tao Liu > 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@redhat.com https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki