Re: [PATCH] mm/slub: free returned pfmemalloc sheaves via free_empty_sheaf()

[hu.shengming]

Harry wrote:
> Hi Shengming,

Hi Harry,

> On 5/21/26 8:50 PM, hu.shengming@xxxxxxxxxx wrote:
> > From: Shengming Hu <hu.shengming@xxxxxxxxxx>
> > 
> > Regular sized sheaves are allocated through alloc_empty_sheaf(), which
> > sets __GFP_NO_OBJ_EXT. free_empty_sheaf() is the matching free helper
> > for such sheaves. It marks kmalloc sheaves with an empty codetag before
> > kfree(), avoiding the alloc_tag_sub() warning for intentionally missing
> > object extensions.
>  >
> > kmem_cache_return_sheaf() currently handles returned pfmemalloc sheaves
> > in the same branch as oversize sheaves and uses kfree(). That is right
> > for oversize sheaves, which are allocated directly with kzalloc_flex(),
> > but it misses the matching free path for regular sized pfmemalloc sheaves
> > that came from alloc_empty_sheaf().
> 
> I think it'd be better (in terms of readability) to teach 
> alloc_empty_sheaf() how to allocate and initialize an oversized empty 
> sheaf, and always free empty sheaves with free_empty_sheaf().
> 

Thanks, that makes sense.

I will move the existing capacity and pfmemalloc initialization into
__alloc_empty_sheaf() and make oversized prefilled sheaves use that
helper too. This way, oversized prefilled sheaves can use free_empty_sheaf()
instead of kfree().

> > Split the two cases. Keep plain kfree() for capacity-mismatched oversize
> > sheaves, and use free_empty_sheaf() after flushing returned pfmemalloc
> > regular sheaves.
> > 
> > Fixes: <1ce20c2> ("slab: handle pfmemalloc slabs properly with sheaves")
> 
> When referring to a commit, the commit id should contain at least 12 
> characters to avoid collisions (per the process document).
> 
> ....but probably Fixes: is not needed at all, because currently no user 
> uses kmalloc caches to allocate prefilled sheaves.
> 

Oh, yes. I will also remove the Fixes tag as suggested.

> > Signed-off-by: Shengming Hu <hu.shengming@xxxxxxxxxx>
> > ---
> >   mm/slub.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 04692a6f9128..a6b720fa79ad 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -5094,13 +5094,18 @@ void kmem_cache_return_sheaf(struct kmem_cache *s, gfp_t gfp,
> >       struct slub_percpu_sheaves *pcs;
> >       struct node_barn *barn;
> > 
> > -    if (unlikely((sheaf->capacity != s->sheaf_capacity)
> > -             || sheaf->pfmemalloc)) {
> > +    if (unlikely(sheaf->capacity != s->sheaf_capacity)) {
> >           sheaf_flush_unused(s, sheaf);
> >           kfree(sheaf);
> >           return;
> >       }
> > 
> > +    if (unlikely(sheaf->pfmemalloc)) {
> > +        sheaf_flush_unused(s, sheaf);
> > +        free_empty_sheaf(s, sheaf);
> > +        return;
> > +    }
> > +
> >       local_lock(&s->cpu_sheaves->lock);
> >       pcs = this_cpu_ptr(s->cpu_sheaves);
> >       barn = get_barn(s);
> 
> -- 
> Cheers,
> Harry / Hyeonggon

--
With Best Regards,
Shengming