Re: [PATCH v2 3/6] mm/slab: Support disabling SLAB_BUCKETS at boot

[Chris Down <chris@chrisdown.name>]

Hi Kees,

Kees Cook writes:
>I thought the issue here was about calling convention (i.e. with
>CONFIG_SLAB_BUCKETS Vlastimil had suggested making the argument lists
>for the call chain shorter by dropping the bucket argument entirely).

I think you might be thinking of Harry's kmalloc_params idea from the
slab_alloc_context thread [1], which would fold the bucket and token
arguments into one struct. That's a separate thing. This patch can't
change the calling convention: with slab_buckets=0 the kmem_buckets
argument is still passed down, it's just NULL, so kmalloc_slab() picks
the normal caches.

What this patch is about is being able to build one image with
CONFIG_SLAB_BUCKETS=y and not pay for the separate caches on hosts which
don't want them. Harry measured that at about 1 MiB after boot [2], so
it's the smaller of the two, but it's the same "needs a separate kernel"
problem as kmalloc_partition=.

>Regardless, I still prefer this being a static branch so we don't end
>up with a needless rodata global check.

The flag is only read in kmem_buckets_create() and
create_kmalloc_caches(), when a set of buckets is created. It isn't read
on the allocation path. Allocations never look at it: they just get the
NULL that kmem_buckets_create() returned. So a static key wouldn't save
anything at runtime. Also, some architectures (arm, for example) parse
early params before jump_label_init(). So we'd still need the bool, and
would have to flip the key later as init_on_alloc does.

Is there somewhere else you had in mind for it?

[1] https://lore.kernel.org/r/e650e125-5f6c-4de7-88b5-9da666bb0a4e@kernel.org
[2] https://lore.kernel.org/r/179312451613.474906.1475329637924619088@kernel.org

Thanks!

Chris