On Fri, Nov 4, 2022 at 5:07 PM lijiang <lijiang@xxxxxxxxxx> wrote:
>
> 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 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@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
