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

[Dave Anderson]

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.

Thanks,
  Dave