[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YxC7Kda5g/aEqarm@hyeyoo>
Date: Thu, 1 Sep 2022 23:01:13 +0900
From: Hyeonggon Yoo <42.hyeyoo@...il.com>
To: Feng Tang <feng.tang@...el.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>,
Dave Hansen <dave.hansen@...el.com>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, Robin Murphy <robin.murphy@....com>,
John Garry <john.garry@...wei.com>,
Kefeng Wang <wangkefeng.wang@...wei.com>
Subject: Re: [PATCH v4 1/4] mm/slub: enable debugging memory wasting of
kmalloc
On Mon, Aug 29, 2022 at 03:56:15PM +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+0xa5/0x4a0 [ixgbe] waste=233856/1856 age=1493302/1493830/1494358 pid=1284 cpus=32 nodes=1
> __slab_alloc.isra.86+0x52/0x80
> __kmalloc_node+0x143/0x350
> ixgbe_alloc_q_vector+0xa5/0x4a0 [ixgbe]
> ixgbe_init_interrupt_scheme+0x1a6/0x730 [ixgbe]
> ixgbe_probe+0xc8e/0x10d0 [ixgbe]
> local_pci_probe+0x42/0x80
> work_for_cpu_fn+0x13/0x20
> process_one_work+0x1c5/0x390
>
> 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). And when system starts some real
> workload like multiple docker instances, there 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>
> ---
> include/linux/slab.h | 2 +
> mm/slub.c | 94 +++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 81 insertions(+), 15 deletions(-)
Would you update Documentation/mm/slub.rst as well?
(alloc_traces part)
[...]
> */
> static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
> - unsigned long addr, struct kmem_cache_cpu *c)
> + unsigned long addr, struct kmem_cache_cpu *c, unsigned int orig_size)
> {
> void *freelist;
> struct slab *slab;
> @@ -3115,6 +3158,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>
> if (s->flags & SLAB_STORE_USER)
> set_track(s, freelist, TRACK_ALLOC, addr);
> + set_orig_size(s, freelist, orig_size);
>
> return freelist;
> }
> @@ -3140,6 +3184,8 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
> */
> if (s->flags & SLAB_STORE_USER)
> set_track(s, freelist, TRACK_ALLOC, addr);
> + set_orig_size(s, freelist, orig_size);
> +
> return freelist;
> }
This patch is okay but with patch 4, init_object() initializes redzone/poison area
using s->object_size, and init_kmalloc_object() fixes redzone/poison area using orig_size.
Why not do it in init_object() in the first time?
Also, updating redzone/poison area after alloc_single_from_new_slab()
(outside list_lock, after adding slab to list) will introduce races with validation.
So I think doing set_orig_size()/init_kmalloc_object() in alloc_debug_processing() would make more sense.
I can miss something, please kindly let me know if I did ;)
Anything else looks good to me.
Thanks!
--
Thanks,
Hyeonggon
Powered by blists - more mailing lists