[Crash-utility] [PATCH] Fix for 'kmem -i' to display correct SLAB statistics

[lijiang]

Thank you for the comment, Kazu.

On Mon, Dec 26, 2022 at 1:29 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab at nec.com>
wrote:

> On 2022/12/23 19:42, Lianbo Jiang wrote:
> > Kernel commit d42f3245c7e2 ("mm: memcg: convert vmstat slab counters to
> > bytes"), which is contained in linux v5.9-rc1 and later kernels, renamed
> > NR_SLAB_{RECLAIMABLE,UNRECLAIMABLE} to
> NR_SLAB_{RECLAIMABLE,UNRECLAIMABLE}_B.
> >
> > Without the patch, "kmem -i" command will display incorrect SLAB
> > statistics:
> >
> >    crash> kmem -i | grep -e PAGES -e SLAB
> >                     PAGES        TOTAL      PERCENTAGE
> >             SLAB    89458     349.4 MB    0% of TOTAL MEM
> >                     ^^^^^     ^^^^^
> >
> > With the patch, the actual result is:
> >    crash> kmem -i | grep -e PAGES -e SLAB
> >                     PAGES        TOTAL      PERCENTAGE
> >             SLAB   261953    1023.3 MB    0% of TOTAL MEM
>
> Good catch!  This kind of no error issue is hard to detect..
>
> >
> > Reported-by: Buland Kumar Singh <bsingh at redhat.com>
> > Signed-off-by: Lianbo Jiang <lijiang at redhat.com>
> > ---
> >   memory.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/memory.c b/memory.c
> > index 9d003713534b..a8f08f1a4d09 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -8382,9 +8382,11 @@ dump_kmeminfo(void)
> >       if (vm_stat_init()) {
> >               if (dump_vm_stat("NR_SLAB", &nr_slab, 0))
> >                       get_slabs = nr_slab;
> > -             else if (dump_vm_stat("NR_SLAB_RECLAIMABLE", &nr_slab, 0))
> {
> > +             else if (dump_vm_stat("NR_SLAB_RECLAIMABLE", &nr_slab, 0)
> ||
> > +                             dump_vm_stat("NR_SLAB_RECLAIMABLE_B",
> &nr_slab, 0)) {
> >                       get_slabs = nr_slab;
> > -                     if (dump_vm_stat("NR_SLAB_UNRECLAIMABLE",
> &nr_slab, 0))
> > +                     if (dump_vm_stat("NR_SLAB_UNRECLAIMABLE",
> &nr_slab, 0) ||
> > +
>  dump_vm_stat("NR_SLAB_UNRECLAIMABLE_B", &nr_slab, 0))
> >                               get_slabs += nr_slab;
> >               }
> >       }
>
> Isn't this better?  If no NR_SLAB_RECLAIMABLE, there is no need to
> search for NR_SLAB_UNRECLAIMABLE.
>
>
Originally, I had the same idea as you.  But later, I noticed that there
was too much duplication of code. So, eventually I used the current fix.

But anyway, if you would prefer the following change, It's also good to me.

Thanks
Lianbo

--- a/memory.c
> +++ b/memory.c
> @@ -8457,6 +8457,11 @@ dump_kmeminfo(void)
>                          get_slabs = nr_slab;
>                          if (dump_vm_stat("NR_SLAB_UNRECLAIMABLE",
> &nr_slab, 0))
>                                  get_slabs += nr_slab;
> +               } else if (dump_vm_stat("NR_SLAB_RECLAIMABLE_B", &nr_slab,
> 0)) {
> +                       /* 5.9 and later */
> +                       get_slabs = nr_slab;
> +                       if (dump_vm_stat("NR_SLAB_UNRECLAIMABLE_B",
> &nr_slab, 0))
> +                               get_slabs += nr_slab;
>                  }
>          }
>
> Thanks,
> Kazu
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20221226/3124403f/attachment.htm>