[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YxlSpK4Jo97g1OB/@feng-clx>
Date: Thu, 8 Sep 2022 10:25:40 +0800
From: Feng Tang <feng.tang@...el.com>
To: Hyeonggon Yoo <42.hyeyoo@...il.com>
CC: Andrew Morton <akpm@...ux-foundation.org>,
Vlastimil Babka <vbabka@...e.cz>,
Christoph Lameter <cl@...ux.com>,
Pekka Enberg <penberg@...nel.org>,
David Rientjes <rientjes@...gle.com>,
Joonsoo Kim <iamjoonsoo.kim@....com>,
Roman Gushchin <roman.gushchin@...ux.dev>,
"Dmitry Vyukov" <dvyukov@...gle.com>,
Jonathan Corbet <corbet@....net>,
"Hansen, Dave" <dave.hansen@...el.com>,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"kasan-dev@...glegroups.com" <kasan-dev@...glegroups.com>,
Robin Murphy <robin.murphy@....com>,
John Garry <john.garry@...wei.com>,
Kefeng Wang <wangkefeng.wang@...wei.com>
Subject: Re: [PATCH v5 1/4] mm/slub: enable debugging memory wasting of
kmalloc
On Wed, Sep 07, 2022 at 10:17:22PM +0800, Hyeonggon Yoo wrote:
> On Wed, Sep 07, 2022 at 03:10:20PM +0800, Feng Tang wrote:
> > kmalloc's API family is critical for mm, with one nature that it will
> > round up the request size to a fixed one (mostly power of 2). Say
> > when user requests memory for '2^n + 1' bytes, actually 2^(n+1) bytes
> > could be allocated, so in worst case, there is around 50% memory
> > space waste.
> >
> > The wastage is not a big issue for requests that get allocated/freed
> > quickly, but may cause problems with objects that have longer life
> > time.
> >
> > We've met a kernel boot OOM panic (v5.10), and from the dumped slab
> > info:
> >
> > [ 26.062145] kmalloc-2k 814056KB 814056KB
> >
> > >From debug we found there are huge number of 'struct iova_magazine',
> > whose size is 1032 bytes (1024 + 8), so each allocation will waste
> > 1016 bytes. Though the issue was solved by giving the right (bigger)
> > size of RAM, it is still nice to optimize the size (either use a
> > kmalloc friendly size or create a dedicated slab for it).
> >
> > And from lkml archive, there was another crash kernel OOM case [1]
> > back in 2019, which seems to be related with the similar slab waste
> > situation, as the log is similar:
> >
> > [ 4.332648] iommu: Adding device 0000:20:02.0 to group 16
> > [ 4.338946] swapper/0 invoked oom-killer: gfp_mask=0x6040c0(GFP_KERNEL|__GFP_COMP), nodemask=(null), order=0, oom_score_adj=0
> > ...
> > [ 4.857565] kmalloc-2048 59164KB 59164KB
> >
> > The crash kernel only has 256M memory, and 59M is pretty big here.
> > (Note: the related code has been changed and optimised in recent
> > kernel [2], these logs are just picked to demo the problem, also
> > a patch changing its size to 1024 bytes has been merged)
> >
> > So add an way to track each kmalloc's memory waste info, and
> > leverage the existing SLUB debug framework (specifically
> > SLUB_STORE_USER) to show its call stack of original allocation,
> > so that user can evaluate the waste situation, identify some hot
> > spots and optimize accordingly, for a better utilization of memory.
> >
> > The waste info is integrated into existing interface:
> > '/sys/kernel/debug/slab/kmalloc-xx/alloc_traces', one example of
> > 'kmalloc-4k' after boot is:
> >
> > 126 ixgbe_alloc_q_vector+0xbe/0x830 [ixgbe] waste=233856/1856 age=280763/281414/282065 pid=1330 cpus=32 nodes=1
> > __kmem_cache_alloc_node+0x11f/0x4e0
> > __kmalloc_node+0x4e/0x140
> > ixgbe_alloc_q_vector+0xbe/0x830 [ixgbe]
> > ixgbe_init_interrupt_scheme+0x2ae/0xc90 [ixgbe]
> > ixgbe_probe+0x165f/0x1d20 [ixgbe]
> > local_pci_probe+0x78/0xc0
> > work_for_cpu_fn+0x26/0x40
> > ...
> >
> > which means in 'kmalloc-4k' slab, there are 126 requests of
> > 2240 bytes which got a 4KB space (wasting 1856 bytes each
> > and 233856 bytes in total), from ixgbe_alloc_q_vector().
> >
> > And when system starts some real workload like multiple docker
> > instances, there could are more severe waste.
> >
> > [1]. https://lkml.org/lkml/2019/8/12/266
> > [2]. https://lore.kernel.org/lkml/2920df89-9975-5785-f79b-257d3052dfaf@huawei.com/
> >
> > [Thanks Hyeonggon for pointing out several bugs about sorting/format]
> > [Thanks Vlastimil for suggesting way to reduce memory usage of
> > orig_size and keep it only for kmalloc objects]
> >
> > Signed-off-by: Feng Tang <feng.tang@...el.com>
> > Cc: Robin Murphy <robin.murphy@....com>
> > Cc: John Garry <john.garry@...wei.com>
> > Cc: Kefeng Wang <wangkefeng.wang@...wei.com>
> > ---
> > Documentation/mm/slub.rst | 33 +++++---
> > include/linux/slab.h | 2 +
> > mm/slub.c | 156 ++++++++++++++++++++++++++++----------
> > 3 files changed, 141 insertions(+), 50 deletions(-)
> >
>
> Looks good to me.
> Reviewed-by: Hyeonggon Yoo <42.hyeyoo@...il.com>
Thank you!
> > diff --git a/Documentation/mm/slub.rst b/Documentation/mm/slub.rst
>
> [...]
>
> > +/* Structure holding parameters for get_partial() call chain */
> > +struct partial_context {
> > + struct slab **slab;
> > + gfp_t flags;
> > + int orig_size;
>
> Nit: unsigned int orig_size
Yes, will change. 'unsigned int' is more consistent with the orig_size saved
in meta data and others members size/object_size/inuse/offset of kmem_cache.
Thanks,
Feng
> Thanks!
>
Powered by blists - more mailing lists