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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
kmalloc_caches[KMALLOC_NORMAL][0] will never be initialized,
so the loop should start at 1 instead of 0
Signed-off-by: Pengfei Li <[email protected]>
---
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
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?
On Thu, Sep 5, 2019 at 3:27 AM Christopher Lameter <[email protected]> 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 :)
On Thu, Sep 5, 2019 at 8:25 PM Vlastimil Babka <[email protected]> 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(-)
> >
>
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(-)
>
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 <[email protected]>
Ack
> ---
> mm/slab_common.c | 15 ---------------
> 1 file changed, 15 deletions(-)
On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <[email protected]> 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 <[email protected]>
>
> Ack
>
> > ---
> > mm/slab_common.c | 15 ---------------
> > 1 file changed, 15 deletions(-)
On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <[email protected]> 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 <[email protected]>
>
> Acked-by: Vlastimil Babka <[email protected]>
>
> > /*
> > * 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.
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 <[email protected]>
Acked-by: Vlastimil Babka <[email protected]>
> /*
> * 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?
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 <[email protected]>
Acked-by: Vlastimil Babka <[email protected]>
> ---
> 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
>
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 <[email protected]>
Acked-by: Vlastimil Babka <[email protected]>
> ---
> 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++) {
>
On 09/09/2019 18.53, Pengfei Li wrote:
> On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <[email protected]> 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
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 <[email protected]> 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
>
On Tue, Sep 10, 2019 at 2:30 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 09/09/2019 18.53, Pengfei Li wrote:
> > On Mon, Sep 9, 2019 at 10:59 PM Vlastimil Babka <[email protected]> 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