[PATCH] mm/slub: remove duplicate initialization for early_kmem_cache_node_alloc()

[PATCH] mm/slub: remove duplicate initialization for early_kmem_cache_node_alloc()

[Sangyun Kim]

The struct track for every object in a new slab is already set up by
new_slab(),
so remove the duplicate initialization in early_kmem_cache_node_alloc().

Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>

Co-developed-by: Hyunmin Lee <hyunminlr@gmail.com>
Signed-off-by: Hyunmin Lee <hyunminlr@gmail.com>

Co-developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 mm/slub.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index 0dfc0c18a78b..5ffe46843b36 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4938,7 +4938,6 @@ static void early_kmem_cache_node_alloc(int node)
 	BUG_ON(!n);
 #ifdef CONFIG_SLUB_DEBUG
 	init_object(kmem_cache_node, n, SLUB_RED_ACTIVE);
-	init_tracking(kmem_cache_node, n);
 #endif
 	n = kasan_slab_alloc(kmem_cache_node, n, GFP_KERNEL, false);
 	slab->freelist = get_freepointer(kmem_cache_node, n);
-- 
2.25.1

Re: [PATCH] mm/slub: remove duplicate initialization for early_kmem_cache_node_alloc()

[David Rientjes]

On Sat, 6 Apr 2024, Sangyun Kim wrote:

> The struct track for every object in a new slab is already set up by
> new_slab(),
> so remove the duplicate initialization in early_kmem_cache_node_alloc().
> 
> Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> 
> Co-developed-by: Hyunmin Lee <hyunminlr@gmail.com>
> Signed-off-by: Hyunmin Lee <hyunminlr@gmail.com>
> 
> Co-developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>
> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  mm/slub.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 0dfc0c18a78b..5ffe46843b36 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4938,7 +4938,6 @@ static void early_kmem_cache_node_alloc(int node)
>  	BUG_ON(!n);
>  #ifdef CONFIG_SLUB_DEBUG
>  	init_object(kmem_cache_node, n, SLUB_RED_ACTIVE);
> -	init_tracking(kmem_cache_node, n);
>  #endif
>  	n = kasan_slab_alloc(kmem_cache_node, n, GFP_KERNEL, false);
>  	slab->freelist = get_freepointer(kmem_cache_node, n);

I think this is technically safe based on the current implementation 
because, as you said, allocate_slab() takes care of this for 
SLAB_STORE_USER.

What user observable effect does this have given it would only make a 
difference when slab_state == DOWN?

Re: [PATCH] mm/slub: remove duplicate initialization for early_kmem_cache_node_alloc()

[Christoph Lameter (Ampere)]

On Sun, 7 Apr 2024, David Rientjes wrote:

> What user observable effect does this have given it would only make a
> difference when slab_state == DOWN?

It reduces the kernel text size and removes a line from source code.

Re: [PATCH] mm/slub: remove duplicate initialization for early_kmem_cache_node_alloc()

[김상윤]

On Tue, Apr 9, 2024 at 2:23 AM Christoph Lameter (Ampere) <cl@linux.com> wrote:
>
> On Sun, 7 Apr 2024, David Rientjes wrote:
>
> > What user observable effect does this have given it would only make a
> > difference when slab_state == DOWN?
>
> It reduces the kernel text size and removes a line from source code.
>
>
>

Yes, it only affects when slab_state == DOWN.
early_kmem_cache_node_alloc() is called exclusively
when the slab_state == DOWN, by init_kmem_cache_nodes().

Re: [PATCH] mm/slub: remove duplicate initialization for early_kmem_cache_node_alloc()

[Vlastimil Babka]

On 4/8/24 7:14 PM, Christoph Lameter (Ampere) wrote:
> On Sun, 7 Apr 2024, David Rientjes wrote:
> 
>> What user observable effect does this have given it would only make a
>> difference when slab_state == DOWN?
> 
> It reduces the kernel text size and removes a line from source code.

Yeah it's not helpful to have redundant code, it may mislead people that
it's actually important.

Merged to slab/for-next, thanks