[Crash-utility] RFC: string search for crash

[Dave Anderson]

Hi Bob,

So 5.1.2 is finally available for you to merge in your new string 
search capability...

As it turns out, the changes made for fixing and correctly implementing 
the search command has made things look quite a bit different.  As I
mentioned before, there are now two search functions, search_virtual()
and search_physical().  The major change was to the next_kpage() function,
where I wanted to avoid hacking in a bunch of "if (machine_type(XXX))" 
sections.  So taking your original suggestion, I created a helper
machine-dependent function called via machdep->get_kvaddr_ranges().
It returns an array of kernel virtual address ranges and their type, 
sorted by their starting kernel virtual address.  At the bottom of 
cmd_search(), search_virtual() gets called repeatedly for each type 
of kernel virtual addresses if it's appropriate for the user's request.
There are 5 possible kernel virtual range types:

  KVADDR_UNITY_MAP
  KVADDR_VMALLOC
  KVADDR_VMEMMAP    (possible on x86_64, ia64, ppc64 and s390x)
  KVADDR_START_MAP  (ia64 and x86_64 only)
  KVADDR_MODULES    (x86_64 only if modules are not in vmalloc() vmlist)

x86_64, ia64, ppc64 and s390c implement their own machine-dependent
xxxx_get_kvaddr_ranges() function; all of the other architectures
use generic_get_kvaddr_ranges(), which returns KVADDR_UNITY_MAP and
KVADDR_VMALLOC ranges.

If you "set debug 1" before the search, you can see what the
range values and types are, and then each call to search_virtual()
displays the start/end address, does the search, and then 
indicates how many pages were checked, and of those, how many
were actually read -- like here from a 2.6.37 dumpfile:
  
  crash> set debug 1
  debug: 1
  crash> search -k deadbeef
  kvaddr ranges:
    [0] ffff880000000000 ffff880040000000 KVADDR_UNITY_MAP
    [1] ffffc90000000000 ffffffffa051c000 KVADDR_VMALLOC
    [2] ffffea0000000000 ffffea0000e00000 KVADDR_VMEMMAP
    [3] ffffffff80000000 ffffffff8202f000 KVADDR_START_MAP
  search_virtual: start: ffff880000000000 end: ffff880040000000
  ffff880008703438: deadbeef
  ffff880016ffd730: deadbeef
  ffff88002cf9e580: deadbeef
  ffff8800320f9170: deadbeef
  ffff8800327dc580: deadbeef
  ffff880032ded170: deadbeef
  ffff880033191170: deadbeef
  ffff88003f504580: deadbeef
  search_virtual: read 262128 (99%) of 262144 pages checked in 36 seconds
  search_virtual: start: ffffc90000000000 end: ffffffffa051c000
  search_virtual: read 3995 (5%) of 70924 pages checked in 2 seconds
  search_virtual: start: ffffea0000000000 end: ffffea0000e00000
  search_virtual: read 3584 (100%) of 3584 pages checked in 0 seconds
  search_virtual: start: ffffffff80000000 end: ffffffff8202f000
  search_virtual: read 8223 (99%) of 8239 pages checked in 3 seconds
  crash>
  
I did implement the -K and -V flags, but unlike your original problem,
the vmalloc range search is several orders of magnitude shorter
in time consumed.  The problem was that next_vmlist_vaddr() was
repeatedly calling dump_vmlist() to get the vm_struct list; I
changed it to call it only once for a dumpfile, and once-per-command
invocation on a live system.  That being the case, the -V flag
is far more useful than -K, because it will not check unity-mapped 
memory -- say for example, if you want to check the virtual mem_map 
range for pages pointing to a particular address_space mapping.

In any case, getting back to your string search option.  I'd
prefer it if you could continue to make the function separate,
and because there are now search_physical() and search_virtual()
functions, your string-search function would have to be called 
from the bottom of their respective page-cycling loops.  In other
words, please don't try to merge the string-search code with 
the existing value-search code.  I'm pretty sure you wouldn't
want to anyway.

Then, getting back to the original discussion as to how to handle
strings that cross page boundaries.  I think that now I am of the
opinion that you shouldn't do anything special for physical or
unity-mapped memory.  I understand that they don't necessarily
(actually probably will not) be contiguous in actual use -- but 
they very well might be.  Consider huge-pages, or page allocations
that are order-1 or larger.  And even if they are not contiguous,
what's the harm of displaying a cross-page string match if you
find one?  I would think that would be something of interest
rather than something to avoid. 

That being the case, you wouldn't have to do any kind of special
handling for the various page types.  If a string crosses a page
boundary -- then show it dammit!  ;-)

It would seem that if even the very last character of a page
matches the beginning of a string, you could save that information,
(or the whole page), and upon checking the next *contiguous* page,
you could initially check for a cross-page string.  Of course the 
"multiple-search-argument" capability makes it a little bit trickier,
but it still seems doable.

And as far as the round-down to a word boundary issue, now I'm not 
convinced that that is really necessary.  You had mentioned concerns 
about using "rd", but you can pass it any address and it will start
displaying at that address:
  
  crash> rd ffffffff80274020 5
  ffffffff80274020:  65762078756e694c 2e32206e6f697372   Linux version 2.
  ffffffff80274030:  3832312d38312e36 6f6d2820356c652e   6.18-128.el5 (mo
  ffffffff80274040:  40646c6975626b63                    ckbuild@
  crash> rd ffffffff80274021 5
  ffffffff80274021:  7265762078756e69 362e32206e6f6973   inux version 2.6
  ffffffff80274031:  2e3832312d38312e 636f6d2820356c65   .18-128.el5 (moc
  ffffffff80274041:  6840646c6975626b                    kbuild at h
  crash>

You probably had some other concern?

Anyway, have at it.  And thanks again very much for persuing this to begin
with, as it really was time to overhaul the search command.  The days
of a simple unity-mapped region followed by a small vmalloc range are
over.

I look forward to your next patch...

Thanks,
  Dave