[PATCH] mm/slub: create kmalloc 96 and 192 caches regardless cache size order

[PATCH] mm/slub: create kmalloc 96 and 192 caches regardless cache size order

[Hyunmin Lee]

For SLAB the kmalloc caches needed to be created in ascending sizes in
order. However, the constraint is not necessary anymore because SLAB has
been deprecated and SLUB doesn't need to comply with the constraint. Thus,
kmalloc 96 and 192 caches can be created after the other size kmalloc
caches are created instead of checking every time to find their order to
be created. Also, this change could prevent engineers from being confused
by the deprecated constraint.

Signed-off-by: Hyunmin Lee <hyunminlr@gmail.com>
Co-developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>
Co-developed-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 mm/slab_common.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 3179a6aeffc5..c3271b17eb76 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -919,19 +919,12 @@ void __init create_kmalloc_caches(void)
 		for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
 			if (!kmalloc_caches[type][i])
 				new_kmalloc_cache(i, type);
-
-			/*
-			 * Caches that are not of the two-to-the-power-of size.
-			 * These have to be created immediately after the
-			 * earlier power of two caches
-			 */
-			if (KMALLOC_MIN_SIZE <= 32 && i == 6 &&
-					!kmalloc_caches[type][1])
-				new_kmalloc_cache(1, type);
-			if (KMALLOC_MIN_SIZE <= 64 && i == 7 &&
-					!kmalloc_caches[type][2])
-				new_kmalloc_cache(2, type);
 		}
+
+		if (KMALLOC_MIN_SIZE <= 32 && !kmalloc_caches[type][1])
+			new_kmalloc_cache(1, type);
+		if (KMALLOC_MIN_SIZE <= 64 && !kmalloc_caches[type][2])
+			new_kmalloc_cache(2, type);
 	}
 #ifdef CONFIG_RANDOM_KMALLOC_CACHES
 	random_kmalloc_seed = get_random_u64();
-- 
2.34.1

Re: [PATCH] mm/slub: create kmalloc 96 and 192 caches regardless cache size order

[David Rientjes]

On Sat, 20 Apr 2024, Hyunmin Lee wrote:

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 3179a6aeffc5..c3271b17eb76 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -919,19 +919,12 @@ void __init create_kmalloc_caches(void)
>  		for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
>  			if (!kmalloc_caches[type][i])
>  				new_kmalloc_cache(i, type);
> -
> -			/*
> -			 * Caches that are not of the two-to-the-power-of size.
> -			 * These have to be created immediately after the
> -			 * earlier power of two caches
> -			 */
> -			if (KMALLOC_MIN_SIZE <= 32 && i == 6 &&
> -					!kmalloc_caches[type][1])
> -				new_kmalloc_cache(1, type);
> -			if (KMALLOC_MIN_SIZE <= 64 && i == 7 &&
> -					!kmalloc_caches[type][2])
> -				new_kmalloc_cache(2, type);
>  		}
> +
> +		if (KMALLOC_MIN_SIZE <= 32 && !kmalloc_caches[type][1])
> +			new_kmalloc_cache(1, type);
> +		if (KMALLOC_MIN_SIZE <= 64 && !kmalloc_caches[type][2])
> +			new_kmalloc_cache(2, type);
>  	}
>  #ifdef CONFIG_RANDOM_KMALLOC_CACHES
>  	random_kmalloc_seed = get_random_u64();

No objection to this, but it would be nice to continue to have a comment 
about why these slab caches are special.

Another thought: can we move the check for NULL kmalloc_caches to 
new_kmalloc_cache() instead of handling it in the caller?

Re: [PATCH] mm/slub: create kmalloc 96 and 192 caches regardless cache size order

[Christoph Lameter (Ampere)]

On Sat, 20 Apr 2024, Hyunmin Lee wrote:

> For SLAB the kmalloc caches needed to be created in ascending sizes in
> order. However, the constraint is not necessary anymore because SLAB has
> been deprecated and SLUB doesn't need to comply with the constraint. Thus,
> kmalloc 96 and 192 caches can be created after the other size kmalloc
> caches are created instead of checking every time to find their order to
> be created. Also, this change could prevent engineers from being confused
> by the deprecated constraint.


Ok but you are creating index 1 2 after going through KMALLOC_SHIFT_LOW 
to KMALLOC_SHIFT_HIGH. 1 and 2 should come before the loop in 
order to make it easily readable.

At that point you can also skip the check for the kmalloc caches already 
being present I think.