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

[hu.shengming]

Harry worte:
> On 5/25/26 10:10 PM, 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>
> > ---
> >   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().
> 

Thanks for the review.

That makes sense. I will keep the capacity initialization in the
oversized prefill path.

> >       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 */
> 

Yes, I will update the stat comments and mention this in the commit message.

> > -        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?
> 

The pfmemalloc comment is still useful as well. I dropped it too
aggressively, so I will restore it in v4.

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

--
With Best Regards,
Shengming