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