[PATCH 0/5] mm, slab: Make kmalloc_info[] contain all types of names

[PATCH 0/5] mm, slab: Make kmalloc_info[] contain all types of names

[Pengfei Li]

There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
and KMALLOC_DMA.

The name of KMALLOC_NORMAL is contained in kmalloc_info[].name,
but the names of KMALLOC_RECLAIM and KMALLOC_DMA are dynamically
generated by kmalloc_cache_name().

Patch1 predefines the names of all types of kmalloc to save
the time spent dynamically generating names.

The other 4 patches did some cleanup work.

These changes make sense, and the time spent by new_kmalloc_cache()
has been reduced by approximately 36.3%.

                         Time spent by
                         new_kmalloc_cache()
5.3-rc7                       66264
5.3-rc7+patch                 42188

Pengfei Li (5):
  mm, slab: Make kmalloc_info[] contain all types of names
  mm, slab_common: Remove unused kmalloc_cache_name()
  mm, slab: Remove unused kmalloc_size()
  mm, slab_common: Make 'type' is enum kmalloc_cache_type
  mm, slab_common: Make initializing KMALLOC_DMA start from 1

 include/linux/slab.h |  20 ---------
 mm/slab.c            |   7 +--
 mm/slab.h            |   2 +-
 mm/slab_common.c     | 101 +++++++++++++++++++++++--------------------
 4 files changed, 59 insertions(+), 71 deletions(-)

-- 
2.21.0

[PATCH 1/5] mm, slab: Make kmalloc_info[] contain all types of names

[Pengfei Li]

There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
and KMALLOC_DMA.

The name of KMALLOC_NORMAL is contained in kmalloc_info[].name,
but the names of KMALLOC_RECLAIM and KMALLOC_DMA are dynamically
generated by kmalloc_cache_name().

This patch predefines the names of all types of kmalloc to save
the time spent dynamically generating names.

Signed-off-by: Pengfei Li <lpf.vector@gmail.com>
---
 mm/slab.c        |  2 +-
 mm/slab.h        |  2 +-
 mm/slab_common.c | 76 +++++++++++++++++++++++++++++++-----------------
 3 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/mm/slab.c b/mm/slab.c
index 9df370558e5d..c42b6211f42e 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1247,7 +1247,7 @@ void __init kmem_cache_init(void)
 	 * structures first.  Without this, further allocations will bug.
 	 */
 	kmalloc_caches[KMALLOC_NORMAL][INDEX_NODE] = create_kmalloc_cache(
-				kmalloc_info[INDEX_NODE].name,
+				kmalloc_info[INDEX_NODE].name[KMALLOC_NORMAL],
 				kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS,
 				0, kmalloc_size(INDEX_NODE));
 	slab_state = PARTIAL_NODE;
diff --git a/mm/slab.h b/mm/slab.h
index 9057b8056b07..2fc8f956906a 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -76,7 +76,7 @@ extern struct kmem_cache *kmem_cache;
 
 /* A table of kmalloc cache names and sizes */
 extern const struct kmalloc_info_struct {
-	const char *name;
+	const char *name[NR_KMALLOC_TYPES];
 	unsigned int size;
 } kmalloc_info[];
 
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 807490fe217a..7bd88cc09987 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1092,26 +1092,56 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
 	return kmalloc_caches[kmalloc_type(flags)][index];
 }
 
+#ifdef CONFIG_ZONE_DMA
+#define SET_KMALLOC_SIZE(__size, __short_size)			\
+{								\
+	.name[KMALLOC_NORMAL]  = "kmalloc-" #__short_size,	\
+	.name[KMALLOC_RECLAIM] = "kmalloc-rcl-" #__short_size,	\
+	.name[KMALLOC_DMA]     = "dma-kmalloc-" #__short_size,	\
+	.size = __size,						\
+}
+#else
+#define SET_KMALLOC_SIZE(__size, __short_size)			\
+{								\
+	.name[KMALLOC_NORMAL]  = "kmalloc-" #__short_size,	\
+	.name[KMALLOC_RECLAIM] = "kmalloc-rcl-" #__short_size,	\
+	.size = __size,						\
+}
+#endif
+
 /*
  * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
  * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
  * kmalloc-67108864.
  */
 const struct kmalloc_info_struct kmalloc_info[] __initconst = {
-	{NULL,                      0},		{"kmalloc-96",             96},
-	{"kmalloc-192",           192},		{"kmalloc-8",               8},
-	{"kmalloc-16",             16},		{"kmalloc-32",             32},
-	{"kmalloc-64",             64},		{"kmalloc-128",           128},
-	{"kmalloc-256",           256},		{"kmalloc-512",           512},
-	{"kmalloc-1k",           1024},		{"kmalloc-2k",           2048},
-	{"kmalloc-4k",           4096},		{"kmalloc-8k",           8192},
-	{"kmalloc-16k",         16384},		{"kmalloc-32k",         32768},
-	{"kmalloc-64k",         65536},		{"kmalloc-128k",       131072},
-	{"kmalloc-256k",       262144},		{"kmalloc-512k",       524288},
-	{"kmalloc-1M",        1048576},		{"kmalloc-2M",        2097152},
-	{"kmalloc-4M",        4194304},		{"kmalloc-8M",        8388608},
-	{"kmalloc-16M",      16777216},		{"kmalloc-32M",      33554432},
-	{"kmalloc-64M",      67108864}
+	SET_KMALLOC_SIZE(0, 0),
+	SET_KMALLOC_SIZE(96, 96),
+	SET_KMALLOC_SIZE(192, 192),
+	SET_KMALLOC_SIZE(8, 8),
+	SET_KMALLOC_SIZE(16, 16),
+	SET_KMALLOC_SIZE(32, 32),
+	SET_KMALLOC_SIZE(64, 64),
+	SET_KMALLOC_SIZE(128, 128),
+	SET_KMALLOC_SIZE(256, 256),
+	SET_KMALLOC_SIZE(512, 512),
+	SET_KMALLOC_SIZE(1024, 1k),
+	SET_KMALLOC_SIZE(2048, 2k),
+	SET_KMALLOC_SIZE(4096, 4k),
+	SET_KMALLOC_SIZE(8192, 8k),
+	SET_KMALLOC_SIZE(16384, 16k),
+	SET_KMALLOC_SIZE(32768, 32k),
+	SET_KMALLOC_SIZE(65536, 64k),
+	SET_KMALLOC_SIZE(131072, 128k),
+	SET_KMALLOC_SIZE(262144, 256k),
+	SET_KMALLOC_SIZE(524288, 512k),
+	SET_KMALLOC_SIZE(1048576, 1M),
+	SET_KMALLOC_SIZE(2097152, 2M),
+	SET_KMALLOC_SIZE(4194304, 4M),
+	SET_KMALLOC_SIZE(8388608, 8M),
+	SET_KMALLOC_SIZE(16777216, 16M),
+	SET_KMALLOC_SIZE(33554432, 32M),
+	SET_KMALLOC_SIZE(67108864, 64M)
 };
 
 /*
@@ -1179,18 +1209,11 @@ kmalloc_cache_name(const char *prefix, unsigned int size)
 static void __init
 new_kmalloc_cache(int idx, int type, slab_flags_t flags)
 {
-	const char *name;
-
-	if (type == KMALLOC_RECLAIM) {
+	if (type == KMALLOC_RECLAIM)
 		flags |= SLAB_RECLAIM_ACCOUNT;
-		name = kmalloc_cache_name("kmalloc-rcl",
-						kmalloc_info[idx].size);
-		BUG_ON(!name);
-	} else {
-		name = kmalloc_info[idx].name;
-	}
 
-	kmalloc_caches[type][idx] = create_kmalloc_cache(name,
+	kmalloc_caches[type][idx] = create_kmalloc_cache(
+					kmalloc_info[idx].name[type],
 					kmalloc_info[idx].size, flags, 0,
 					kmalloc_info[idx].size);
 }
@@ -1232,11 +1255,10 @@ void __init create_kmalloc_caches(slab_flags_t flags)
 
 		if (s) {
 			unsigned int size = kmalloc_size(i);
-			const char *n = kmalloc_cache_name("dma-kmalloc", size);
 
-			BUG_ON(!n);
 			kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
-				n, size, SLAB_CACHE_DMA | flags, 0, 0);
+				kmalloc_info[i].name[KMALLOC_DMA],
+				size, SLAB_CACHE_DMA | flags, 0, 0);
 		}
 	}
 #endif
-- 
2.21.0

[PATCH 2/5] mm, slab_common: Remove unused kmalloc_cache_name()

[Pengfei Li]

Since the name of kmalloc can be obtained from kmalloc_info[],
remove the kmalloc_cache_name() that is no longer used.

Signed-off-by: Pengfei Li <lpf.vector@gmail.com>
---
 mm/slab_common.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 7bd88cc09987..002e16673581 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1191,21 +1191,6 @@ void __init setup_kmalloc_cache_index_table(void)
 	}
 }
 
-static const char *
-kmalloc_cache_name(const char *prefix, unsigned int size)
-{
-
-	static const char units[3] = "\0kM";
-	int idx = 0;
-
-	while (size >= 1024 && (size % 1024 == 0)) {
-		size /= 1024;
-		idx++;
-	}
-
-	return kasprintf(GFP_NOWAIT, "%s-%u%c", prefix, size, units[idx]);
-}
-
 static void __init
 new_kmalloc_cache(int idx, int type, slab_flags_t flags)
 {
-- 
2.21.0

[PATCH 3/5] mm, slab: Remove unused kmalloc_size()

[Pengfei Li]

The size of kmalloc can be obtained from kmalloc_info[],
so remove kmalloc_size() that will not be used anymore.

Signed-off-by: Pengfei Li <lpf.vector@gmail.com>
---
 include/linux/slab.h | 20 --------------------
 mm/slab.c            |  5 +++--
 mm/slab_common.c     |  5 ++---
 3 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 56c9c7eed34e..e773e5764b7b 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -557,26 +557,6 @@ static __always_inline void *kmalloc(size_t size, gfp_t flags)
 	return __kmalloc(size, flags);
 }
 
-/*
- * Determine size used for the nth kmalloc cache.
- * return size or 0 if a kmalloc cache for that
- * size does not exist
- */
-static __always_inline unsigned int kmalloc_size(unsigned int n)
-{
-#ifndef CONFIG_SLOB
-	if (n > 2)
-		return 1U << n;
-
-	if (n == 1 && KMALLOC_MIN_SIZE <= 32)
-		return 96;
-
-	if (n == 2 && KMALLOC_MIN_SIZE <= 64)
-		return 192;
-#endif
-	return 0;
-}
-
 static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
 {
 #ifndef CONFIG_SLOB
diff --git a/mm/slab.c b/mm/slab.c
index c42b6211f42e..7bc4e90e1147 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1248,8 +1248,9 @@ void __init kmem_cache_init(void)
 	 */
 	kmalloc_caches[KMALLOC_NORMAL][INDEX_NODE] = create_kmalloc_cache(
 				kmalloc_info[INDEX_NODE].name[KMALLOC_NORMAL],
-				kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS,
-				0, kmalloc_size(INDEX_NODE));
+				kmalloc_info[INDEX_NODE].size,
+				ARCH_KMALLOC_FLAGS, 0,
+				kmalloc_info[INDEX_NODE].size);
 	slab_state = PARTIAL_NODE;
 	setup_kmalloc_cache_index_table();
 
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 002e16673581..8b542cfcc4f2 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1239,11 +1239,10 @@ void __init create_kmalloc_caches(slab_flags_t flags)
 		struct kmem_cache *s = kmalloc_caches[KMALLOC_NORMAL][i];
 
 		if (s) {
-			unsigned int size = kmalloc_size(i);
-
 			kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
 				kmalloc_info[i].name[KMALLOC_DMA],
-				size, SLAB_CACHE_DMA | flags, 0, 0);
+				kmalloc_info[i].size,
+				SLAB_CACHE_DMA | flags, 0, 0);
 		}
 	}
 #endif
-- 
2.21.0

[PATCH 4/5] mm, slab_common: Make 'type' is enum kmalloc_cache_type

[Pengfei Li]

The 'type' of the function new_kmalloc_cache should be
enum kmalloc_cache_type instead of int, so correct it.

Signed-off-by: Pengfei Li <lpf.vector@gmail.com>
---
 mm/slab_common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 8b542cfcc4f2..af45b5278fdc 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1192,7 +1192,7 @@ void __init setup_kmalloc_cache_index_table(void)
 }
 
 static void __init
-new_kmalloc_cache(int idx, int type, slab_flags_t flags)
+new_kmalloc_cache(int idx, enum kmalloc_cache_type type, slab_flags_t flags)
 {
 	if (type == KMALLOC_RECLAIM)
 		flags |= SLAB_RECLAIM_ACCOUNT;
@@ -1210,7 +1210,8 @@ new_kmalloc_cache(int idx, int type, slab_flags_t flags)
  */
 void __init create_kmalloc_caches(slab_flags_t flags)
 {
-	int i, type;
+	int i;
+	enum kmalloc_cache_type type;
 
 	for (type = KMALLOC_NORMAL; type <= KMALLOC_RECLAIM; type++) {
 		for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
-- 
2.21.0

[PATCH 5/5] mm, slab_common: Make initializing KMALLOC_DMA start from 1

[Pengfei Li]

kmalloc_caches[KMALLOC_NORMAL][0] will never be initialized,
so the loop should start at 1 instead of 0

Signed-off-by: Pengfei Li <lpf.vector@gmail.com>
---
 mm/slab_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index af45b5278fdc..c81fc7dc2946 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1236,7 +1236,7 @@ void __init create_kmalloc_caches(slab_flags_t flags)
 	slab_state = UP;
 
 #ifdef CONFIG_ZONE_DMA
-	for (i = 0; i <= KMALLOC_SHIFT_HIGH; i++) {
+	for (i = 1; i <= KMALLOC_SHIFT_HIGH; i++) {
 		struct kmem_cache *s = kmalloc_caches[KMALLOC_NORMAL][i];
 
 		if (s) {
-- 
2.21.0

Re: [PATCH 0/5] mm, slab: Make kmalloc_info[] contain all types of names

[Christopher Lameter]

On Wed, 4 Sep 2019, Pengfei Li wrote:

> There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
> and KMALLOC_DMA.

I only got a few patches of this set. Can I see the complete patchset
somewhere?

Re: [PATCH 0/5] mm, slab: Make kmalloc_info[] contain all types of names

[Pengfei Li]

On Thu, Sep 5, 2019 at 3:27 AM Christopher Lameter <cl@linux.com> wrote:
>
> On Wed, 4 Sep 2019, Pengfei Li wrote:
>
> > There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
> > and KMALLOC_DMA.
>
> I only got a few patches of this set. Can I see the complete patchset
> somewhere?

Yes, you can get the patches from
https://patchwork.kernel.org/cover/11128325/ or
https://lore.kernel.org/patchwork/cover/1123412/

Hope you like it :)

Re: [PATCH 0/5] mm, slab: Make kmalloc_info[] contain all types of names

[Vlastimil Babka]

On 9/3/19 6:04 PM, Pengfei Li wrote:
> There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
> and KMALLOC_DMA.
> 
> The name of KMALLOC_NORMAL is contained in kmalloc_info[].name,
> but the names of KMALLOC_RECLAIM and KMALLOC_DMA are dynamically
> generated by kmalloc_cache_name().
> 
> Patch1 predefines the names of all types of kmalloc to save
> the time spent dynamically generating names.
> 
> The other 4 patches did some cleanup work.
> 
> These changes make sense, and the time spent by new_kmalloc_cache()
> has been reduced by approximately 36.3%.
> 
>                          Time spent by
>                          new_kmalloc_cache()
> 5.3-rc7                       66264
> 5.3-rc7+patch                 42188

Note that the caches are created only once upon boot, so I doubt that
these time savings (is it in CPU cycles?) will be noticeable at all. But
diffstat looks ok, and it avoids using kmalloc() (via kasprintf()) to
allocate names for kmalloc(), so in that sense I think it's worthwhile
to consider. Thanks.

> Pengfei Li (5):
>   mm, slab: Make kmalloc_info[] contain all types of names
>   mm, slab_common: Remove unused kmalloc_cache_name()
>   mm, slab: Remove unused kmalloc_size()
>   mm, slab_common: Make 'type' is enum kmalloc_cache_type
>   mm, slab_common: Make initializing KMALLOC_DMA start from 1
> 
>  include/linux/slab.h |  20 ---------
>  mm/slab.c            |   7 +--
>  mm/slab.h            |   2 +-
>  mm/slab_common.c     | 101 +++++++++++++++++++++++--------------------
>  4 files changed, 59 insertions(+), 71 deletions(-)
> 

Re: [PATCH 0/5] mm, slab: Make kmalloc_info[] contain all types of names

[Pengfei Li]

On Thu, Sep 5, 2019 at 8:25 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 9/3/19 6:04 PM, Pengfei Li wrote:
> > There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
> > and KMALLOC_DMA.
> >
> > The name of KMALLOC_NORMAL is contained in kmalloc_info[].name,
> > but the names of KMALLOC_RECLAIM and KMALLOC_DMA are dynamically
> > generated by kmalloc_cache_name().
> >
> > Patch1 predefines the names of all types of kmalloc to save
> > the time spent dynamically generating names.
> >
> > The other 4 patches did some cleanup work.
> >
> > These changes make sense, and the time spent by new_kmalloc_cache()
> > has been reduced by approximately 36.3%.
> >
> >                          Time spent by
> >                          new_kmalloc_cache()
> > 5.3-rc7                       66264
> > 5.3-rc7+patch                 42188
>
> Note that the caches are created only once upon boot, so I doubt that

Thank you for your comments.
Yes, kmalloc-xxx are only created at boot time.

> these time savings (is it in CPU cycles?) will be noticeable at all.

Yes, it is CPU cycles.

> But diffstat looks ok, and it avoids using kmalloc() (via kasprintf()) to
> allocate names for kmalloc(), so in that sense I think it's worthwhile
> to consider. Thanks.
>

Thanks.

> > Pengfei Li (5):
> >   mm, slab: Make kmalloc_info[] contain all types of names
> >   mm, slab_common: Remove unused kmalloc_cache_name()
> >   mm, slab: Remove unused kmalloc_size()
> >   mm, slab_common: Make 'type' is enum kmalloc_cache_type
> >   mm, slab_common: Make initializing KMALLOC_DMA start from 1
> >
> >  include/linux/slab.h |  20 ---------
> >  mm/slab.c            |   7 +--
> >  mm/slab.h            |   2 +-
> >  mm/slab_common.c     | 101 +++++++++++++++++++++++--------------------
> >  4 files changed, 59 insertions(+), 71 deletions(-)
> >
>

Re: [PATCH 1/5] mm, slab: Make kmalloc_info[] contain all types of names

[Vlastimil Babka]

On 9/3/19 6:04 PM, Pengfei Li wrote:
> There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
> and KMALLOC_DMA.
> 
> The name of KMALLOC_NORMAL is contained in kmalloc_info[].name,
> but the names of KMALLOC_RECLAIM and KMALLOC_DMA are dynamically
> generated by kmalloc_cache_name().
> 
> This patch predefines the names of all types of kmalloc to save
> the time spent dynamically generating names.

As I said, IMHO it's more useful that we don't need to allocate the 
names dynamically anymore, and it's simpler overall.

> Signed-off-by: Pengfei Li <lpf.vector@gmail.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

>   /*
>    * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
>    * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
>    * kmalloc-67108864.
>    */
>   const struct kmalloc_info_struct kmalloc_info[] __initconst = {

BTW should it really be an __initconst, when references to the names 
keep on living in kmem_cache structs? Isn't this for data that's 
discarded after init?

Re: [PATCH 2/5] mm, slab_common: Remove unused kmalloc_cache_name()

[Vlastimil Babka]

On 9/3/19 6:04 PM, Pengfei Li wrote:
> Since the name of kmalloc can be obtained from kmalloc_info[],
> remove the kmalloc_cache_name() that is no longer used.

That could simply be part of patch 1/5 really.

> Signed-off-by: Pengfei Li <lpf.vector@gmail.com>

Ack

> ---
>   mm/slab_common.c | 15 ---------------
>   1 file changed, 15 deletions(-)

Re: [PATCH 3/5] mm, slab: Remove unused kmalloc_size()

[Vlastimil Babka]

On 9/3/19 6:04 PM, Pengfei Li wrote:
> The size of kmalloc can be obtained from kmalloc_info[],
> so remove kmalloc_size() that will not be used anymore.
> 
> Signed-off-by: Pengfei Li <lpf.vector@gmail.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>   include/linux/slab.h | 20 --------------------
>   mm/slab.c            |  5 +++--
>   mm/slab_common.c     |  5 ++---
>   3 files changed, 5 insertions(+), 25 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 56c9c7eed34e..e773e5764b7b 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -557,26 +557,6 @@ static __always_inline void *kmalloc(size_t size, gfp_t flags)
>   	return __kmalloc(size, flags);
>   }
>   
> -/*
> - * Determine size used for the nth kmalloc cache.
> - * return size or 0 if a kmalloc cache for that
> - * size does not exist
> - */
> -static __always_inline unsigned int kmalloc_size(unsigned int n)
> -{
> -#ifndef CONFIG_SLOB
> -	if (n > 2)
> -		return 1U << n;
> -
> -	if (n == 1 && KMALLOC_MIN_SIZE <= 32)
> -		return 96;
> -
> -	if (n == 2 && KMALLOC_MIN_SIZE <= 64)
> -		return 192;
> -#endif
> -	return 0;
> -}
> -
>   static __always_inline void *kmalloc_node(size_t size, gfp_t flags, int node)
>   {
>   #ifndef CONFIG_SLOB
> diff --git a/mm/slab.c b/mm/slab.c
> index c42b6211f42e..7bc4e90e1147 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1248,8 +1248,9 @@ void __init kmem_cache_init(void)
>   	 */
>   	kmalloc_caches[KMALLOC_NORMAL][INDEX_NODE] = create_kmalloc_cache(
>   				kmalloc_info[INDEX_NODE].name[KMALLOC_NORMAL],
> -				kmalloc_size(INDEX_NODE), ARCH_KMALLOC_FLAGS,
> -				0, kmalloc_size(INDEX_NODE));
> +				kmalloc_info[INDEX_NODE].size,
> +				ARCH_KMALLOC_FLAGS, 0,
> +				kmalloc_info[INDEX_NODE].size);
>   	slab_state = PARTIAL_NODE;
>   	setup_kmalloc_cache_index_table();
>   
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 002e16673581..8b542cfcc4f2 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1239,11 +1239,10 @@ void __init create_kmalloc_caches(slab_flags_t flags)
>   		struct kmem_cache *s = kmalloc_caches[KMALLOC_NORMAL][i];
>   
>   		if (s) {
> -			unsigned int size = kmalloc_size(i);
> -
>   			kmalloc_caches[KMALLOC_DMA][i] = create_kmalloc_cache(
>   				kmalloc_info[i].name[KMALLOC_DMA],
> -				size, SLAB_CACHE_DMA | flags, 0, 0);
> +				kmalloc_info[i].size,
> +				SLAB_CACHE_DMA | flags, 0, 0);
>   		}
>   	}
>   #endif
> 

Re: [PATCH 4/5] mm, slab_common: Make 'type' is enum kmalloc_cache_type

[Vlastimil Babka]

On 9/3/19 6:04 PM, Pengfei Li wrote:
> The 'type' of the function new_kmalloc_cache should be
> enum kmalloc_cache_type instead of int, so correct it.

OK

> Signed-off-by: Pengfei Li <lpf.vector@gmail.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>   mm/slab_common.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 8b542cfcc4f2..af45b5278fdc 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1192,7 +1192,7 @@ void __init setup_kmalloc_cache_index_table(void)
>   }
>   
>   static void __init
> -new_kmalloc_cache(int idx, int type, slab_flags_t flags)
> +new_kmalloc_cache(int idx, enum kmalloc_cache_type type, slab_flags_t flags)
>   {
>   	if (type == KMALLOC_RECLAIM)
>   		flags |= SLAB_RECLAIM_ACCOUNT;
> @@ -1210,7 +1210,8 @@ new_kmalloc_cache(int idx, int type, slab_flags_t flags)
>    */
>   void __init create_kmalloc_caches(slab_flags_t flags)
>   {
> -	int i, type;
> +	int i;
> +	enum kmalloc_cache_type type;
>   
>   	for (type = KMALLOC_NORMAL; type <= KMALLOC_RECLAIM; type++) {
>   		for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) {
> 

Re: [PATCH 1/5] mm, slab: Make kmalloc_info[] contain all types of names

[Pengfei Li]

On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 9/3/19 6:04 PM, Pengfei Li wrote:
> > There are three types of kmalloc, KMALLOC_NORMAL, KMALLOC_RECLAIM
> > and KMALLOC_DMA.
> >
> > The name of KMALLOC_NORMAL is contained in kmalloc_info[].name,
> > but the names of KMALLOC_RECLAIM and KMALLOC_DMA are dynamically
> > generated by kmalloc_cache_name().
> >
> > This patch predefines the names of all types of kmalloc to save
> > the time spent dynamically generating names.
>
> As I said, IMHO it's more useful that we don't need to allocate the
> names dynamically anymore, and it's simpler overall.
>

Thank you very much for your review.

> > Signed-off-by: Pengfei Li <lpf.vector@gmail.com>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
>
> >   /*
> >    * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
> >    * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
> >    * kmalloc-67108864.
> >    */
> >   const struct kmalloc_info_struct kmalloc_info[] __initconst = {
>
> BTW should it really be an __initconst, when references to the names
> keep on living in kmem_cache structs? Isn't this for data that's
> discarded after init?

You are right, I will remove __initconst in v2.

Re: [PATCH 2/5] mm, slab_common: Remove unused kmalloc_cache_name()

[Pengfei Li]

On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> On 9/3/19 6:04 PM, Pengfei Li wrote:
> > Since the name of kmalloc can be obtained from kmalloc_info[],
> > remove the kmalloc_cache_name() that is no longer used.
>
> That could simply be part of patch 1/5 really.
>

Ok, thanks.

> > Signed-off-by: Pengfei Li <lpf.vector@gmail.com>
>
> Ack
>
> > ---
> >   mm/slab_common.c | 15 ---------------
> >   1 file changed, 15 deletions(-)

Re: [PATCH 1/5] mm, slab: Make kmalloc_info[] contain all types of names

[Rasmus Villemoes]

On 09/09/2019 18.53, Pengfei Li wrote:
> On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <vbabka@suse.cz> wrote:

>>>   /*
>>>    * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
>>>    * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
>>>    * kmalloc-67108864.
>>>    */
>>>   const struct kmalloc_info_struct kmalloc_info[] __initconst = {
>>
>> BTW should it really be an __initconst, when references to the names
>> keep on living in kmem_cache structs? Isn't this for data that's
>> discarded after init?
> 
> You are right, I will remove __initconst in v2.

No, __initconst is correct, and should be kept. The string literals
which the .name pointers point to live in .rodata, and we're copying the
values of these .name pointers. Nothing refers to something inside
kmalloc_info[] after init. (It would be a whole different matter if
struct kmalloc_info_struct consisted of { char name[NN]; unsigned int
size; }).

Rasmus

Re: [PATCH 1/5] mm, slab: Make kmalloc_info[] contain all types of names

[Vlastimil Babka]

On 9/9/19 8:30 PM, Rasmus Villemoes wrote:
> On 09/09/2019 18.53, Pengfei Li wrote:
>> On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <vbabka@suse.cz> wrote:
> 
>>>>   /*
>>>>    * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
>>>>    * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
>>>>    * kmalloc-67108864.
>>>>    */
>>>>   const struct kmalloc_info_struct kmalloc_info[] __initconst = {
>>>
>>> BTW should it really be an __initconst, when references to the names
>>> keep on living in kmem_cache structs? Isn't this for data that's
>>> discarded after init?
>>
>> You are right, I will remove __initconst in v2.
> 
> No, __initconst is correct, and should be kept. The string literals
> which the .name pointers point to live in .rodata, and we're copying the
> values of these .name pointers. Nothing refers to something inside
> kmalloc_info[] after init. (It would be a whole different matter if
> struct kmalloc_info_struct consisted of { char name[NN]; unsigned int
> size; }).

*slaps forehead* ah, of course, string literals themselves are not
affected by the __initconst, thanks! Sorry for the wrong suggestion Pengfei.

> Rasmus
> 

Re: [PATCH 1/5] mm, slab: Make kmalloc_info[] contain all types of names

[Pengfei Li]

On Tue, Sep 10, 2019 at 2:30 AM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 09/09/2019 18.53, Pengfei Li wrote:
> > On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> >>>   /*
> >>>    * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time.
> >>>    * kmalloc_index() supports up to 2^26=64MB, so the final entry of the table is
> >>>    * kmalloc-67108864.
> >>>    */
> >>>   const struct kmalloc_info_struct kmalloc_info[] __initconst = {
> >>
> >> BTW should it really be an __initconst, when references to the names
> >> keep on living in kmem_cache structs? Isn't this for data that's
> >> discarded after init?
> >
> > You are right, I will remove __initconst in v2.
>
> No, __initconst is correct, and should be kept. The string literals
> which the .name pointers point to live in .rodata, and we're copying the
> values of these .name pointers. Nothing refers to something inside
> kmalloc_info[] after init. (It would be a whole different matter if
> struct kmalloc_info_struct consisted of { char name[NN]; unsigned int
> size; }).
>

Thank you for your comment. I will keep it in v3.

I did learn :)


> Rasmus