[Crash-utility] [PATCH 0/2] Move NAME column in "kmem -s" output to the last of line

[Kazuhito Hagio]

Hi Dave,


On 8/8/2018 2:24 PM, Dave Anderson wrote:
> 
> Kazu,
> 
> Nicely done!  It's a huge improvement in readability, cleans up some
> unnecessary duplication, and tests well on all of my old and new
> kernel dumpfiles that I have on hand.  Queued for crash-7.2.4:
> 
>   https://github.com/crash-utility/crash/commit/455da1ae5c7f22ba870aa57e071dad340749bdcd

Thank you for the test and merging!

but I just found the rest of work, -I option is not adjusted
to the patch.

  crash> kmem -s -I nfs_commit_data
  CACHE             OBJSIZE  ALLOCATED     TOTAL  SLABS  SSIZE  NAME
  ffff88017fc79800      352          0         0      0    16k  nfs_direct_cache
  ffff88017fc79700 nfs_commit_data    [IGNORED]
  ffff88017fc79600      888         32      2880     80    32k  nfs_read_data

How about the following?

  crash> kmem -s -I nfs_commit_data
  CACHE             OBJSIZE  ALLOCATED     TOTAL  SLABS  SSIZE  NAME
  ffff88017fc79800      352          0         0      0    16k  nfs_direct_cache
  ffff88017fc79700 [IGNORED]                                    nfs_commit_data
  ffff88017fc79600      888         32      2880     80    32k  nfs_read_data

---
 memory.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/memory.c b/memory.c
index 5bcf09e..4790cf6 100644
--- a/memory.c
+++ b/memory.c
@@ -9963,6 +9963,9 @@ ignore_cache(struct meminfo *si, char *name)
 #define KMEM_SLAB_OVERLOAD_PAGE (8)
 #define KMEM_SLAB_FREELIST      (9)
 
+#define DUMP_KMEM_CACHE_TAG(addr, name, tag) \
+	fprintf(fp, "%lx %-43s  %s\n", addr, tag, name)
+
 #define DUMP_KMEM_CACHE_INFO()  dump_kmem_cache_info(si)
 
 static void
@@ -10094,7 +10097,7 @@ dump_kmem_cache(struct meminfo *si)
 			goto next_cache;
 
 		if (ignore_cache(si, buf)) {
-			fprintf(fp, "%lx %-18s [IGNORED]\n", si->cache, buf);
+			DUMP_KMEM_CACHE_TAG(si->cache, buf, "[IGNORED]");
 			goto next_cache;
 		}
 
@@ -10303,7 +10306,7 @@ dump_kmem_cache_percpu_v1(struct meminfo *si)
 			goto next_cache;
 
                 if (ignore_cache(si, buf)) {
-                        fprintf(fp, "%lx %-18s [IGNORED]\n", si->cache, buf);
+                        DUMP_KMEM_CACHE_TAG(si->cache, buf, "[IGNORED]");
                         goto next_cache;
                 }
 
@@ -10547,12 +10550,12 @@ dump_kmem_cache_percpu_v2(struct meminfo *si)
 			goto next_cache;
 
                 if (ignore_cache(si, buf)) {
-                        fprintf(fp, "%lx %-18s [IGNORED]\n", si->cache, buf);
+                        DUMP_KMEM_CACHE_TAG(si->cache, buf, "[IGNORED]");
                         goto next_cache;
                 }
 
 		if (bad_slab_cache(si->cache)) {
-                        fprintf(fp, "%lx %-18s [INVALID/CORRUPTED]\n", si->cache, buf);
+                        DUMP_KMEM_CACHE_TAG(si->cache, buf, "[INVALID/CORRUPTED]");
                         goto next_cache;
 		}
 
@@ -18109,8 +18112,7 @@ dump_kmem_cache_slub(struct meminfo *si)
 			fprintf(fp, "%s", kmem_cache_hdr);
 		}
 		if (ignore_cache(si, buf)) {
-			fprintf(fp, "%lx %-18s [IGNORED]\n", 
-				si->cache_list[i], buf);
+			DUMP_KMEM_CACHE_TAG(si->cache_list[i], buf, "[IGNORED]");
 			goto next_cache;
 		}
 
-- 
1.8.3.1



> 
> Thanks,
>   Dave
> 
> 
> ----- Original Message -----
>> Nowadays, "kmem -s" output can have long lines due to cache name with
>> memcg name, and I don't think that it's human-readable as it is.
>>
>> crash> kmem -s
>> CACHE            NAME                 OBJSIZE  ALLOCATED     TOTAL  SLABS
>> SSIZE
>> ffff8a1522c15380 kmalloc-128(12536:session-11.scope) 128  111       128
>> 4     4k
>> ffff8a1522c15200 radix_tree_node(12536:session-11.scope) 576  340
>> 434     31     8k
>> ffff8a1523d15380 xfs_inode(12536:session-11.scope) 904  1567      1581     93
>> 16k
>> ffff8a1522c15080 nfs_inode_cache(12536:session-11.scope) 1072  140
>> 165     11    16k
>> ffff8a1523d14c00 shmem_inode_cache(12536:session-11.scope) 696  0
>> 46      2    16k
>>
>> So, can we move the 'NAME' column to the last of line like this?
>>
>> crash> kmem -s
>> CACHE             OBJSIZE  ALLOCATED     TOTAL  SLABS  SSIZE  NAME
>> ffff8a1522c15380      128        111       128      4     4k
>> kmalloc-128(12536:session-11.scope)
>> ffff8a1522c15200      576        340       434     31     8k
>> radix_tree_node(12536:session-11.scope)
>> ffff8a1523d15380      904       1567      1581     93    16k
>> xfs_inode(12536:session-11.scope)
>> ffff8a1522c15080     1072        140       165     11    16k
>> nfs_inode_cache(12536:session-11.scope)
>> ffff8a1523d14c00      696          0        46      2    16k
>> shmem_inode_cache(12536:session-11.scope)
>>
>> If we can, crash has the three functions to print them for each slab/slub
>> version and I think that it would be good to change all of them together.
>> Fortunately, the header is same among them, and it looks like we can unify
>> them into one function. [Patch 1]
>>
>> And then, move it to the last of line. [Patch 2]
>>
>> I tested this with some vmcores having PERCPU_KMALLOC_V2 or KMALLOC_SLUB
>> and found no problem, but I don't have any vmcores having PERCPU_KMALLOC_V1
>> or no flag.
>>
>> Kazuhito Hagio (2):
>>   Unify the three functions printing "kmem -s" line into one function
>>   Move NAME column in "kmem -s" output to the last of line
>>
>>  help.c   | 136
>>  +++++++++++++++++++++++++++++++--------------------------------
>>  memory.c | 132 +++++++++++++++----------------------------------------------
>>  2 files changed, 99 insertions(+), 169 deletions(-)
>>
>> --
>> 1.8.3.1
>>
>> --
>> Crash-utility mailing list
>> Crash-utility at redhat.com
>> https://www.redhat.com/mailman/listinfo/crash-utility
>>
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>