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

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