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

[hu.shengming]

Hao wrote:
> On Mon, May 25, 2026 at 09:10:00PM +0800, hu.shengming@xxxxxxxxxx wrote:
> > From: Shengming Hu <hu.shengming@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> > ---
> > 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/
> > 
> > ---
> >  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;
> > 
> >      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);
> > -        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
> > -         */
> 
> just a bit curious, why did you decide to delete these comments?
> 

Sorry about that.

I removed the comment while moving the oversized sheaf allocation to
__alloc_empty_sheaf(), because the local cache/capacity initialization was
removed from this path. However, looking at it again, the comment is still
worth.

I will restore it in v4.

Thanks a lot.

> >          if (!__kmem_cache_alloc_bulk(s, gfp, size,
> >                           &sheaf->objects[0])) {
> > -            kfree(sheaf);
> > +            free_empty_sheaf(s, sheaf);
> >              return NULL;
> >          }
> > 
> > @@ -5097,7 +5092,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;
> >      }
> > 
> > -- 
> > 2.25.1

--
With Best Regards,
Shengming