----- 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
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
