----- Original Message -----
> Re-posting the patch as an attachment. I'm sorry for the mess on the
> first pass.
>
> --
> David.
Hi Dave,
Your patch has merit, but I'm going to pare it down a bit. I use CRASHDEBUG(8)
fairly frequently, but with your changes as-is, it's far too verbose. It's also
a bit redundant -- here's an example of a CRASHDEBUG(8) kdump read currently:
crash> rd ffff88003c1a5cc0
<addr: ffff88003c1a5cc0 count: 1 flag: 490 (KVADDR)>
<readmem: ffff88003c1a5cc0, KVADDR, "64-bit KVADDR", 8, (FOE), 7fff0096b5a8>
addr: ffff88003c1a5cc0 paddr: 3c1a5cc0 cnt: 8
ffff88003c1a5cc0: 0000000000000000 ........
text hit rate: 65% (3505 of 5378)
crash>
And then with your patch:
crash> rd ffff88003c1a5cc0
<addr: ffff88003c1a5cc0 count: 1 flag: 490 (KVADDR)>
<readmem: ffff88003c1a5cc0, KVADDR, "64-bit KVADDR", 8, (FOE), 7fff1c97b6e8>
addr: ffff88003c1a5cc0 paddr: 3c1a5cc0 cnt: 8
rd: read_kdump for: virt=ffff88003c1a5cc0 phys=3c1a5cc0
rd: read_kdump trying read_netdump for: virt=ffff88003c1a5cc0 phys=3c1a5cc0
rd: read_netdump(-1, 7fff1c97b6e8, 8, ffff88003c1a5cc0, 3c1a5cc0)
rd: read KDUMP or NETDUMP_ELF64 for multi-segment: 3c195fb8
rd: seek and read offset: 3c195fb8
ffff88003c1a5cc0: 0000000000000000 ........
text hit rate: 65% (3505 of 5378)
crash>
The "<readmem: ...>" and subsequent "addr: ..." lines come from readmem().
I don't understand the need for the next 3 lines, as they are completely
redundant:
rd: read_kdump for: virt=ffff88003c1a5cc0 phys=3c1a5cc0
rd: read_kdump trying read_netdump for: virt=ffff88003c1a5cc0 phys=3c1a5cc0
rd: read_netdump(-1, 7fff1c97b6e8, 8, ffff88003c1a5cc0, 3c1a5cc0)
I don't argue with the displays of the resultant file offsets, but I may move
them to CRASHDEBUG(9):
rd: read KDUMP or NETDUMP_ELF64 for multi-segment: 3c195fb8
rd: seek and read offset: 3c195fb8
And the same goes for compressed kdumps:
crash> rd ffff81005286e0c0
<addr: ffff81005286e0c0 count: 1 flag: 490 (KVADDR)>
<readmem: ffff81005286e0c0, KVADDR, "64-bit KVADDR", 8, (FOE), 7fff11d16958>
addr: ffff81005286e0c0 paddr: 5286e0c0 cnt: 8
ffff81005286e0c0: 0000000000000000 ........
text hit rate: 66% (239 of 357)
crash>
With the patch, aside from the "pfn" displays, it's redundant information:
crash> rd ffff81005286e0c0
<addr: ffff81005286e0c0 count: 1 flag: 490 (KVADDR)>
<readmem: ffff81005286e0c0, KVADDR, "64-bit KVADDR", 8, (FOE), 7fffd3ca4478>
addr: ffff81005286e0c0 paddr: 5286e0c0 cnt: 8
rd: read_diskdump(-1, 7fffd3ca4478, 8, ffff81005286e0c0, 5286e0c0), pfn=5286e
rd: Buffering page with pfn=5286e and current physaddr=5286e000
ffff81005286e0c0: 0000000000000000 ........
text hit rate: 66% (239 of 357)
crash>
For starters, I think the patch can be simplified by modifying the "addr: ..."
line that is displayed by readmem() just prior to issuing it to the configured
pc->readmem function so that it shows the receiving function. So that line,
which may be displayed several times if the read crosses a page boundary, could
be reworked to show something like this:
crash> rd ffff81005286e0c0
<addr: ffff81005286e0c0 count: 1 flag: 490 (KVADDR)>
<readmem: ffff81005286e0c0, KVADDR, "64-bit KVADDR", 8, (FOE), 7fffd3ca4478>
<read_netdump: addr: ffff81005286e0c0 paddr: 5286e0c0 cnt: 8
...
and then a line or two from the relevant read functions would show
the file offset that was resolved from the physical address.
Also, the new function that translates the pc->readmem function address to a
string for display could also be used in a single location in main.c instead of
having to put debug statements in each location where where pc->readmem is
initialized.
So let me play around with this a bit...
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
