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

2018-08-09 Thread Dave Anderson



- Original Message -
> 
> 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.

Hi Kazu,

Looks good -- and I also fixed the "help kmem" description of the -I option.
Queued for crash-7.2.4:

  
https://github.com/crash-utility/crash/commit/e9532aea6818b347bc0740f39efd4ec844cfb9b0
   
Thanks again,
  Dave

> 
>   crash> kmem -s -I nfs_commit_data
>   CACHE OBJSIZE  ALLOCATED TOTAL  SLABS  SSIZE  NAME
>   88017fc79800  352  0 0  016k
>   nfs_direct_cache
>   88017fc79700 nfs_commit_data[IGNORED]
>   88017fc79600  888 32  2880 8032k  nfs_read_data
> 
> How about the following?
> 
>   crash> kmem -s -I nfs_commit_data
>   CACHE OBJSIZE  ALLOCATED TOTAL  SLABS  SSIZE  NAME
>   88017fc79800  352  0 0  016k
>   nfs_direct_cache
>   88017fc79700 [IGNORED]
>   nfs_commit_data
>   88017fc79600  888 32  2880 8032k  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
> >> CACHENAME OBJSIZE  ALLOCATED TOTAL  SLABS
> >> SSIZE
> >> 8a1522c15380 kmalloc-128(12536:session-11.scope) 128  111   128
> >> 4 4k
> >> 8a1522c15200 radix_tree_node(12536:session-11.scope) 576  340
> >> 434 31 8k
> >> 8a1523d15380 xfs_inode(12536:session-11.scope) 904  1567  1581
> >> 93
> >> 16k
> >> 8a1522c15080 nfs_inode_cache(12536:session-11.scope) 1072  140
> >> 165 1116k
> >> 8a1523d14c00 shmem_inode_cache(12536:session-11.scope) 696  0
> >> 46  216k
> >>
> >> So, can we move the 'NAME' column to the last of line like this?
> >>
> >> crash> kmem -s
> >> CACHE OBJSIZE  ALLOCATED TOTAL  SLABS  SSIZE  NAME
> >> 8a1522c15380  128111   128  4 4k
> >> 

Re: [Crash-utility] [PATCH] fix open fds display when process using large amount of file descriptors

2018-08-09 Thread Dave Anderson



- Original Message -
> Usually, structure fd_set only has 1024 bits size, when a process
> using large amount of file descriptors that exceed the size of fd_set,
> then the display of files and net sockets would mistake caused by the
> out of bounds reading in loop.
> 
> Signed-off-by: Tan Hu 

Hello Tan,

Sorry for the delay -- the patch looks good, and my testing verified the issue. 
 
Queued for crash-7.2.4:


https://github.com/crash-utility/crash/commit/ba03b66cec24fc0033d4378be08f5f9a96cd4a91

Thanks,
  Dave



> ---
>  filesys.c | 24 ++--
>  net.c | 17 +
>  2 files changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/filesys.c b/filesys.c
> index 0ace8f4..47f5a24 100755
> --- a/filesys.c
> +++ b/filesys.c
> @@ -2380,7 +2380,8 @@ open_files_dump(ulong task, int flags, struct reference
> *ref)
>   int max_fdset = 0;
>   int max_fds = 0;
>   ulong open_fds_addr;
> - fd_set open_fds;
> + int open_fds_size;
> + ulong *open_fds;
>   ulong fd;
>   ulong file;
>   ulong value;
> @@ -2583,16 +2584,25 @@ open_files_dump(ulong task, int flags, struct
> reference *ref)
>   open_fds_addr = ULONG(files_struct_buf +
>   OFFSET(files_struct_open_fds));
>  
> + open_fds_size = MAX(max_fdset, max_fds) / BITS_PER_BYTE;
> + open_fds = (ulong *)GETBUF(open_fds_size);
> + if (!open_fds) {
> + if (fdtable_buf)
> + FREEBUF(fdtable_buf);
> + FREEBUF(files_struct_buf);
> + return;
> + }
> +
>   if (open_fds_addr) {
>   if (VALID_MEMBER(files_struct_open_fds_init) &&
>   (open_fds_addr == (files_struct_addr +
>   OFFSET(files_struct_open_fds_init
>   BCOPY(files_struct_buf +
>   OFFSET(files_struct_open_fds_init),
> - _fds, sizeof(fd_set));
> + open_fds, open_fds_size);
>   else
> - readmem(open_fds_addr, KVADDR, _fds,
> - sizeof(fd_set), "fdtable open_fds",
> + readmem(open_fds_addr, KVADDR, open_fds,
> + open_fds_size, "fdtable open_fds",
>   FAULT_ON_ERROR);
>   }
>  
> @@ -2607,6 +2617,7 @@ open_files_dump(ulong task, int flags, struct reference
> *ref)
>   if (fdtable_buf)
>   FREEBUF(fdtable_buf);
>   FREEBUF(files_struct_buf);
> + FREEBUF(open_fds);
>   return;
>   }
>  
> @@ -2617,11 +2628,11 @@ open_files_dump(ulong task, int flags, struct
> reference *ref)
>   j = 0;
>   for (;;) {
>   unsigned long set;
> - i = j * __NFDBITS;
> + i = j * BITS_PER_LONG;
>   if (((max_fdset >= 0) && (i >= max_fdset)) ||
>   (i >= max_fds))
>break;
> - set = open_fds.__fds_bits[j++];
> + set = open_fds[j++];
>   while (set) {
>   if (set & 1) {
>   readmem(fd + i*sizeof(struct file *), KVADDR,
> @@ -2665,6 +2676,7 @@ open_files_dump(ulong task, int flags, struct reference
> *ref)
>   if (fdtable_buf)
>   FREEBUF(fdtable_buf);
>   FREEBUF(files_struct_buf);
> + FREEBUF(open_fds);
>  }
>  
>  /*
> diff --git a/net.c b/net.c
> index 4199091..f08f22a 100755
> --- a/net.c
> +++ b/net.c
> @@ -1373,7 +1373,8 @@ dump_sockets_workhorse(ulong task, ulong flag, struct
> reference *ref)
>   int max_fdset = 0;
>   int max_fds = 0;
>   ulong open_fds_addr = 0;
> - fd_set open_fds;
> + ulong *open_fds;
> + int open_fds_size;
>   ulong fd;
>   ulong file;
>   int i, j;
> @@ -1446,12 +1447,18 @@ dump_sockets_workhorse(ulong task, ulong flag, struct
> reference *ref)
>   sizeof(void *), "files_struct fd addr", FAULT_ON_ERROR);
>   }
>  
> + open_fds_size = MAX(max_fdset, max_fds) / BITS_PER_BYTE;
> + open_fds = (ulong *)GETBUF(open_fds_size);
> + if (!open_fds)
> + return;
> +
>   if (open_fds_addr)
> - readmem(open_fds_addr, KVADDR, _fds, sizeof(fd_set),
> + readmem(open_fds_addr, KVADDR, open_fds, open_fds_size,
>   "files_struct open_fds", FAULT_ON_ERROR);
>   if (!open_fds_addr || !fd) {
>   if (!NET_REFERENCE_CHECK(ref))
>   fprintf(fp, "No open sockets.\n");
> + FREEBUF(open_fds);
>   return;
>   }
>  
> @@ -1479,10 +1486,10 @@ dump_sockets_workhorse(ulong task, ulong flag, struct
> reference *ref)
>   j = 0;
>   for (;;) {
>   unsigned long set;
> - i = j * __NFDBITS;
> + i = j * BITS_PER_LONG;
>   if (((max_fdset >= 0) && (i >=