[Crash-utility] [PATCH v2] files: support dump file pages from its address space
Dave Anderson
anderson at redhat.com
Thu Jun 25 20:33:40 UTC 2015
----- 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
More information about the Crash-utility
mailing list