linux-kernel - Re: [RESEND v4 5/7] mm, slab_common: Make kmalloc_caches[] start at size KMALLOC_MIN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.1909151425490.211705@chino.kir.corp.google.com>
Date:   Sun, 15 Sep 2019 14:38:35 -0700 (PDT)
From:   David Rientjes <rientjes@...gle.com>
To:     Pengfei Li <lpf.vector@...il.com>
cc:     akpm@...ux-foundation.org, vbabka@...e.cz, cl@...ux.com,
        penberg@...nel.org, iamjoonsoo.kim@....com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, guro@...com
Subject: Re: [RESEND v4 5/7] mm, slab_common: Make kmalloc_caches[] start at
 size KMALLOC_MIN_SIZE

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