Re: [PATCH] mm/slab: take n->list_lock for the list_add() in __refill_objects_node()

[Harry Yoo]

On Wed, Sep 02, 2026 at 08:24:27PM +0900, Hyunwoo Kim wrote:
> On Wed, Sep 02, 2026 at 12:20:34PM +0100, Harry Yoo wrote:
> > On Mon, Aug 31, 2026 at 02:55:06PM +0200, Vlastimil Babka (SUSE) wrote:
> > > On 8/30/26 16:35, Hao Li wrote:
> > > > On Sun, Aug 30, 2026 at 12:45:17PM +0000, Harry Yoo wrote:
> > > >> On Sun, Aug 30, 2026 at 04:25:45PM +0900, Hyunwoo Kim wrote:
> > > > Since introducing a new variable seems unavoidable, what if we temporarily
> > > > stash this slab in a pointer like below, and then add it to pc.slabs once we
> > > > acquire the lock.
> > > > 
> > > > struct slab *leftover_slab = NULL;
> > > > 
> > > > ...
> > > > ...
> > > > 	if (__slab_try_return_freelist(s, slab, head, count)) {
> > > > 		leftover_slab = slab;
> > > > 		break;
> > > > 	}
> > > > 
> > > > ...
> > > > ...
> > > > if (!list_empty(&pc.slabs)) {
> > > > 	spin_lock_irqsave(&n->list_lock, flags);
> > > > 
> > > > 	if (leftover_slab)
> > > > 		list_add(&leftover_slab->slab_list, &pc.slabs);
> > > > 	...
> > > > 	...
> > > > }
> > > > 
> > > > PS: If I recall correctly, Vlastimil's initial patch was actually fine. It was
> > > > my suggestion to save an extra lock/unlock pair that accidentally led to this
> > > > trap...
> > > 
> > > Ah, thanks for the reminder. This [1] was the original attempt.
> > > 
> > > [1]
> > > https://lore.kernel.org/all/20260421-b4-refill-optimistic-return-v1-1-24f0bfc1acff@xxxxxxxxxx/
> > > 
> > > I wonder if the fix should be to return to that approach and just have
> > > __slab_try_return_freelist() handle the list_lock. The code would be simpler
> > > with not "bool locked".
> > > 
> > > It should be really very rare that we would end up returning a partial list
> > > and also have additional slabs to return on pc.slabs? So I think there would
> > > be no noticeable performance downside to the simpler code potentially ending
> > > up taking the list_lock twice instead of once.
> > 
> > Agreed that it should be rare and not worth the complexity unless
> > we have data to support that.
> > 
> > Hyunwoo, would you please adjust the feedback and post v2?
> 
> If you don't mind, could the maintainers handle this patch on your side? 
> I'd then move to Reported-by:.

I'll post v2 with your Reported-by: then.
Thanks!
 
> Best regards,
> Hyunwoo Kim

-- 
Cheers,
Harry / Hyeonggon