Re: [PATCH RFC] mm, slab: add an optimistic __slab_try_return_freelist()

[Hao Li <hao.li@linux.dev>]

On Wed, Apr 22, 2026 at 01:55:09PM +0200, Vlastimil Babka (SUSE) wrote:
> On 4/22/26 09:09, Hao Li wrote:
> > On Tue, Apr 21, 2026 at 04:49:52PM +0200, Vlastimil Babka (SUSE) wrote:
> >> When we end up returning extraneous objects during refill to a slab
> >> where we just did a get_freelist_nofreeze(), it is likely no other CPU
> >> has freed objects to it meanwhile. We can then reattach the remainder of
> >> the freelist without having to walk the (potentially cache cold)
> >> freelist to to find its tail to connect slab->freelist to it.
> > 
> > this approach is clever!
> 
> Thanks!
> 
> > 
> > I was just brainstorming a bit here: what if we only try calling
> > slab_update_freelist without grabbing the lock or touching the partial list at
> > all?
> > Instead, we could just toss the slab right back into pc.slabs. That way,
> > we can let the downstream logic for handling "leftover slabs" take care of this
> > slab together. It could save us a whole lock/unlock pair.
> 
> Great suggestion, thanks! Indeed it should not be necessary to reattach the
> freelist and return the slab to the partial list at the same moment, AFAICS.
> Makes the code simpler. Here's a pre-v2:

Thanks for ack and working on this!

> 
> From 6f56844b79fcca5d1dd4203b879af7daa11d09e5 Mon Sep 17 00:00:00 2001
> From: "Vlastimil Babka (SUSE)" <vbabka@kernel.org>
> Date: Tue, 21 Apr 2026 16:28:01 +0200
> Subject: [PATCH RFC] mm, slab: add an optimistic __slab_try_return_freelist()
> 
> When we end up returning extraneous objects during refill to a slab
> where we just did a get_freelist_nofreeze(), it is likely no other CPU
> has freed objects to it meanwhile. We can then reattach the remainder of
> the freelist without having to walk the (potentially cache cold)
> freelist to to find its tail to connect slab->freelist to it.
> 
> Add a __slab_try_return_freelist() function that does that. As suggested
> by Hao Li, it doesn't need to also return the slab to the partial list,
> because there's code in __refill_objects_node() that already does that
> for any slabs where we don't detach the freelist.
> 
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
>  mm/slub.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 54 insertions(+), 9 deletions(-)

Looks good to me.

Reviewed-by: Hao Li <hao.li@linux.dev>

> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 35b6cd0efc3b..95e4289671b3 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -373,6 +373,8 @@ enum stat_item {
>  	SHEAF_PREFILL_OVERSIZE,	/* Allocation of oversize sheaf for prefill */
>  	SHEAF_RETURN_FAST,	/* Sheaf return reattached spare sheaf */
>  	SHEAF_RETURN_SLOW,	/* Sheaf return could not reattach spare */
> +	REFILL_RETURN_FAST,
> +	REFILL_RETURN_SLOW,
>  	NR_SLUB_STAT_ITEMS
>  };
>  
> @@ -4323,7 +4325,8 @@ static inline bool pfmemalloc_match(struct slab *slab, gfp_t gfpflags)
>   * Assumes this is performed only for caches without debugging so we
>   * don't need to worry about adding the slab to the full list.
>   */
> -static inline void *get_freelist_nofreeze(struct kmem_cache *s, struct slab *slab)
> +static inline void *get_freelist_nofreeze(struct kmem_cache *s, struct slab *slab,
> +					  unsigned int *count)
>  {
>  	struct freelist_counters old, new;
>  
> @@ -4339,6 +4342,7 @@ static inline void *get_freelist_nofreeze(struct kmem_cache *s, struct slab *sla
>  
>  	} while (!slab_update_freelist(s, slab, &old, &new, "get_freelist_nofreeze"));
>  
> +	*count = old.objects - old.inuse;
>  	return old.freelist;
>  }
>  
> @@ -5502,6 +5506,35 @@ static noinline void free_to_partial_list(
>  	}
>  }
>  
> +/*
> + * Try returning (remainder of) the freelist that we just detached from the
> + * slab.  Optimistically assume the slab is still full, so we don't need to find
> + * the tail of the detached freelist.
> + *
> + * Fail if the slab isn't full anymore due to a cocurrent free.
> + */
> +static bool __slab_try_return_freelist(struct kmem_cache *s, struct slab *slab,
> +				       void *head, int cnt)
> +{
> +	struct freelist_counters old, new;
> +
> +	old.freelist = slab->freelist;
> +	old.counters = slab->counters;
> +
> +	if (old.freelist)
> +		return false;
> +
> +	new.freelist = head;
> +	new.counters = old.counters;
> +	new.inuse -= cnt;
> +
> +	if (!slab_update_freelist(s, slab, &old, &new, "__slab_try_return_freelist"))
> +		return false;
> +
> +	stat(s, REFILL_RETURN_FAST);
> +	return true;
> +}
> +
>  /*
>   * Slow path handling. This may still be called frequently since objects
>   * have a longer lifetime than the cpu slabs in most processing loads.
> @@ -7113,34 +7146,42 @@ __refill_objects_node(struct kmem_cache *s, void **p, gfp_t gfp, unsigned int mi
>  
>  	list_for_each_entry_safe(slab, slab2, &pc.slabs, slab_list) {
>  
> +		unsigned int count;
> +
>  		list_del(&slab->slab_list);
>  
> -		object = get_freelist_nofreeze(s, slab);
> +		object = get_freelist_nofreeze(s, slab, &count);
>  
> -		while (object && refilled < max) {
> +		while (count && refilled < max) {
>  			p[refilled] = object;
>  			object = get_freepointer(s, object);
>  			maybe_wipe_obj_freeptr(s, p[refilled]);
>  
>  			refilled++;
> +			count--;
>  		}
>  
>  		/*
>  		 * Freelist had more objects than we can accommodate, we need to
> -		 * free them back. We can treat it like a detached freelist, just
> -		 * need to find the tail object.
> +		 * free them back. First we try to be optimistic and assume the
> +		 * slab is stil full since we just detached its freelist.
> +		 * Otherwise we must need to find the tail object.
>  		 */
> -		if (unlikely(object)) {
> +		if (unlikely(count)) {
>  			void *head = object;
>  			void *tail;
> -			int cnt = 0;
> +
> +			if (__slab_try_return_freelist(s, slab, head, count)) {
> +				list_add(&slab->slab_list, &pc.slabs);
> +				break;
> +			}
>  
>  			do {
>  				tail = object;
> -				cnt++;
>  				object = get_freepointer(s, object);
>  			} while (object);
> -			__slab_free(s, slab, head, tail, cnt, _RET_IP_);
> +			__slab_free(s, slab, head, tail, count, _RET_IP_);
> +			stat(s, REFILL_RETURN_SLOW);
>  		}
>  
>  		if (refilled >= max)
> @@ -9366,6 +9407,8 @@ STAT_ATTR(SHEAF_PREFILL_SLOW, sheaf_prefill_slow);
>  STAT_ATTR(SHEAF_PREFILL_OVERSIZE, sheaf_prefill_oversize);
>  STAT_ATTR(SHEAF_RETURN_FAST, sheaf_return_fast);
>  STAT_ATTR(SHEAF_RETURN_SLOW, sheaf_return_slow);
> +STAT_ATTR(REFILL_RETURN_FAST, refill_return_fast);
> +STAT_ATTR(REFILL_RETURN_SLOW, refill_return_slow);
>  #endif	/* CONFIG_SLUB_STATS */
>  
>  #ifdef CONFIG_KFENCE
> @@ -9454,6 +9497,8 @@ static const struct attribute *const slab_attrs[] = {
>  	&sheaf_prefill_oversize_attr.attr,
>  	&sheaf_return_fast_attr.attr,
>  	&sheaf_return_slow_attr.attr,
> +	&refill_return_fast_attr.attr,
> +	&refill_return_slow_attr.attr,
>  #endif
>  #ifdef CONFIG_FAILSLAB
>  	&failslab_attr.attr,
> -- 
> 2.53.0
> 
> 

-- 
Thanks,
Hao