From: yang_oliver(a)hotmail.com To: crash-utility(a)redhat.com Date: Wed, 17 Jun 2015 23:49:02 +0800 Subject: [Crash-utility] [PATCH v2] files: support dump file pages from its address space Dave, Sorry, I can't reply the mail to orignial thread, as my vpn got blocked and I can't access my gmail account. This is the v2 version of files -M and -m support. Following issues got fixed in this version, 1. Patch is based on current git HEAD 2. Fixed warnings during make warn 3. files -M show full file path 4. Change PGCACHE-PGS to PAGE-COUNT Some background informtion: Each Linux fd points to a file struct. And each file struct or inode had a address space. If the file is a regular file, the address space defined a page tree, which are the page caches for this file. Some of file types don't use page cache. Page tree support was from 2.6 kernel, I couldn't access early Linux commit log. But at least 2.6.8 already had the definition. 5. Fixied page tree dump bugs, make sure page dump matched page number This is a bug in my radix tree dump code. I will share a method to valid page dumps later. 6. Use common dump_mem_map api instead of SPARSEMEM version Original dump_mem_map had a bug, I use SPARSEMEM version as workaround. But forgot switch it back. 7. Fixed the page tree dump bugs on 32bit kernel This is a bug in my radix tree dump code. I had fixed and verified it on Fedora 20 32bit kernel. 8. Check address space page tree number, will print error kernel is old I checked struct address_space.page_tree member. 9. Reused existing radix tree dump api, and extended it. RADIX_TREE_DUMP has no users in crash, this patch is first user. I add a call back to make it more flexible. 10. Page count is gotten from address_space.nrpages Original way(loop in radix tree) is not efficient. Let me know your comments, thanks. files: support dump file pages from its address space Added two options in files command, 1. -M option, which allows dump address space and page number for each files 2. -m option, which could dump each pages in given address mapping The foreach command also could work with -M, so that we can easily find which processes/files hold most page cache pages within the system. Signed-off-by: Yong Yang <yangoliver(a)gmail.com&gt; --- defs.h | 11 ++++++- filesys.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++----------- kernel.c | 4 +-- memory.c | 73 +++++++++++++++++++++++++++++++++++++++++ task.c | 25 +++++++++++--- 5 files changed, 197 insertions(+), 27 deletions(-) diff --git a/defs.h b/defs.h index b25b505..608e09f 100644 --- a/defs.h +++ b/defs.h @@ -1111,6 +1111,7 @@ extern struct machdep_table *machdep; #define FOREACH_a_FLAG (0x4000000) #define FOREACH_G_FLAG (0x8000000) #define FOREACH_F_FLAG2 (0x10000000) +#define FOREACH_M_FLAG (0x20000000) #define FOREACH_PS_EXCLUSIVE \ (FOREACH_g_FLAG|FOREACH_a_FLAG|FOREACH_t_FLAG|FOREACH_c_FLAG|FOREACH_p_FLAG|FOREACH_l_FLAG|FOREACH_r_FLAG|FOREACH_m_FLAG) @@ -1416,6 +1417,7 @@ struct offset_table { /* stash of commonly-used offsets */ long inode_i_flock; long inode_i_fop; long inode_i_mapping; + long address_space_page_tree; long address_space_nrpages; long vfsmount_mnt_next; long vfsmount_mnt_devname; @@ -2286,11 +2288,13 @@ struct vm_table { /* kernel VM-related data */ #define PAGEFLAGS (0x4000000) #define SLAB_OVERLOAD_PAGE (0x8000000) #define SLAB_CPU_CACHE (0x10000000) +#define AS_PAGE_TREE (0x20000000) #define IS_FLATMEM() (vt->flags & FLATMEM) #define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM) #define IS_SPARSEMEM() (vt->flags & SPARSEMEM) #define IS_SPARSEMEM_EX() (vt->flags & SPARSEMEM_EX) +#define IS_AS_PAGE_TREE() (vt->flags & AS_PAGE_TREE) #define COMMON_VADDR_SPACE() (vt->flags & COMMON_VADDR) #define PADDR_PRLEN (vt->paddr_prlen) @@ -2598,6 +2602,7 @@ struct load_module { #define PRINT_SINGLE_VMA (0x80) #define PRINT_RADIX_10 (0x100) #define PRINT_RADIX_16 (0x200) +#define PRINT_PAGES (0x400) #define MIN_PAGE_SIZE (4096) @@ -4707,6 +4712,9 @@ void alter_stackbuf(struct bt_info *); int vaddr_type(ulong, struct task_context *); char *format_stack_entry(struct bt_info *bt, char *, ulong, ulong); int in_user_stack(ulong, ulong); +void dump_file_address_mappings(ulong); +long get_page_tree_count(ulong i_mapping); +int is_as_page_tree_supported(void); /* * filesys.c @@ -4747,12 +4755,13 @@ struct radix_tree_pair { ulong index; void *value; }; -ulong do_radix_tree(ulong, int, struct radix_tree_pair *); +ulong do_radix_tree(ulong, int, struct radix_tree_pair *, int (*)(ulong)); int file_dump(ulong, ulong, ulong, int, int); #define DUMP_FULL_NAME 1 #define DUMP_INODE_ONLY 2 #define DUMP_DENTRY_ONLY 4 #define DUMP_EMPTY_FILE 8 +#define DUMP_FILE_PAGE 16 #endif /* !GDB_COMMON */ int same_file(char *, char *); #ifndef GDB_COMMON diff --git a/filesys.c b/filesys.c index 0573fe6..f0ec78b 100644 --- a/filesys.c +++ b/filesys.c @@ -2187,11 +2187,12 @@ cmd_files(void) int subsequent; struct reference reference, *ref; char *refarg; + int open_flags = 0; ref = NULL; refarg = NULL; - while ((c = getopt(argcnt, args, "d:R:")) != EOF) { + while ((c = getopt(argcnt, args, "d:R:m:M")) != EOF) { switch(c) { case 'R': @@ -2209,7 +2210,20 @@ cmd_files(void) value = htol(optarg, FAULT_ON_ERROR, NULL); display_dentry_info(value); return; - + case 'm': + if (is_as_page_tree_supported()) { + value = htol(optarg, FAULT_ON_ERROR, NULL); + dump_file_address_mappings(value); + } else { + option_not_supported('m'); + } + return; + case 'M': + if (is_as_page_tree_supported()) + open_flags |= PRINT_PAGES; + else + option_not_supported('M'); + break; default: argerrs++; break; @@ -2222,7 +2236,9 @@ cmd_files(void) if (!args[optind]) { if (!ref) print_task_header(fp, CURRENT_CONTEXT(), 0); - open_files_dump(CURRENT_TASK(), 0, ref); + + open_files_dump(CURRENT_TASK(), open_flags, ref); + return; } @@ -2241,7 +2257,7 @@ cmd_files(void) for (tc = pid_to_context(value); tc; tc = tc->tc_next) { if (!ref) print_task_header(fp, tc, subsequent); - open_files_dump(tc->task, 0, ref); + open_files_dump(tc->task, open_flags, ref); fprintf(fp, "\n"); } break; @@ -2249,7 +2265,7 @@ cmd_files(void) case STR_TASK: if (!ref) print_task_header(fp, tc, subsequent); - open_files_dump(tc->task, 0, ref); + open_files_dump(tc->task, open_flags, ref); break; case STR_INVALID: @@ -2321,6 +2337,7 @@ open_files_dump(ulong task, int flags, struct reference *ref) char buf4[BUFSIZE]; char root_pwd[BUFSIZE]; int root_pwd_printed = 0; + int file_dump_flags = 0; BZERO(root_pathname, BUFSIZE); BZERO(pwd_pathname, BUFSIZE); @@ -2329,15 +2346,27 @@ open_files_dump(ulong task, int flags, struct reference *ref) fdtable_buf = GETBUF(SIZE(fdtable)); fill_task_struct(task); - sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n", - space(MINSPACE), - mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "FILE"), - space(MINSPACE), - mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "DENTRY"), - space(MINSPACE), - mkstring(buf3, VADDR_PRLEN, CENTER|LJUST, "INODE"), - space(MINSPACE), - space(MINSPACE)); + if (flags & PRINT_PAGES) { + sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n", + space(MINSPACE), + mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "ADDR-SPACE"), + space(MINSPACE), + mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "PAGE-COUNT"), + space(MINSPACE), + mkstring(buf3, VADDR_PRLEN, CENTER|LJUST, "INODE"), + space(MINSPACE), + space(MINSPACE)); + } else { + sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n", + space(MINSPACE), + mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "FILE"), + space(MINSPACE), + mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "DENTRY"), + space(MINSPACE), + mkstring(buf3, VADDR_PRLEN, CENTER|LJUST, "INODE"), + space(MINSPACE), + space(MINSPACE)); + } tc = task_to_context(task); @@ -2523,6 +2552,10 @@ open_files_dump(ulong task, int flags, struct reference *ref) return; } + file_dump_flags = DUMP_FULL_NAME | DUMP_EMPTY_FILE; + if (flags & PRINT_PAGES) + file_dump_flags |= DUMP_FILE_PAGE; + j = 0; for (;;) { unsigned long set; @@ -2539,8 +2572,7 @@ open_files_dump(ulong task, int flags, struct reference *ref) if (ref && file) { open_tmpfile(); - if (file_dump(file, 0, 0, i, - DUMP_FULL_NAME|DUMP_EMPTY_FILE)) { + if (file_dump(file, 0, 0, i, file_dump_flags)) { BZERO(buf4, BUFSIZE); rewind(pc->tmpfile); ret = fgets(buf4, BUFSIZE, @@ -2558,8 +2590,7 @@ open_files_dump(ulong task, int flags, struct reference *ref) fprintf(fp, "%s", files_header); header_printed = 1; } - file_dump(file, 0, 0, i, - DUMP_FULL_NAME|DUMP_EMPTY_FILE); + file_dump(file, 0, 0, i, file_dump_flags); } } i++; @@ -2754,6 +2785,8 @@ file_dump(ulong file, ulong dentry, ulong inode, int fd, int flags) char buf1[BUFSIZE]; char buf2[BUFSIZE]; char buf3[BUFSIZE]; + ulong i_mapping = 0; + ulong count = 0; file_buf = NULL; @@ -2863,6 +2896,28 @@ file_dump(ulong file, ulong dentry, ulong inode, int fd, int flags) type, space(MINSPACE), pathname+1); + } else if (flags & DUMP_FILE_PAGE) { + i_mapping = ULONG(inode_buf + OFFSET(inode_i_mapping)); + count = get_page_tree_count(i_mapping); + + fprintf(fp, "%3d%s%s%s%s%s%s%s%s%s%s\n", + fd, + space(MINSPACE), + mkstring(buf1, VADDR_PRLEN, + CENTER|RJUST|LONG_HEX, + MKSTR(i_mapping)), + space(MINSPACE), + mkstring(buf2, VADDR_PRLEN, + CENTER|RJUST|LONG_DEC, + MKSTR(count)), + space(MINSPACE), + mkstring(buf3, VADDR_PRLEN, + CENTER|RJUST|LONG_HEX, + MKSTR(inode)), + space(MINSPACE), + type, + space(MINSPACE), + pathname); } else { fprintf(fp, "%3d%s%s%s%s%s%s%s%s%s%s\n", fd, @@ -3877,9 +3932,13 @@ ulong RADIX_TREE_MAP_MASK = UNINITIALIZED; * RADIX_TREE_GATHER; the dimension (max count) of the array may * be stored in the index field of the first structure to avoid * any chance of an overrun. + * + * entry_ops: The operation against each of returned entry value. + * Only used by RADIX_TREE_DUMP. */ ulong -do_radix_tree(ulong root, int flag, struct radix_tree_pair *rtp) +do_radix_tree(ulong root, int flag, struct radix_tree_pair *rtp, + int (*entry_ops)(ulong)) { int i, ilen, height; long nlen; @@ -3970,7 +4029,19 @@ do_radix_tree(ulong root, int flag, struct radix_tree_pair *rtp) for (index = count = 0; index <= maxindex; index++) { if ((ret = radix_tree_lookup(root_rnode, index, height))) { - fprintf(fp, "[%ld] %lx\n", index, (ulong)ret); + if (entry_ops == NULL) { + /* Default operation */ + fprintf(fp, "[%ld] %lx\n", + index, (ulong)ret); + } else { + /* Caller defined operation */ + if (entry_ops((ulong)ret) != 0) { + error(FATAL, "do_radix_tree: " + "dump operation failed, " + "count: %ld\n", count); + return -EIO; + } + } count++; } } diff --git a/kernel.c b/kernel.c index cb8084a..53d2e1d 100644 --- a/kernel.c +++ b/kernel.c @@ -5746,12 +5746,12 @@ get_irq_desc_addr(int irq) return addr; cnt = do_radix_tree(symbol_value("irq_desc_tree"), - RADIX_TREE_COUNT, NULL); + RADIX_TREE_COUNT, NULL, NULL); len = sizeof(struct radix_tree_pair) * (cnt+1); rtp = (struct radix_tree_pair *)GETBUF(len); rtp[0].index = cnt; cnt = do_radix_tree(symbol_value("irq_desc_tree"), - RADIX_TREE_GATHER, rtp); + RADIX_TREE_GATHER, rtp, NULL); if (kt->highest_irq == 0) kt->highest_irq = rtp[cnt-1].index; diff --git a/memory.c b/memory.c index 765732b..0102dbc 100644 --- a/memory.c +++ b/memory.c @@ -292,6 +292,7 @@ static void dump_per_cpu_offsets(void); static void dump_page_flags(ulonglong); static ulong kmem_cache_nodelists(ulong); static void dump_hstates(void); +static int dump_file_page(ulong); /* * Memory display modes specific to this file. @@ -476,6 +477,9 @@ vm_init(void) MEMBER_OFFSET_INIT(block_device_bd_list, "block_device", "bd_list"); MEMBER_OFFSET_INIT(block_device_bd_disk, "block_device", "bd_disk"); MEMBER_OFFSET_INIT(inode_i_mapping, "inode", "i_mapping"); + MEMBER_OFFSET_INIT(address_space_page_tree, "address_space", "page_tree"); + if (VALID_MEMBER(address_space_page_tree)) + vt->flags |= AS_PAGE_TREE; MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "nrpages"); if (INVALID_MEMBER(address_space_nrpages)) MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "__nrpages"); @@ -6465,6 +6469,75 @@ translate_page_flags(char *buffer, ulong flags) } /* + * Page tree dump ops. + */ +static int +dump_file_page(ulong page) +{ + struct meminfo meminfo; + + BZERO(&meminfo, sizeof(struct meminfo)); + meminfo.spec_addr = page; + meminfo.memtype = KVADDR; + meminfo.flags = ADDRESS_SPECIFIED; + dump_mem_map(&meminfo); + + return 0; +} + +/* + * The address space file mapping radix tree walker. + */ +void +dump_file_address_mappings(ulong i_mapping) +{ + ulong root_rnode; + ulong count; + + root_rnode = i_mapping + OFFSET(address_space_page_tree); + count = get_page_tree_count(i_mapping); + fprintf(fp, "Address Space %lx, page tree %lx, %ld pages\n\n", + i_mapping, root_rnode, count); + + /* Dump each pages in radix tree */ + (void) do_radix_tree(root_rnode, RADIX_TREE_DUMP, + NULL, &dump_file_page); + + return; +} + +/* + * Get the page count for the specific mapping + */ +long +get_page_tree_count(ulong i_mapping) +{ + ulong address_space = i_mapping; + char *address_space_buf; + ulong nrpages = 0; + + address_space_buf = GETBUF(SIZE(address_space)); + + readmem(address_space, KVADDR, address_space_buf, + SIZE(address_space), "address_space buffer", + FAULT_ON_ERROR); + nrpages = ULONG(address_space_buf + OFFSET(address_space_nrpages)); + + FREEBUF(address_space_buf); + + return nrpages; +} + +/* + * Check the availability of address space page tree + */ +int +is_as_page_tree_supported(void) +{ + return (IS_AS_PAGE_TREE() ? TRUE : FALSE); +} + +/* * dump_page_hash_table() displays the entries in each page_hash_table. */ diff --git a/task.c b/task.c index 45be68c..4c95259 100644 --- a/task.c +++ b/task.c @@ -5612,7 +5612,7 @@ cmd_foreach(void) BZERO(&foreach_data, sizeof(struct foreach_data)); fd = &foreach_data; - while ((c = getopt(argcnt, args, "R:vomlgersStTpukcfFxhdaG")) != EOF) { + while ((c = getopt(argcnt, args, "R:vomMlgersStTpukcfFxhdaG")) != EOF) { switch(c) { case 'R': @@ -5636,6 +5636,10 @@ cmd_foreach(void) fd->flags |= FOREACH_m_FLAG; break; + case 'M': + fd->flags |= FOREACH_M_FLAG; + break; + case 'l': fd->flags |= FOREACH_l_FLAG; break; @@ -6140,6 +6144,13 @@ foreach(struct foreach_data *fd) print_header = FALSE; break; + case FOREACH_FILES: + if (fd->flags & FOREACH_m_FLAG) + error(FATAL, + "foreach files command does not " + "support -m option\n"); + break; + case FOREACH_TEST: break; } @@ -6366,9 +6377,15 @@ foreach(struct foreach_data *fd) case FOREACH_FILES: pc->curcmd = "files"; - open_files_dump(tc->task, - fd->flags & FOREACH_i_FLAG ? - PRINT_INODES : 0, + cmdflags = 0; + + if (fd->flags & FOREACH_i_FLAG) + cmdflags |= PRINT_INODES; + if (fd->flags & FOREACH_M_FLAG) + cmdflags |= PRINT_PAGES; + + open_files_dump(tc->task, + cmdflags, fd->reference ? ref : NULL); break; -- 1.9.1 -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility

Date: Thu, 18 Jun 2015 17:18:27 -0400 From: anderson(a)redhat.com To: crash-utility(a)redhat.com Subject: Re: [Crash-utility] [PATCH v2] files: support dump file pages from its address space

Hi Oliver,> I have only had a chance to look at the patch contents, and haven't been able to fully test it. I'll try to do that tomorrow, or if not, next week. In the meantime, I do have a few comments regarding the patch itself that need to be addressed. The major problem is maintaining the API for extension modules. Any changes to defs.h such as the function prototypes or structure declarations that are exported in that file should not require the re-compilation of pre-existing extension modules.

One simple example is this addition you make to the offset_table: @@ -1416,6 +1417,7 @@ struct offset_table { /* stash of commonly-used offsets */ long inode_i_flock; long inode_i_fop; long inode_i_mapping; + long address_space_page_tree; long address_space_nrpages; long vfsmount_mnt_next; long vfsmount_mnt_devname; By inserting the new address_space_page_tree member where you did, it will break the usage of OFFSET() for all subsequent members. Note this comment at the beginning of the structure: struct offset_table { /* stash of commonly-used offsets */ long list_head_next; /* add new entries to end of table */ ... So please put address_space_page_tree at the end of the offset_table structure.

On the other hand, please add an entry to the dump_offset_table() function for use by "help -o", and in that function you can place the address_space_page_tree display right next to address_space_nrpages.

But more importantly with respect to the API is the change where you added the callback argument to the do_radix_tree() function. The do_radix_tree API cannot be changed without breaking any pre-existing extension modules that currently use it.

Here's what I suggest. Instead of using RADIX_TREE_DUMP, create a new flag RADIX_TREE_DUMP_CALLBACK, and you can store the callback function address inside the general purpose pc->curcmd_private member.

And lastly (for now), I am a big believer in keeping things as simple as possible. You have added all of these constructs: + #define AS_PAGE_TREE (0x20000000) + #define IS_AS_PAGE_TREE() (vt->flags & AS_PAGE_TREE) + MEMBER_OFFSET_INIT(address_space_page_tree, "address_space", "page_tree"); + if (VALID_MEMBER(address_space_page_tree)) + vt->flags |= AS_PAGE_TREE; + /* + * Check the availability of address space page tree + */ + int + is_as_page_tree_supported(void) + { + return (IS_AS_PAGE_TREE() ? TRUE : FALSE); + } and where your code calls it twice: + if (is_as_page_tree_supported()) { + if (is_as_page_tree_supported()) Please get rid of AS_PAGE_TREE, IS_AS_PAGE_TREE() and is_as_page_tree_supported() and simply use "if (VALID_MEMBER(address_space_page_tree))". That's precisely the kind of situation that VALID_MEMBER() is meant to be used for.

Thanks, Dave > > > files: support dump file pages from its address space > > Added two options in files command, > > 1. -M option, which allows dump address space and page number for each files > 2. -m option, which could dump each pages in given address mapping > > The foreach command also could work with -M, so that we can easily > find which processes/files hold most page cache pages within the system. > > Signed-off-by: Yong Yang <yangoliver(a)gmail.com&gt; > --- > defs.h | 11 ++++++- > filesys.c | 111 > +++++++++++++++++++++++++++++++++++++++++++++++++++----------- > kernel.c | 4 +-- > memory.c | 73 +++++++++++++++++++++++++++++++++++++++++ > task.c | 25 +++++++++++--- > 5 files changed, 197 insertions(+), 27 deletions(-) > > diff --git a/defs.h b/defs.h > index b25b505..608e09f 100644 > --- a/defs.h > +++ b/defs.h > @@ -1111,6 +1111,7 @@ extern struct machdep_table *machdep; > #define FOREACH_a_FLAG (0x4000000) > #define FOREACH_G_FLAG (0x8000000) > #define FOREACH_F_FLAG2 (0x10000000) > +#define FOREACH_M_FLAG (0x20000000) > > #define FOREACH_PS_EXCLUSIVE \ > (FOREACH_g_FLAG|FOREACH_a_FLAG|FOREACH_t_FLAG|FOREACH_c_FLAG|FOREACH_p_FLAG|FOREACH_l_FLAG|FOREACH_r_FLAG|FOREACH_m_FLAG) > @@ -1416,6 +1417,7 @@ struct offset_table { /* stash of > commonly-used offsets */ > long inode_i_flock; > long inode_i_fop; > long inode_i_mapping; > + long address_space_page_tree; > long address_space_nrpages; > long vfsmount_mnt_next; > long vfsmount_mnt_devname; > @@ -2286,11 +2288,13 @@ struct vm_table { /* kernel VM-related > data */ > #define PAGEFLAGS (0x4000000) > #define SLAB_OVERLOAD_PAGE (0x8000000) > #define SLAB_CPU_CACHE (0x10000000) > +#define AS_PAGE_TREE (0x20000000) > > #define IS_FLATMEM() (vt->flags & FLATMEM) > #define IS_DISCONTIGMEM() (vt->flags & DISCONTIGMEM) > #define IS_SPARSEMEM() (vt->flags & SPARSEMEM) > #define IS_SPARSEMEM_EX() (vt->flags & SPARSEMEM_EX) > +#define IS_AS_PAGE_TREE() (vt->flags & AS_PAGE_TREE) > > #define COMMON_VADDR_SPACE() (vt->flags & COMMON_VADDR) > #define PADDR_PRLEN (vt->paddr_prlen) > @@ -2598,6 +2602,7 @@ struct load_module { > #define PRINT_SINGLE_VMA (0x80) > #define PRINT_RADIX_10 (0x100) > #define PRINT_RADIX_16 (0x200) > +#define PRINT_PAGES (0x400) > > #define MIN_PAGE_SIZE (4096) > > @@ -4707,6 +4712,9 @@ void alter_stackbuf(struct bt_info *); > int vaddr_type(ulong, struct task_context *); > char *format_stack_entry(struct bt_info *bt, char *, ulong, ulong); > int in_user_stack(ulong, ulong); > +void dump_file_address_mappings(ulong); > +long get_page_tree_count(ulong i_mapping); > +int is_as_page_tree_supported(void); > > /* > * filesys.c > @@ -4747,12 +4755,13 @@ struct radix_tree_pair { > ulong index; > void *value; > }; > -ulong do_radix_tree(ulong, int, struct radix_tree_pair *); > +ulong do_radix_tree(ulong, int, struct radix_tree_pair *, int (*)(ulong)); > int file_dump(ulong, ulong, ulong, int, int); > #define DUMP_FULL_NAME 1 > #define DUMP_INODE_ONLY 2 > #define DUMP_DENTRY_ONLY 4 > #define DUMP_EMPTY_FILE 8 > +#define DUMP_FILE_PAGE 16 > #endif /* !GDB_COMMON */ > int same_file(char *, char *); > #ifndef GDB_COMMON > diff --git a/filesys.c b/filesys.c > index 0573fe6..f0ec78b 100644 > --- a/filesys.c > +++ b/filesys.c > @@ -2187,11 +2187,12 @@ cmd_files(void) > int subsequent; > struct reference reference, *ref; > char *refarg; > + int open_flags = 0; > > ref = NULL; > refarg = NULL; > > - while ((c = getopt(argcnt, args, "d:R:")) != EOF) { > + while ((c = getopt(argcnt, args, "d:R:m:M")) != EOF) { > switch(c) > { > case 'R': > @@ -2209,7 +2210,20 @@ cmd_files(void) > value = htol(optarg, FAULT_ON_ERROR, NULL); > display_dentry_info(value); > return; > - > + case 'm': > + if (is_as_page_tree_supported()) { > + value = htol(optarg, FAULT_ON_ERROR, NULL); > + dump_file_address_mappings(value); > + } else { > + option_not_supported('m'); > + } > + return; > + case 'M': > + if (is_as_page_tree_supported()) > + open_flags |= PRINT_PAGES; > + else > + option_not_supported('M'); > + break; > default: > argerrs++; > break; > @@ -2222,7 +2236,9 @@ cmd_files(void) > if (!args[optind]) { > if (!ref) > print_task_header(fp, CURRENT_CONTEXT(), 0); > - open_files_dump(CURRENT_TASK(), 0, ref); > + > + open_files_dump(CURRENT_TASK(), open_flags, ref); > + > return; > } > > @@ -2241,7 +2257,7 @@ cmd_files(void) > for (tc = pid_to_context(value); tc; tc = > tc->tc_next) { > if (!ref) > print_task_header(fp, tc, > subsequent); > - open_files_dump(tc->task, 0, ref); > + open_files_dump(tc->task, open_flags, ref); > fprintf(fp, "\n"); > } > break; > @@ -2249,7 +2265,7 @@ cmd_files(void) > case STR_TASK: > if (!ref) > print_task_header(fp, tc, subsequent); > - open_files_dump(tc->task, 0, ref); > + open_files_dump(tc->task, open_flags, ref); > break; > > case STR_INVALID: > @@ -2321,6 +2337,7 @@ open_files_dump(ulong task, int flags, struct reference > *ref) > char buf4[BUFSIZE]; > char root_pwd[BUFSIZE]; > int root_pwd_printed = 0; > + int file_dump_flags = 0; > > BZERO(root_pathname, BUFSIZE); > BZERO(pwd_pathname, BUFSIZE); > @@ -2329,15 +2346,27 @@ open_files_dump(ulong task, int flags, struct > reference *ref) > fdtable_buf = GETBUF(SIZE(fdtable)); > fill_task_struct(task); > > - sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n", > - space(MINSPACE), > - mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "FILE"), > - space(MINSPACE), > - mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "DENTRY"), > - space(MINSPACE), > - mkstring(buf3, VADDR_PRLEN, CENTER|LJUST, "INODE"), > - space(MINSPACE), > - space(MINSPACE)); > + if (flags & PRINT_PAGES) { > + sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n", > + space(MINSPACE), > + mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "ADDR-SPACE"), > + space(MINSPACE), > + mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "PAGE-COUNT"), > + space(MINSPACE), > + mkstring(buf3, VADDR_PRLEN, CENTER|LJUST, "INODE"), > + space(MINSPACE), > + space(MINSPACE)); > + } else { > + sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n", > + space(MINSPACE), > + mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "FILE"), > + space(MINSPACE), > + mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "DENTRY"), > + space(MINSPACE), > + mkstring(buf3, VADDR_PRLEN, CENTER|LJUST, "INODE"), > + space(MINSPACE), > + space(MINSPACE)); > + } > > tc = task_to_context(task); > > @@ -2523,6 +2552,10 @@ open_files_dump(ulong task, int flags, struct > reference *ref) > return; > } > > + file_dump_flags = DUMP_FULL_NAME | DUMP_EMPTY_FILE; > + if (flags & PRINT_PAGES) > + file_dump_flags |= DUMP_FILE_PAGE; > + > j = 0; > for (;;) { > unsigned long set; > @@ -2539,8 +2572,7 @@ open_files_dump(ulong task, int flags, struct reference > *ref) > > if (ref && file) { > open_tmpfile(); > - if (file_dump(file, 0, 0, i, > - DUMP_FULL_NAME|DUMP_EMPTY_FILE)) > { > + if (file_dump(file, 0, 0, i, > file_dump_flags)) { > BZERO(buf4, BUFSIZE); > rewind(pc->tmpfile); > ret = fgets(buf4, BUFSIZE, > @@ -2558,8 +2590,7 @@ open_files_dump(ulong task, int flags, struct reference > *ref) > fprintf(fp, "%s", files_header); > header_printed = 1; > } > - file_dump(file, 0, 0, i, > - DUMP_FULL_NAME|DUMP_EMPTY_FILE); > + file_dump(file, 0, 0, i, file_dump_flags); > } > } > i++; > @@ -2754,6 +2785,8 @@ file_dump(ulong file, ulong dentry, ulong inode, int > fd, int flags) > char buf1[BUFSIZE]; > char buf2[BUFSIZE]; > char buf3[BUFSIZE]; > + ulong i_mapping = 0; > + ulong count = 0; > > file_buf = NULL; > > @@ -2863,6 +2896,28 @@ file_dump(ulong file, ulong dentry, ulong inode, int > fd, int flags) > type, > space(MINSPACE), > pathname+1); > + } else if (flags & DUMP_FILE_PAGE) { > + i_mapping = ULONG(inode_buf + OFFSET(inode_i_mapping)); > + count = get_page_tree_count(i_mapping); > + > + fprintf(fp, "%3d%s%s%s%s%s%s%s%s%s%s\n", > + fd, > + space(MINSPACE), > + mkstring(buf1, VADDR_PRLEN, > + CENTER|RJUST|LONG_HEX, > + MKSTR(i_mapping)), > + space(MINSPACE), > + mkstring(buf2, VADDR_PRLEN, > + CENTER|RJUST|LONG_DEC, > + MKSTR(count)), > + space(MINSPACE), > + mkstring(buf3, VADDR_PRLEN, > + CENTER|RJUST|LONG_HEX, > + MKSTR(inode)), > + space(MINSPACE), > + type, > + space(MINSPACE), > + pathname); > } else { > fprintf(fp, "%3d%s%s%s%s%s%s%s%s%s%s\n", > fd, > @@ -3877,9 +3932,13 @@ ulong RADIX_TREE_MAP_MASK = UNINITIALIZED; > * RADIX_TREE_GATHER; the dimension (max count) of the array may > * be stored in the index field of the first structure to avoid > * any chance of an overrun. > + * > + * entry_ops: The operation against each of returned entry value. > + * Only used by RADIX_TREE_DUMP. > */ > ulong > -do_radix_tree(ulong root, int flag, struct radix_tree_pair *rtp) > +do_radix_tree(ulong root, int flag, struct radix_tree_pair *rtp, > + int (*entry_ops)(ulong)) > { > int i, ilen, height; > long nlen; > @@ -3970,7 +4029,19 @@ do_radix_tree(ulong root, int flag, struct > radix_tree_pair *rtp) > for (index = count = 0; index <= maxindex; index++) { > if ((ret = > radix_tree_lookup(root_rnode, index, height))) { > - fprintf(fp, "[%ld] %lx\n", index, (ulong)ret); > + if (entry_ops == NULL) { > + /* Default operation */ > + fprintf(fp, "[%ld] %lx\n", > + index, (ulong)ret); > + } else { > + /* Caller defined operation */ > + if (entry_ops((ulong)ret) != 0) { > + error(FATAL, "do_radix_tree: " > + "dump operation failed, " > + "count: %ld\n", count); > + return -EIO; > + } > + } > count++; > } > } > diff --git a/kernel.c b/kernel.c > index cb8084a..53d2e1d 100644 > --- a/kernel.c > +++ b/kernel.c > @@ -5746,12 +5746,12 @@ get_irq_desc_addr(int irq) > return addr; > > cnt = do_radix_tree(symbol_value("irq_desc_tree"), > - RADIX_TREE_COUNT, NULL); > + RADIX_TREE_COUNT, NULL, NULL); > len = sizeof(struct radix_tree_pair) * (cnt+1); > rtp = (struct radix_tree_pair *)GETBUF(len); > rtp[0].index = cnt; > cnt = do_radix_tree(symbol_value("irq_desc_tree"), > - RADIX_TREE_GATHER, rtp); > + RADIX_TREE_GATHER, rtp, NULL); > > if (kt->highest_irq == 0) > kt->highest_irq = rtp[cnt-1].index; > diff --git a/memory.c b/memory.c > index 765732b..0102dbc 100644 > --- a/memory.c > +++ b/memory.c > @@ -292,6 +292,7 @@ static void dump_per_cpu_offsets(void); > static void dump_page_flags(ulonglong); > static ulong kmem_cache_nodelists(ulong); > static void dump_hstates(void); > +static int dump_file_page(ulong); > > /* > * Memory display modes specific to this file. > @@ -476,6 +477,9 @@ vm_init(void) > MEMBER_OFFSET_INIT(block_device_bd_list, "block_device", "bd_list"); > MEMBER_OFFSET_INIT(block_device_bd_disk, "block_device", "bd_disk"); > MEMBER_OFFSET_INIT(inode_i_mapping, "inode", "i_mapping"); > + MEMBER_OFFSET_INIT(address_space_page_tree, "address_space", "page_tree"); > + if (VALID_MEMBER(address_space_page_tree)) > + vt->flags |= AS_PAGE_TREE; > MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "nrpages"); > if (INVALID_MEMBER(address_space_nrpages)) > MEMBER_OFFSET_INIT(address_space_nrpages, "address_space", "__nrpages"); > @@ -6465,6 +6469,75 @@ translate_page_flags(char *buffer, ulong flags) > } > > /* > + * Page tree dump ops. > + */ > +static int > +dump_file_page(ulong page) > +{ > + struct meminfo meminfo; > + > + BZERO(&meminfo, sizeof(struct meminfo)); > + meminfo.spec_addr = page; > + meminfo.memtype = KVADDR; > + meminfo.flags = ADDRESS_SPECIFIED; > + dump_mem_map(&meminfo); > + > + return 0; > +} > + > +/* > + * The address space file mapping radix tree walker. > + */ > +void > +dump_file_address_mappings(ulong i_mapping) > +{ > + ulong root_rnode; > + ulong count; > + > + root_rnode = i_mapping + OFFSET(address_space_page_tree); > + count = get_page_tree_count(i_mapping); > + fprintf(fp, "Address Space %lx, page tree %lx, %ld pages\n\n", > + i_mapping, root_rnode, count); > + > + /* Dump each pages in radix tree */ > + (void) do_radix_tree(root_rnode, RADIX_TREE_DUMP, > + NULL, &dump_file_page); > + > + return; > +} > + > +/* > + * Get the page count for the specific mapping > + */ > +long > +get_page_tree_count(ulong i_mapping) > +{ > + ulong address_space = i_mapping; > + char *address_space_buf; > + ulong nrpages = 0; > + > + address_space_buf = GETBUF(SIZE(address_space)); > + > + readmem(address_space, KVADDR, address_space_buf, > + SIZE(address_space), "address_space buffer", > + FAULT_ON_ERROR); > + nrpages = ULONG(address_space_buf + OFFSET(address_space_nrpages)); > + > + FREEBUF(address_space_buf); > + > + return nrpages; > +} > + > +/* > + * Check the availability of address space page tree > + */ > +int > +is_as_page_tree_supported(void) > +{ > + return (IS_AS_PAGE_TREE() ? TRUE : FALSE); > +} > + > +/* > * dump_page_hash_table() displays the entries in each page_hash_table. > */ > > diff --git a/task.c b/task.c > index 45be68c..4c95259 100644 > --- a/task.c > +++ b/task.c > @@ -5612,7 +5612,7 @@ cmd_foreach(void) > BZERO(&foreach_data, sizeof(struct foreach_data)); > fd = &foreach_data; > > - while ((c = getopt(argcnt, args, "R:vomlgersStTpukcfFxhdaG")) != > EOF) { > + while ((c = getopt(argcnt, args, "R:vomMlgersStTpukcfFxhdaG")) != > EOF) { > switch(c) > { > case 'R': > @@ -5636,6 +5636,10 @@ cmd_foreach(void) > fd->flags |= FOREACH_m_FLAG; > break; > > + case 'M': > + fd->flags |= FOREACH_M_FLAG; > + break; > + > case 'l': > fd->flags |= FOREACH_l_FLAG; > break; > @@ -6140,6 +6144,13 @@ foreach(struct foreach_data *fd) > print_header = FALSE; > break; > > + case FOREACH_FILES: > + if (fd->flags & FOREACH_m_FLAG) > + error(FATAL, > + "foreach files command does not " > + "support -m option\n"); > + break; > + > case FOREACH_TEST: > break; > } > @@ -6366,9 +6377,15 @@ foreach(struct foreach_data *fd) > > case FOREACH_FILES: > pc->curcmd = "files"; > - open_files_dump(tc->task, > - fd->flags & FOREACH_i_FLAG ? > - PRINT_INODES : 0, > + cmdflags = 0; > + > + if (fd->flags & FOREACH_i_FLAG) > + cmdflags |= PRINT_INODES; > + if (fd->flags & FOREACH_M_FLAG) > + cmdflags |= PRINT_PAGES; > + > + open_files_dump(tc->task, > + cmdflags, > fd->reference ? ref : NULL); > break; > > -- > 1.9.1 > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility

> I fixed line up problem in v3 patch, please check v3 patch in my another mail. > The major problem here is ADDR-SPACE and PAGE-COUNT width is greater than long size on 32 bit kernel. OK, thanks. But as far as the output display goes, I don't particularly like the naming of the "PAGE-CNT" column header. I'd prefer that it be changed to reflect the "address_space.nrpages" member that it comes from. I also think that the page count should be right-justified. So with this additional patch: --- filesys.c_oliver 2015-06-25 15:30:44.549942532 -0400 +++ filesys.c 2015-06-25 15:28:17.157947564 -0400 @@ -2370,13 +2370,12 @@ fill_task_struct(task); if (flags & PRINT_PAGES) { - sprintf(files_header, " FD%s%s%s%s%s%s%sTYPE%sPATH\n", + sprintf(files_header, " FD%s%s%s%s%sNRPAGES%sTYPE%sPATH\n", space(MINSPACE), mkstring(buf1, VADDR_PRLEN, CENTER|LJUST, "INODE"), space(MINSPACE), mkstring(buf2, VADDR_PRLEN, CENTER|LJUST, "MAPPING"), space(MINSPACE), - mkstring(buf3, LONG_PRLEN, CENTER|LJUST, "PAGE-CNT"), space(MINSPACE), space(MINSPACE)); } else { @@ -2935,8 +2934,8 @@ CENTER|RJUST|LONG_HEX, MKSTR(i_mapping)), space(MINSPACE), - mkstring(buf3, LONG_PRLEN, - CENTER|RJUST|LONG_DEC, + mkstring(buf3, strlen("NRPAGES"), + RJUST|LONG_DEC, MKSTR(count)), space(MINSPACE), type,

>> And secondly, taking the address_space e6c6a754 from the task above, >> again, shouldn't the page count above be reflected in the number of >> shown by the address_space tree dump, where the page dump seems to >> be missing about 20000 pages?: >> >> crash> files -m e6c6a754 | wc -l >> 128825 >> crash> > > I can't reproduce this issue on my 32bit kernel. > > Could you verify whether you can get the same number of pages by tree command? > > First step, get the page_tree address, > > address_space.page_tree e6c6a754 > > Second step, using tree command dump pages, > > tree -t radix -N <page_tree address> As it turns out, that dumpfile was created with the "snap.so" extension module, which creates a vmcore while running on a live system. And since the kernel is running while the dumpfile is being created, you may see any number of odd outputs. Running the "tree" command above fails with a duplicate list entry failure, so the "files -a" command should also fail prematurely. So it's not a problem.

>> >> For that matter, displaying the address_space address is redundant >> since (1) it has to be entered as the command argument, and (2) it gets >> shown in every page line "MAPPING". On the other hand, perhaps the inode >> that contains the address_space structure would be helpful, say, like >> this: > > In my v3 patch, I files -a accept inode as parameter, instead of address space address. OK, although I suggested "-a" because it was taking an "address_space" address as an argument. So perhaps it should be "-i <inode-address>"?

> The benefit of this change is, files -a is more separate with files -m. > > We decoupled two commands here. The first line of the page dump is kind of unusual: crash> files -a ffff8801085680b0 Address space ffff8801085681f0, 35002 pages PAGE PHYSICAL MAPPING INDEX CNT FLAGS ffffea00072ca3c0 1cb28f000 ffff8801085681f0 0 1 5ff0000002006c referenced,uptodate,lru,active,mappedtodisk ffffea0000c1f340 307cd000 ffff8801085681f0 200 1 3ff0000002006c referenced,uptodate,lru,active,mappedtodisk ffffea00030dea40 c37a9000 ffff8801085681f0 201 1 3ff0000002006c referenced,uptodate,lru,active,mappedtodisk ... Can you make it show INODE, ADDRESS_SPACE, and NRPAGES values (in that order), similar to how I suggested before? > But how about using -p instead of -a for page dump? Yes, sure -- either -i or -p makes sense.

So, aside from the patch I added above, the addition of the INODE, ADDRESS_SPACE and NRPAGES display to the top of the "files -i|p" command, and the help page additions, this is looking pretty good.

Date: Fri, 19 Jun 2015 12:23:39 -0400 From: anderson(a)redhat.com To: crash-utility(a)redhat.com Subject: Re: [Crash-utility] [PATCH v2] files: support dump file pages from its address space > > Here's what I suggest. Instead of using RADIX_TREE_DUMP, create a new flag > > RADIX_TREE_DUMP_CALLBACK, and you can store the callback function address inside > > the general purpose pc->curcmd_private member. > > I'm ok with use a different flag. Although I didn't find any source codes are > using RADIX_TREE_DUMP flag. > > But I had some concerns for storing callback address in pc->curcmd_private. > > I noticed that the pc->curcmd_private is already used by other crash code at > many locations. Right, but only within the context of specific commands, such as "irq -u", "vm -M", "search", "struct -f" and "runq -c". Similar to using GETBUF(), the pc->curcmd_private is only in effect within the period of time a particular command is running, and is cleared by restore_sanity() prior to the next "crash> " prompt. > > > If someday people want to use the RADIX_TREE_DUMP_CALLBACK at that context, > it may cause the troubles. How? If they want to do so within the context of a different command, they can set the pc->curcmd_private field to their own function. That's what it's designed to be used for.

> If we really don't want to add new args for do_radix_tree, maybe we can > define a new global function pointer for this purpose. > > If you don't like my suggestion, we also can consider to use RADIX_TREE_SEARCH. Yes I suppose you could define a global pointer, but it doesn't offer any more benefit or safety than using pc->curcmd_private. You could use RADIX_TREE_SEARCH, or perhaps RADIX_TREE_COUNT/RADIX_TREE_GATHER. But both of those options would seemingly be inefficient for large lists? Having an option that utilizes a callback function does seem to be the best way to do it. How about this: have your new RADIX_TREE_DUMP_CALLBACK option use the rtp->value (void *) member to store the callback function? As long as it's documented, I would have no problem accepting that as well.