Several small things about indent and style..
On 2023/01/03 21:05, Tao Liu wrote:
> 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 | 327 +++++++++++++++++++++++++++++++++----------------------
> 2 files changed, 199 insertions(+), 130 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 102597f..a94a243 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 625a94b..534f82f 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))
> @@ -3868,7 +3895,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"), \
> @@ -3890,9 +3917,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), \
> @@ -3919,18 +3946,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));
> + }
Let's join these wrapped lines and remove unnecessary {}s at this
opportunity like this?
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 & UVADDR) || ((args->flag & UVADDR) &&
> + ((args->vaddr >= vm_start) && (args->vaddr < vm_end)))) {
^^^^^^^^
Please replace this tab with 4 spaces to distinguish this line and the
following lines.
> + *(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)) {
^^^^^^^^
Same as above.
> + 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)) {
^^^^^^^^
Same as above.
> + 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)) {
^^^^^^^^
Please replace this tab with 3 tabs here (I prefer to put args close),
or 4 spaces same as above.
> + 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)) {
^^^^^^^^
Please replace this tab with 4 spaces.
> + 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;
> @@ -3942,6 +4094,10 @@ 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;
> + int i;
> + ulong mm_mt, entry_num;
> + struct list_pair *entry_list;
>
> tc = task_to_context(task);
> tm = &task_mem_usage;
> @@ -3975,14 +4131,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) {
> @@ -4006,9 +4162,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),
> @@ -4021,125 +4174,41 @@ 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;
> - }
> + found = FALSE;
>
> - 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));
> + struct handle_each_vm_area_args args = {
> + .task = task, .flag = flag, .vaddr = vaddr,
> + .ref = ref, .tc = tc, .radix = radix,
> + .tm = tm, .buf1 = buf1, .buf2 = buf2,
> + .buf3 = buf3, .buf4 = buf4, .buf5 = buf5,
> + .vma_header = vma_header, .single_vma = &single_vma,
> + .single_vma_found = &single_vma_found, .found = &found,
> + .vma_buf = &vma_buf,
> + };
> +
> + if (INVALID_MEMBER(mm_struct_mmap) && VALID_MEMBER(mm_struct_mm_mt)) {
> + mm_mt = tm->mm_struct_addr + OFFSET(mm_struct_mm_mt);
> + entry_num = do_maple_tree(mm_mt, MAPLE_TREE_COUNT, NULL);
> + entry_list = (struct list_pair *)GETBUF(entry_num * sizeof(struct list_pair));
> + do_maple_tree(mm_mt, MAPLE_TREE_GATHER, entry_list);
> +
> + for (i = 0; i < entry_num; i++) {
> + if (!!(args.vma = (ulong)entry_list[i].value) &&
> + handle_each_vm_area(&args)) {
This is good :)
Thanks,
Kazu
> + FREEBUF(entry_list);
> + return args.vma;
> }
> }
> -
> - if (!(flag & UVADDR) || ((flag & UVADDR) &&
> - ((vaddr >= vm_start) && (vaddr < vm_end)))) {
> - found = TRUE;
> -
> - if (flag & VERIFY_ADDR)
> - 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)
> + FREEBUF(entry_list);
> + } 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)
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
