[Crash-utility] [PATCH] add -s option to vm to display one vma at a time
qiaonuohan
qiaonuohan at cn.fujitsu.com
Thu Mar 15 02:31:33 UTC 2012
At 2012-3-15 3:06, Dave Anderson wrote:
>
>
> ----- Original Message -----
>> At 2012-3-14 4:45, Dave Anderson wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> At 2012-3-13 17:56, qiaonuohan wrote:
>>>>
>>>> Hello Dave,
>>>>
>>>> When I am using "vm -p" command, I feel it is chaotic when too
>>>> much data is printed. I think it is clear to display one vma
>>>> each time.
>>>>
>>>> In the patch, I compare all vmas with the argument of -s option.
>>>> If an equal vma is found, it will be printed like below.
>>>>
>>>> crash> vm -ps ffff88028ff7d3a0
>>>> VMA START END FLAGS FILE
>>>> ffff88028ff7d3a0 7fff29b71000 7fff29b87000 100173
>>>> VIRTUAL PHYSICAL
>>>> 7fff29b71000 (not mapped)
>>>> 7fff29b72000 (not mapped)
>>>> 7fff29b73000 (not mapped)
>>>> 7fff29b74000 (not mapped)
>>>> 7fff29b75000 (not mapped)
>>>> 7fff29b76000 (not mapped)
>>>> 7fff29b77000 27faad000
>>>> 7fff29b78000 2807aa000
>>>> 7fff29b79000 280781000
>>>> 7fff29b7a000 280787000
>>>> 7fff29b7b000 280776000
>>>> 7fff29b7c000 280786000
>>>> 7fff29b7d000 27f2df000
>>>> 7fff29b7e000 27f2e0000
>>>> 7fff29b7f000 27f2e1000
>>>> 7fff29b80000 27f2d7000
>>>> 7fff29b81000 28b1ac000
>>>> 7fff29b82000 28ecc1000
>>>> 7fff29b83000 28c5c2000
>>>> 7fff29b84000 28aaf4000
>>>> 7fff29b85000 28aaf9000
>>>> 7fff29b86000 289566000
>>>> crash>
>>>
>>> Seems like a reasonable request.
>>>
>>> However, I don't like your mixing it with the "-R reference" usage,
>>> because it confuses things like this simple example:
>>>
>>> crash> vm -p -s ffff810ec9f57818
>>> VMA START END FLAGS FILE
>>> ffff810ec9f57818 4010d000 40110000 101877 /usr/local/java/jdk1.5.0_19/bin/java
>>> VIRTUAL PHYSICAL
>>> 4010d000 ec89b1000
>>> 4010e000 FILE: /usr/local/java/jdk1.5.0_19/bin/java OFFSET: e000
>>> 4010f000 e99803000
>>> crash>
>>>
>>> If the command above were to be qualified with a "-R 4010e000" reference
>>> check, it results in a nonsensical error message:
>>>
>>> crash> vm -p -s ffff810ec9f57818 -R 4010e000
>>> vm: only one -R option allowed
>>> Usage:
>>> vm [-p | -v | -m | [-R reference] | [-f vm_flags]] [pid |
>>> taskp] ...
>>> Enter "help vm" for details.
>>> crash>
>>>
>>> A few suggestions:
>>>
>>> (1) Don't even bother to tie it in with the "vm -v" option, because if
>>> you already know the vm_area_struct address, then just print it with
>>> "vm_area_struct<address>".
>>>
>>> (2) Then, since it *only* applies with the -p functionality, make it
>>> a new "-P<vmaddr>" option, where the help page explains that the
>>> <vmaddr> argument must be a vm_area_struct of the current context:
>>>
>>> Usage:
>>> vm [-p | -P vmaddr | -v | -m | [-R reference] | [-f
>>> vm_flags]] [pid | taskp] ...
>>>
>>> (3) Make -P mutually exclusive with all of the other options.
>>>
>>> (4) Do not use the reference structure for this feature. Just use your new
>>> flag, and pass the vma address in the 3rd "vaddr" argument to vm_area_dump(),
>>> since it's not even being used by cmd_vm().
>>
>> I have modified the patch, please check.
>
> OK, these are my suggestions to finalize this patch. First, fix this:
>
> # make warn
> ...
> cc -c -g -DX86_64 -DGDB_7_3_1 memory.c -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
> memory.c: In function "vm_area_dump":
> memory.c:3503:7: warning: "single_vma" may be used uninitialized in this function [-Wuninitialized]
> ...
>
> Secondly, this command is no different from the other context-sensitive
> "vm" command options, so please always print the task header like they
> do. Just remove the three restrictions in cmd_vm() that do this:
>
> - if (!ref)
> + if (!(ref || (flag& PRINT_SINGLE_VMA)))
> print_task_header(fp, CURRENT_CONTEXT(), 0);
>
> Third, the option is either going to work OK or fail to find the VMA.
> Your patch does this if an invalid vm_area_struct address is entered:
>
> crash> vm -P ffff880617e7af44
> VMA START END FLAGS FILE
> crash>
>
> Regardless of success or failure, the print_task_header() call in cmd_vm()
> should always show the task information. Then, if you actually find the VMA,
> print the "VMA START ..." header along with its data:
>
> crash> vm -P ffff880284cf87e0
> PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
> VMA START END FLAGS FILE
> ffff880284cf87e0 400000 9fa000 8001875 /root/crash-6.0.4/crash
> VIRTUAL PHYSICAL
> 400000 28c7fa000
> 401000 29470a000
> 402000 29470b000
> 403000 29470c000
> 404000 29470d000
> 405000 29470e000
> 406000 29470f000
> ...
>
> But if the command fails to find the VMA, then indicate that under
> the task header:
>
> crash> vm -P ffff880284cf87e8
> PID: 13318 TASK: ffff8806294f2690 CPU: 11 COMMAND: "crash"
> (not found)
> crash>
>
> Lastly, please don't use subterfuge to accomplish the task at
> hand. The setting of PHYSADDR in the vm_area_dump() function
> makes no sense at all:
>
> + single_vma_header = 0;
> + if (flag& PRINT_SINGLE_VMA) {
> + flag |= PHYSADDR;
> + single_vma = vaddr;
> + vaddr = 0;
> + }
>
> It may "work" by doing the above, but I'm sure you can accomplish
> it just as easily using your PRINT_SINGLE_VMA flag and/or your new
> local variables, making it both easier to understand and more
> maintainable.
The patch is modified. I think I need to pay more attention to
maintainable and thanks for your patience.
>
> Thanks,
> Dave
>
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>
>
--
--
Regards
Qiao Nuohan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-vm-add-P-option.patch
Type: text/x-patch
Size: 6571 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20120315/fcf9ac9d/attachment.bin>
More information about the Crash-utility
mailing list