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

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

[hu.shengming]

From: Shengming Hu <hu.shengming@zte.com.cn>

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().

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")
Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
---
 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);
-- 
2.25.1

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

[Harry Yoo]

Hi Shengming,

On 5/21/26 8:50 PM, hu.shengming@zte.com.cn wrote:
> From: Shengming Hu <hu.shengming@zte.com.cn>
> 
> 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().

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

> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> ---
>   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

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@zte.com.cn wrote:
> > From: Shengming Hu <hu.shengming@zte.com.cn>
> > 
> > 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@zte.com.cn>
> > ---
> >   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