Re: [PATCH] Fix for 'kmem -i' to display correct SLAB statistics

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thank you for the comment, Kazu.

On Mon, Dec 26, 2022 at 1:29 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
> ---
>   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

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux