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

Dave Anderson anderson at redhat.com
Fri Feb 3 16:56:43 UTC 2012



----- Original Message -----
> At 2012-2-3 2:53, Dave Anderson wrote:
> >
> >
> > ----- Original Message -----
> >>
> >> 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?
> >
> > It looks OK to use "if (si->cache)" in those 3 locations -- but it
> > would be safer to create/set a unique meminfo->flags bit for this
> > feature.
> Thanks for your comments.
> 
> A new flag called "CACHE_SET" is created to inform the
> dump_kmem_cache_xxx functions that "si->cache" has been set.
> 
> As you said in the previous mail, I agree that ~150000 individual slabs
> rarely exist in a real-life scenario. However, ~150000 individual slabs
> are used to illustrate the improvement clearly. Think about the
> situation that I install a big module with some kmem_caches. I need to
> check the state of each kmem_cache related to my module once some
> operations are done. So the times of execution of "kmem -s <addr>" may
> be even big. Time, obviouly, will be saved with my patch in such
> situation.

I suppose, yes -- if your "big module" creates thousands of kmem_cache
slabs?  But that's still a highly-unlikely scenario...

Nonetheless, I like your idea of replacing the readmem() of the full
kmem_cache buffer with the readmem() of just the next pointer in the
is_kmem_cache_addr() function.  In older kernels, the kmem_cache.array[]
was much smaller, and the readmem() was less than a page size, but now
the structure can be several pages in length.  However, you did forget
to delete the "cache_buf" and its GETBUF() and FREEBUF() calls.  So I
removed those references, and queued your patch for crash-6.0.3.

Thanks,
  Dave




More information about the Crash-utility mailing list