[Crash-utility] [PATCH] improve the performance of kmem -s [address]

[Dave Anderson]

----- Original Message -----
> Hello Dave,
> 
> These two patches are used to improve the performance of "kmem -s
> [address]".
> 
> Current code need to search all kmem_caches twice. It is a waste of
> time. With my patch, the address is sent to "dump_kmem_cache" function
> to make the second search be restricted to the kmem_cache found in the
> first search. It means the second search is neglected.
> 
> I have implemented the improvement for three types of kmem_cache. The
> search for "slub" does not waste that much time as its peers, so I did
> not modify the code related to "slub". Additionally, I only get the
> environment to test my first patch, so the second patch has not been
> tested.
> 
> Here is the statistic of my test on RHEL6.2 x86_64, the vmcore used for
> testing has about 150000 kmem_caches commands in file:
> 1. only 'quit'
> 
> origin code : about 36s
> with patch  : about 36s
> 
> 2. 1 time of searching the 100000th kmem_cache
> origin code : about 59s
> with patch  : about 39s
> 
> 3. 100 times of searching the 100000th kmem_cache
> origin code : about 38min30s
> with patch  : about 5min

Clarify this for me -- are you saying that your test kernel 
has 150000 kmem_cache slabs???

In other words, my Fedora 16 system has 103 slab caches:

  crash> kmem -s list | wc -l
  103
  crash>

So your kernel would show ~150000 individual slabs?
Would that ever be done in a real-life scenario?

Anyway, if I test your patch on my kernel doing "kmem -s kmalloc-96",
which is near the end of the list, and then do some time-testing, 
the difference is trivial.  If there were ~150000, then yes, I can
see that there would be much less list-traversal work being done.

That all being said, your patch does have merit -- presuming that
the meminfo.cache field *never* gets set elsewhere prior to entering
the dump_kmem_cache_xxx functions.  I haven't taken the time to
verify that -- did you absolutely verify that? 

> p.s.
> If I create a kmem_cache called "list", a little confusion may happen
> when using "kmem -s list". I am wondering is it necessary to introduce
> an another option replacing list to avoid such collision.

Then don't do that...  ;-)

But in other circumstances where there may be ambiguity, I've put
in the option of putting a "\" in front of the name, i.e., you
could do something like:

  crash> kmem -s list
  ffff880079e44a00 nfs_read_data
  ffff880079e44700 nfs_inode_cache
  ffff880079e44d00 fscache_cookie_jar
  ...

  crash> kmem -s \list
  CACHE            NAME                 OBJSIZE  ALLOCATED     TOTAL  SLABS  SSIZE
  ffff88007b895400 list                     240          0         0      0     4k
  crash>

That would be simple enough fix in cmd_kmem() if you re-work this part:

        if (sflag == 1) {
                if (STREQ(meminfo.reqname, "list"))
                        kmem_cache_list();
                else if (vt->flags & KMEM_CACHE_UNAVAIL)
                        error(FATAL,
                            "kmem cache slab subsystem not available\n");
                else
                        vt->dump_kmem_cache(&meminfo);
        }

Thanks,
  Dave