Re: [PATCH v3] mm, slub: change run-time assertion in kmalloc_index() to compile-time

Hyeonggon Yoo
2021-05-13T12:03:59Z

On Thu, May 13, 2021 at 12:31:38PM +0200, Marco Elver wrote:

> ------ >8 ------
>
> From: Marco Elver <elver@google.com>
> Subject: [PATCH] kfence: test: fix for "mm, slub: change run-time assertion in
> kmalloc_index() to compile-time"
>
> Signed-off-by: Marco Elver <elver@google.com>
> ---
> include/linux/slab.h | 9 +++++++--
> mm/kfence/kfence_test.c | 5 +++--
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 27d142564557..7a10bdc4b7a9 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -350,7 +350,8 @@ static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags)
> * Note: there's no need to optimize kmalloc_index because it's evaluated
> * in compile-time.
> */
> -static __always_inline unsigned int kmalloc_index(size_t size)
> +static __always_inline unsigned int __kmalloc_index(size_t size,
> + bool size_is_constant)
> {
> if (!size)
> return 0;
> @@ -386,11 +387,15 @@ static __always_inline unsigned int kmalloc_index(size_t size)
> if (size <= 16 * 1024 * 1024) return 24;
> if (size <= 32 * 1024 * 1024) return 25;
>
> - BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
> + if (size_is_constant)
> + BUILD_BUG_ON_MSG(1, "unexpected size in kmalloc_index()");
> + else
> + BUG();

what about checking size it on top of kmalloc_index? because by definition of
KMALLOC_SHIFT_HIGH, it's not always 25. it can be less than 25. for some
situations.

below is what I suggested beofre. for just reference:

--- include/linux/slab.h.orig 2021-05-12 17:56:54.504738768 +0900
+++ include/linux/slab.h 2021-05-13 15:06:25.724565850 +0900
@@ -346,9 +346,18 @@ static __always_inline enum kmalloc_cach
* 1 = 65 .. 96 bytes
* 2 = 129 .. 192 bytes
* n = 2^(n-1)+1 .. 2^n
+ *
+ * Note: there's no need to optimize kmalloc_index because it's evaluated
+ * in compile-time.
*/
static __always_inline unsigned int kmalloc_index(size_t size)
{
+ if (__builtin_constant_p(size)) {
+ BUILD_BUG_ON_MSG(size > KMALLOC_MAX_CACHE_SIZE , "unexpected size in kmalloc_index()");
+ } else if (size > KMALLOC_MAX_CACHE_SIZE) {
+ BUG();
+ }
+
if (!size)
return 0;

@@ -382,8 +391,6 @@ static __always_inline unsigned int kmal
if (size <= 8 * 1024 * 1024) return 23;
if (size <= 16 * 1024 * 1024) return 24;
if (size <= 32 * 1024 * 1024) return 25;
- if (size <= 64 * 1024 * 1024) return 26;
- BUG();

/* Will never be reached. Needed because the compiler may complain */
return -1;