Re: [PATCH V1] mm/slub: fix memory leak in free_to_pcs_bulk()

["Liam R. Howlett" <Liam.Howlett@oracle.com>]

* Harry Yoo <harry.yoo@oracle.com> [251111 07:55]:
> The commit 989b09b73978 ("slab: skip percpu sheaves for remote object
> freeing") introduced the remote_objects array in free_to_pcs_bulk() to
> skip sheaves when objects from a remote node are freed.
> 
> However, the array is flushed only when:
>   1) the array becomes full (++remote_nr >= PCS_BATCH_MAX), or
>   2) slab_free_hook() returns false and size becomes zero.
> 
> When neither of the conditions is met, objects in the array are leaked.
> This resulted in a memory leak [1], where 82 GiB of memory was allocated
> for the maple_node cache.
> 
> Flush the array after successfully freeing objects to sheaves
> in the do_free: path.
> 
> In the meantime, move the snippet if (!size) goto flush_remote; outside
> the while loop for readability. Let's say all objects in the array are
> from a remote node: then we acquire s->cpu_sheaves->lock and try to free
> an object even when size is zero. This doesn't appear to be harmful,
> but isn't really readable.
> 
> Reported-by: Tytus Rogalewski <tytanick@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220765 [1]
> Closes: https://lore.kernel.org/linux-mm/20251107094809.12e9d705b7bf4815783eb184@linux-foundation.org
> Closes: https://lore.kernel.org/all/aRGDTwbt2EIz2CYn@hyeyoo
> Fixes: 989b09b73978 ("slab: skip percpu sheaves for remote object freeing")
> Signed-off-by: Harry Yoo <harry.yoo@oracle.com>


Thanks Harry.

Acked-by: Liam R. Howlett <Liam.Howlett@oracle.com>

> ---
>  mm/slub.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index f1a5373eee7b..a787687a0d59 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6332,8 +6332,6 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  
>  		if (unlikely(!slab_free_hook(s, p[i], init, false))) {
>  			p[i] = p[--size];
> -			if (!size)
> -				goto flush_remote;
>  			continue;
>  		}
>  
> @@ -6348,6 +6346,9 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  		i++;
>  	}
>  
> +	if (!size)
> +		goto flush_remote;
> +
>  next_batch:
>  	if (!local_trylock(&s->cpu_sheaves->lock))
>  		goto fallback;
> @@ -6402,6 +6403,9 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  		goto next_batch;
>  	}
>  
> +	if (remote_nr)
> +		goto flush_remote;
> +
>  	return;
>  
>  no_empty:
> -- 
> 2.43.0
>