Re: [Crash-utility] [PATCH v2] files: support dump file pages from its address space

Date: Fri, 19 Jun 2015 15:40:22 -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, A few more comments and suggestions regarding your patch. A couple things I noted when testing on a 32-bit x86. First, the columns don't line up correctly: crash> files -M 3804 PID: 3804 TASK: f466a5e0 CPU: 0 COMMAND: "crash" ROOT: / CWD: /root/crash-5.1.8 FD ADDR-SPACE PAGE-COUNT INODE TYPE PATH 0 e6d6f63c 0 e6d6f570 CHR /dev/pts/0 1 e6d6f63c 0 e6d6f570 CHR /dev/pts/0 2 e6d6f63c 0 e6d6f570 CHR /dev/pts/0 3 f53ec874 0 f53ec7a8 CHR /dev/null 4 f02944d4 0 f0294408 CHR /dev/crash 5 e6c6a294 0 e6c6a1c8 REG /tmp/tmpfvd9PjN 6 e6ca9e54 0 e6ca9d88 FIFO 7 e6ca9e54 0 e6ca9d88 FIFO 8 e6c6a754 147034 e6c6a688 REG /root/crash-5.1.8/snapshot-2.6.40.4-5.fc15.i686.PAE 9 e6caa3f4 0 e6caa328 FIFO crash>

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>

For the address_space page tree dump, I don't see any point in displaying the page_tree member address: crash> files -m ffff810218e31220 Address Space ffff810218e31220, page tree ffff810218e31228, 12 pages

PAGE PHYSICAL MAPPING INDEX CNT FLAGS ffff81010774d1f8 221609000 ffff810218e31220 0 1 22010000001006c ffff81010485b378 14ac59000 ffff810218e31220 1 1 14810000001006c ffff810107993a78 22bc79000 ffff810218e31220 2 1 228100000010028 ffff8101049d3660 1517d4000 ffff810218e31220 3 1 150100000010028 ffff810103b29670 10e742000 ffff810218e31220 4 1 108100000010028 ffff810106d51ba0 1f3bec000 ffff810218e31220 5 1 1f0100000010028 ffff810103ac95f0 10cbd2000 ffff810218e31220 6 1 108100000010028 ffff810106c8cc18 1f03a5000 ffff810218e31220 7 1 1f0100000010028 ffff8101077b1028 223293000 ffff810218e31220 8 1 220100000010028 ffff810106cc03b0 1f125a000 ffff810218e31220 9 1 1f0100000010028 ffff810107a04cd8 22dccd000 ffff810218e31220 a 1 228100000010028 ffff8101078741b8 226a51000 ffff810218e31220 b 1 220100000010028 crash> 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:

crash> files -m ffff810218e31220 ADDRESS_SPACE INODE PAGES ffff810218e31220 ffff810218e310e0 12 PAGE PHYSICAL MAPPING INDEX CNT FLAGS ffff81010774d1f8 221609000 ffff810218e31220 0 1 22010000001006c ffff81010485b378 14ac59000 ffff810218e31220 1 1 14810000001006c ffff810107993a78 22bc79000 ffff810218e31220 2 1 228100000010028 ffff8101049d3660 1517d4000 ffff810218e31220 3 1 150100000010028 ffff810103b29670 10e742000 ffff810218e31220 4 1 108100000010028 ffff810106d51ba0 1f3bec000 ffff810218e31220 5 1 1f0100000010028 ffff810103ac95f0 10cbd2000 ffff810218e31220 6 1 108100000010028 ffff810106c8cc18 1f03a5000 ffff810218e31220 7 1 1f0100000010028 ffff8101077b1028 223293000 ffff810218e31220 8 1 220100000010028 ffff810106cc03b0 1f125a000 ffff810218e31220 9 1 1f0100000010028 ffff810107a04cd8 22dccd000 ffff810218e31220 a 1 228100000010028 ffff8101078741b8 226a51000 ffff810218e31220 b 1 220100000010028 crash> Also, I usually try to avoid using upper-case capital letters as arguments unless there aren't any more logical lower-case letters left to use. How about changing your "files -M" and "files -m" to something like:

crash> files -m [pid|task] (for your current files -M) and crash> files -a <address_space> (for your current "files -m"); And don't forget to update the help_files[] array with your new functionality.