2019-09-10 18:47:15

by Pengfei Li

[permalink] [raw]
Subject: [PATCH v3 0/4] mm, slab: Make kmalloc_info[] contain all types of names

Changes in v3
--
1. restore __initconst (patch 1/4)
2. rename patch 3/4
3. add more clarification for patch 4/4

Changes in v2
--
1. remove __initconst (patch 1/5)
2. squash patch 2/5
3. add ack tag from Vlastimil Babka


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 (4):
mm, slab: Make kmalloc_info[] contain all types of names
mm, slab: Remove unused kmalloc_size()
mm, slab_common: use enum kmalloc_cache_type to iterate over kmalloc
caches
mm, slab_common: Make the loop for 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


2019-09-10 18:47:27

by Pengfei Li

[permalink] [raw]
Subject: [PATCH v3 4/4] mm, slab_common: Make the loop for initializing KMALLOC_DMA start from 1

KMALLOC_DMA will be initialized only if KMALLOC_NORMAL with
the same index exists.

And kmalloc_caches[KMALLOC_NORMAL][0] is always NULL.

Therefore, the loop that initializes KMALLOC_DMA should start
at 1 instead of 0, which will reduce 1 meaningless attempt.

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

2019-09-10 18:55:09

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] mm, slab_common: Make the loop for initializing KMALLOC_DMA start from 1

On 9/10/19 3:26 AM, Pengfei Li wrote:
> KMALLOC_DMA will be initialized only if KMALLOC_NORMAL with
> the same index exists.
>
> And kmalloc_caches[KMALLOC_NORMAL][0] is always NULL.
>
> Therefore, the loop that initializes KMALLOC_DMA should start
> at 1 instead of 0, which will reduce 1 meaningless attempt.

IMHO the saving of one iteration isn't worth making the code more
subtle. KMALLOC_SHIFT_LOW would be nice, but that would skip 1 + 2 which
are special.

Since you're doing these cleanups, have you considered reordering
kmalloc_info, size_index, kmalloc_index() etc so that sizes 96 and 192
are ordered naturally between 64, 128 and 256? That should remove
various special casing such as in create_kmalloc_caches(). I can't
guarantee it will be possible without breaking e.g. constant folding
optimizations etc., but seems to me it should be feasible. (There are
definitely more places to change than those I listed.)

> 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) {
>

2019-09-11 14:36:10

by Pengfei Li

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] mm, slab_common: Make the loop for initializing KMALLOC_DMA start from 1

On Tue, Sep 10, 2019 at 6:26 PM Vlastimil Babka <[email protected]> wrote:
>
> On 9/10/19 3:26 AM, Pengfei Li wrote:
> > KMALLOC_DMA will be initialized only if KMALLOC_NORMAL with
> > the same index exists.
> >
> > And kmalloc_caches[KMALLOC_NORMAL][0] is always NULL.
> >
> > Therefore, the loop that initializes KMALLOC_DMA should start
> > at 1 instead of 0, which will reduce 1 meaningless attempt.
>
> IMHO the saving of one iteration isn't worth making the code more
> subtle. KMALLOC_SHIFT_LOW would be nice, but that would skip 1 + 2 which
> are special.
>

Yes, I agree with you.
This really makes the code more subtle.

> Since you're doing these cleanups, have you considered reordering
> kmalloc_info, size_index, kmalloc_index() etc so that sizes 96 and 192
> are ordered naturally between 64, 128 and 256? That should remove
> various special casing such as in create_kmalloc_caches(). I can't
> guarantee it will be possible without breaking e.g. constant folding
> optimizations etc., but seems to me it should be feasible. (There are
> definitely more places to change than those I listed.)
>

In the past two days, I am working on what you suggested.

So far, I have completed the coding work, but I need some time to make
sure there are no bugs and verify the impact on performance.

I will send v4 soon.

Thank you for your review and suggestions.

--
Pengfei

> > 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) {
> >
>

2019-09-12 09:38:03

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] mm, slab_common: Make the loop for initializing KMALLOC_DMA start from 1

On 9/11/19 4:33 PM, Pengfei Li wrote:
> In the past two days, I am working on what you suggested.

Great!

> So far, I have completed the coding work, but I need some time to make
> sure there are no bugs and verify the impact on performance.

It would probably be hard to measure with sufficient confidence in terms
of runtime performance, but you could use e.g. ./scripts/bloat-o-meter
to look for unexpected code increase due to compile-time optimizations
becoming runtime.