Re: [PATCH] mm/slub: batch-detach node partial slabs

[Hao Li]

On Tue, May 26, 2026 at 04:37:46PM +0900, Harry Yoo wrote:
> 
> 
> On 5/25/26 12:22 PM, Hao Li wrote:
> > get_partial_node_bulk() used to move each selected slab from the node
> > partial list to the local pc->slabs list using a remove_partial() and
> > list_add() pair. In practice, the loop often detaches several adjacent
> > slabs, so this repeatedly manipulates list pointers while holding
> > n->list_lock, which causes unnecessary churn.
> > 
> > Instead, track contiguous runs of matching slabs and move each run with
> > list_bulk_move_tail() in one operation.
> 
> TIL list_bulk_move_tail() :D

I had to dig through list.h for ages just to find it :P

> 
> > This reduces list pointer churn> inside the lock critical section.
> 
> Similar to this, can we return all slabs in pc->slabs at once when returning
> those slabs to the list? ... I see Vlastimil removed 'nr of empty slabs'
> check in the other series already.
> 
> Now that it inserts slabs to the tail with Vlastimil's patchset, let's do a
> list_splice_tail() instead?

Great idea! then both get and put operations will be highly efficient.

> 
> > The mmap2 testcase shows a 5% improvement after applying this patch.
> > 
> > Signed-off-by: Hao Li <hao.li@xxxxxxxxx>
> > ---
> >   mm/slub.c | 22 ++++++++++++++++++----
> >   1 file changed, 18 insertions(+), 4 deletions(-)
> > 
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 04692a6f9128..180973a4a3d2 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -3775,15 +3783,21 @@ static bool get_partial_node_bulk(struct kmem_cache *s,
> >   		    && total_free + slab_free > pc->max_objects)
> >   			break;
> > -		remove_partial(n, slab);
> > -
> > -		list_add(&slab->slab_list, &pc->slabs);
> > +		if (!first)
> > +			first = slab;
> > +		last = slab;
> 
> > +		slab_clear_node_partial(slab);
> > +		n->nr_partial--;
> 
> Perhaps factor out those two statements into to a common function and call
> it in get_partial_node_bulk() and remove_partial()?

Agreed, this is very reasonable.

> >   		total_free += slab_free;
> >   		if (total_free >= pc->max_objects)
> >   			break;
> >   	}
> > +	if (first)
> > +		list_bulk_move_tail(&pc->slabs, &first->slab_list,
> > +				    &last->slab_list);
> > +
> >   	spin_unlock_irqrestore(&n->list_lock, flags);
> >   	return total_free > 0;
> >   }
> 

Thanks for the review!

-- 
Thanks,
Hao