Re: [PATCH v4] mm/slub: use empty sheaf helpers for oversized sheaves

[Vlastimil Babka (SUSE)]

On 5/26/26 17:24, hu.shengming@xxxxxxxxxx wrote:
> From: Shengming Hu <hu.shengming@xxxxxxxxxx>
> 
> Oversized prefilled sheaves are allocated separately, but after they are
> flushed they are empty sheaves as well. Release them through
> free_empty_sheaf() instead of calling kfree() directly.
> 
> Use __alloc_empty_sheaf() for oversized prefilled sheaves as well, so
> that they are allocated through the same helper as regular empty sheaves
> and released through the matching helper.
> 
> Since oversized sheaves are now allocated and freed through the empty
> sheaf helpers, SHEAF_ALLOC and SHEAF_FREE also account for oversized
> sheaves. Update the stat comments accordingly.
> 
> This keeps the oversized and pfmemalloc prefill paths consistent after
> flushing.
> 
> Signed-off-by: Shengming Hu <hu.shengming@xxxxxxxxxx>

Hmm, __alloc_empty_sheaf() does "gfp &= ~OBJCGS_CLEAR_MASK" which includes
__GFP_NOFAIL. A caller of kmem_cache_prefill_sheaf() with __GFP_NOFAIL might
not expect failure. The __GFP_NO_OBJ_EXT handling is also not necessary for
prefilled sheaves.

Well maybe all the gfp handling could be simply moved to
alloc_empty_sheaf()? The other caller of __alloc_empty_sheaf() is
bootstrap_cache_sheaves() and I think it doesn't need it either.

> ---
> Changes in v2:
> - Rework the change as suggested by Harry.
> - Teach __alloc_empty_sheaf() to initialize capacity and pfmemalloc.
> - Allocate oversized prefilled sheaves through __alloc_empty_sheaf().
> - Free flushed oversized and pfmemalloc sheaves through free_empty_sheaf().
> - Link to v1: https://lore.kernel.org/all/20260521195015105Y4zvKHj0TfPZEujixy9Vo@xxxxxxxxxx/
> 
> Changes in v3:
> - Address Hao's comments:
> - Drop the redundant `pfmemalloc` initialization in
>   __alloc_empty_sheaf().
> - Keep initializing `capacity` and `pfmemalloc` in the normal-sized
>   prefill path, since the sheaf may be reused rather than freshly
>   allocated.
> - Link to v2: https://lore.kernel.org/all/20260522145900248m-nBcy07_SCDk2ATDWfmg@xxxxxxxxxx/
> 
> Changes in v4:
> - Address comments from Hao and Harry:
> - Keep capacity initialization in the oversized prefill path.
> - Update SHEAF_ALLOC and SHEAF_FREE comments to mention oversized sheaves.
> - Restore the oversized sheaf pfmemalloc comment.
> - Link to v3: https://lore.kernel.org/all/20260525211000387LYqTHmxYL900XIB8qwV3h@xxxxxxxxxx/
> 
> ---
>  mm/slub.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 04692a6f9128..08812de7a8b5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -362,8 +362,8 @@ enum stat_item {
>  	CMPXCHG_DOUBLE_FAIL,	/* Failures of slab freelist update */
>  	SHEAF_FLUSH,		/* Objects flushed from a sheaf */
>  	SHEAF_REFILL,		/* Objects refilled to a sheaf */
> -	SHEAF_ALLOC,		/* Allocation of an empty sheaf */
> -	SHEAF_FREE,		/* Freeing of an empty sheaf */
> +	SHEAF_ALLOC,		/* Allocation of an empty sheaf including oversized ones */
> +	SHEAF_FREE,		/* Freeing of an empty sheaf including oversized ones */
>  	BARN_GET,		/* Got full sheaf from barn */
>  	BARN_GET_FAIL,		/* Failed to get full sheaf from barn */
>  	BARN_PUT,		/* Put full sheaf to barn */
> @@ -5015,12 +5015,11 @@ 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);
> -		sheaf->cache = s;
>  		sheaf->capacity = size;
> 
>  		/*
> @@ -5029,7 +5028,7 @@ kmem_cache_prefill_sheaf(struct kmem_cache *s, gfp_t gfp, unsigned int size)
>  		 */
>  		if (!__kmem_cache_alloc_bulk(s, gfp, size,
>  					     &sheaf->objects[0])) {
> -			kfree(sheaf);
> +			free_empty_sheaf(s, sheaf);
>  			return NULL;
>  		}
> 
> @@ -5097,7 +5096,7 @@ void kmem_cache_return_sheaf(struct kmem_cache *s, gfp_t gfp,
>  	if (unlikely((sheaf->capacity != s->sheaf_capacity)
>  		     || sheaf->pfmemalloc)) {
>  		sheaf_flush_unused(s, sheaf);
> -		kfree(sheaf);
> +		free_empty_sheaf(s, sheaf);
>  		return;
>  	}
>