3:33 p.m.
----- Original Message -----
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>
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,
The 32-bit display looks like this:
crash> files -m
PID: 5613 TASK: f56fa030 CPU: 4 COMMAND: "beah-beaker-bac"
ROOT: / CWD: /
FD INODE MAPPING NRPAGES TYPE PATH
0 f713f7a0 f713f850 0 CHR /dev/null
1 f5ab1b1c f5ab1bcc 1 REG /tmp/beah-beaker-backend.out
2 f5ab1b1c f5ab1bcc 1 REG /tmp/beah-beaker-backend.out
3 f5a08768 f5a08818 0 SOCK
4 f2c07854 f2c07904 0 REG /var/log/beah_beaker_backend.log
5 f2c0fd64 f2c0fe14 17 REG /var/beah/beah_beaker_backend.runtime
6 f2c0f344 f2c0f3f4 3543 REG /var/beah/journals/beakerlc.journal
7 f2c660fc f2c661ac 1 REG /tmp/ffiu1qDpS
8 f5b264a8 f5b26558 0 FIFO
...
And the 64-bit display looks like this:
PID: 5329 TASK: ffff88011b1b3c80 CPU: 3 COMMAND: "WorkerPool/5329"
ROOT: / CWD: /home/anderson
FD INODE MAPPING NRPAGES TYPE PATH
0 ffff880212be0558 ffff880212be0698 0 CHR /dev/null
1 ffff8802157eb5e8 ffff8802157eb728 0 FIFO
2 ffff8802157eab20 ffff8802157eac60 0 FIFO
3 ffff8800061b1320 ffff8800061b1460 222 REG /opt/google/chrome/icudtl.dat
4 ffff880089d2ce20 ffff880089d2cf60 70 REG /opt/google/chrome/natives_blob.bin
5 ffff880089d32cf0 ffff880089d32e30 140 REG /opt/google/chrome/snapshot_blob.bin
... [ cut ] ...
107 ffff8801013991b0 ffff8801013992f0 0 SOCK UNIX
108 ffff8800aee30460 ffff8800aee305a0 91 REG
/home/anderson/.config/google-chrome/Default/Current Session
109 ffff88011f4c76b0 ffff88011f4c77f0 129 REG
/home/anderson/.cache/google-chrome/Default/Cache/index
110 ffff88011f4c7a60 ffff88011f4c7ba0 326 REG
/home/anderson/.cache/google-chrome/Default/Cache/data_0
114 ffff88020fc2eba0 ffff88020fc2ece0 0 REG
/home/anderson/.config/google-chrome/Default/GCM Store/LOG
115 ffff88004f2b2d30 ffff88004f2b2e70 0 SOCK UNIX
116 ffff88004f2b0030 ffff88004f2b0170 0 SOCK UNIX
117 ffff88020a17dce0 ffff88020a17de20 4 REG
/home/anderson/.config/google-chrome/Default/Web Data-journal
118 ffff88000ff22358 ffff88000ff22498 1 REG /dev/shm/.com.google.Chrome.n6ezvQ
119 ffff8800c762afd8 ffff8800c762b118 256 REG /dev/shm/.com.google.Chrome.NdcC9M
120 ffff8800c7629458 ffff8800c7629598 142 REG /dev/shm/.com.google.Chrome.EmeSNJ
122 ffff8801ed1a8460 ffff8801ed1a85a0 4043 REG
/home/anderson/.cache/google-chrome/Default/Cache/data_1
123 ffff8801ed1a9a80 ffff8801ed1a9bc0 1821 REG
/home/anderson/.cache/google-chrome/Default/Cache/data_4
124 ffff8801ed1a9e30 ffff8801ed1a9f70 4898 REG
/home/anderson/.cache/google-chrome/Default/Cache/data_2
125 ffff8801ed1aa1e0 ffff8801ed1aa320 11718 REG
/home/anderson/.cache/google-chrome/Default/Cache/data_3
...
> 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 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
I removed page_tree address in patch v3.
>
> 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:
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.
> And a couple other things I forgot to mention...
>
> The vast majority of the "files -M" output consists of either the file
> descriptors for CHR, FIFO, SOCK, etc., files, or REG file descriptors
> that don't have any pages mapped into the task's address space.
>
> (1) Wouldn't it make more sense to show *only* the file descriptors
> that have mapped pages? And if the task has no mapped pages,
> just display "(no mapped pages)".
Confirmed some files mapped zero pages is also useful for debugging.
In theory, all of files/inodes have the mapping(address_space), per its inode data
structure.
The command just reflects the inode data structure.
In current kernel implementation, only REG and BLK types used the mapping.
Most important reason is, we can reuse most existing code in filesys.c.
> (2) Is there any good reason to show the ROOT and CWD directories?
Same with above. Because code reuse.
All these behaviors are inherited from original files cmd code.
For example, when -R is specified, the ROOT and CWD is required by original code.
Because we want -R keep working with -m, I don't want to change the behavior.
You can see -R could work with -m option very well, because we share the common code.
Below command can filter all files with REG type,
crash> foreach files -m -R REG
PID: 1 TASK: ffff880139b20000 CPU: 2 COMMAND: "systemd"
ROOT: / CWD: /
FD INODE MAPPING PAGE-CNT TYPE PATH
11 ffff88013ac2d678 ffff88013ac2d7c0 0 REG /proc/1/mountinfo
12 ffff880138e6ea38 ffff880138e6eb80 0 REG /proc/swaps
PID: 422 TASK: ffff880135f74c40 CPU: 2 COMMAND: "systemd-journal"
ROOT: / CWD: /
FD INODE MAPPING PAGE-CNT TYPE PATH
13 ffff880135267050 ffff880135267198 17813 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/system.journal
34 ffff8801352e80b0 ffff8801352e81f8 652 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/user-42.journal
36 ffff8801352e9090 ffff8801352e91d8 905 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/user-530.journal
38 ffff8801352e8c98 ffff8801352e8de0 1 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/user-995.journa
Below command can filter all files with journal pattern in the filename,
foreach files -m -R journal
PID: 422 TASK: ffff880135f74c40 CPU: 2 COMMAND: "systemd-journal"
ROOT: / CWD: /
FD INODE MAPPING PAGE-CNT TYPE PATH
13 ffff880135267050 ffff880135267198 17813 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/system.journal
34 ffff8801352e80b0 ffff8801352e81f8 652 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/user-42.journal
36 ffff8801352e9090 ffff8801352e91d8 905 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/user-530.journal
38 ffff8801352e8c98 ffff8801352e8de0 1 REG
/var/log/journal/2d6f0d3073ff4a60b1e52a8e38e48feb/user-995.journal
OK, you've convinced me... ;-)
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.
Thanks,
Dave