Re: [PATCH v3] mm/slub: free prefilled empty sheaves consistently

[Harry Yoo <harry@kernel.org>]

On 5/25/26 10:10 PM, hu.shengming@zte.com.cn wrote:
> From: Shengming Hu <hu.shengming@zte.com.cn>
> 
> Empty sheaves allocated by alloc_empty_sheaf() should be released with
> free_empty_sheaf().
> 
> Move the empty-sheaf capacity initialization into __alloc_empty_sheaf()
> and use it for oversized prefilled sheaves as well.
> 
> This keeps the prefill paths consistent: after flushing an oversized or
> pfmemalloc sheaf, release the now-empty sheaf through free_empty_sheaf()
> instead of kfree().
> 
> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> ---
>   mm/slub.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 04692a6f9128..f4c06bf43f2d 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2782,6 +2782,7 @@ static struct slab_sheaf *__alloc_empty_sheaf(struct kmem_cache *s, gfp_t gfp,
>   		return NULL;
> 
>   	sheaf->cache = s;
> +	sheaf->capacity = capacity;

the capacity field is only used by oversized sheaves (for now).
It should be outside __alloc_empty_sheaf().

>   	stat(s, SHEAF_ALLOC);
> 
> @@ -5015,21 +5016,15 @@ kmem_cache_prefill_sheaf(struct kmem_cache *s, gfp_t gfp, unsigned int size)
> 
>   	if (unlikely(size > s->sheaf_capacity)) {
> 
> -		sheaf = kzalloc_flex(*sheaf, objects, size, gfp);
> +		sheaf = __alloc_empty_sheaf(s, gfp, size);
>   		if (!sheaf)
>   			return NULL;
> 
>   		stat(s, SHEAF_PREFILL_OVERSIZE);

So now SHEAF_ALLOC and SHEAF_FREE include oversized sheaves.
I think that's fine, but can we update the commit message and the comment?

the comment:
SHEAF_ALLOC, /* Allocation of an empty sheaf including oversized ones */
SHEAF_FREE,  /* Freeing of an empty sheaf including oversized ones */

> -		sheaf->cache = s;
> -		sheaf->capacity = size;
> -		/*
> -		 * we do not need to care about pfmemalloc here because oversize
> -		 * sheaves area always flushed and freed when returned
> -		 */

the comment is still worth having?

>   		if (!__kmem_cache_alloc_bulk(s, gfp, size,
>   					     &sheaf->objects[0])) {
> -			kfree(sheaf);
> +			free_empty_sheaf(s, sheaf);
>   			return NULL;
>   		}

-- 
Cheers,
Harry / Hyeonggon