Re: [RESEND v4 5/7] mm, slab_common: Make kmalloc_caches[] start at size KMALLOC_MIN_SIZE

From: David Rientjes
Date: Sun Sep 15 2019 - 17:38:42 EST


On Mon, 16 Sep 2019, Pengfei Li wrote:

> Currently, kmalloc_cache[] is not sorted by size, kmalloc_cache[0]
> is kmalloc-96, kmalloc_cache[1] is kmalloc-192 (when ARCH_DMA_MINALIGN
> is not defined).
>
> As suggested by Vlastimil Babka,
>
> "Since you're doing these cleanups, have you considered reordering
> kmalloc_info, size_index, kmalloc_index() etc so that sizes 96 and 192
> are ordered naturally between 64, 128 and 256? That should remove
> various special casing such as in create_kmalloc_caches(). I can't
> guarantee it will be possible without breaking e.g. constant folding
> optimizations etc., but seems to me it should be feasible. (There are
> definitely more places to change than those I listed.)"
>
> So this patch reordered kmalloc_info[], kmalloc_caches[], and modified
> kmalloc_index() and kmalloc_slab() accordingly.
>
> As a result, there is no subtle judgment about size in
> create_kmalloc_caches(). And initialize kmalloc_cache[] from 0 instead
> of KMALLOC_SHIFT_LOW.
>
> I used ./scripts/bloat-o-meter to measure the impact of this patch on
> performance. The results show that it brings some benefits.
>
> Considering the size change of kmalloc_info[], the size of the code is
> actually about 641 bytes less.
>

bloat-o-meter is reporting a net benefit of -241 bytes for this, so not
sure about relevancy of the difference for only kmalloc_info.

This, to me, looks like increased complexity for the statically allocated
arrays vs the runtime complexity when initializing the caches themselves.
Not sure that this is an improvement given that you still need to do
things like

+#if KMALLOC_SIZE_96_EXIST == 1
+ if (size > 64 && size <= 96) return (7 - KMALLOC_IDX_ADJ_0);
+#endif
+
+#if KMALLOC_SIZE_192_EXIST == 1
+ if (size > 128 && size <= 192) return (8 - KMALLOC_IDX_ADJ_1);
+#endif