Re: [PATCH] mm/lruvec: preemptively free dead folios during lru_add drain

[JP Kobryn (Meta)]

On 4/23/26 10:15 AM, Matthew Wilcox wrote:
> On Thu, Apr 23, 2026 at 09:43:07AM -0700, JP Kobryn (Meta) wrote:
> > Of all observable lruvec lock contention in our fleet, we find that ~24%
> > occurs when dead folios are present in lru_add batches at drain time. This
> > is wasteful in the sense that the folio is added to the LRU just to be
> > immediately removed via folios_put_refs(), incurring two unnecessary lock
> > acquisitions.
>
> Well, this is a lovely patch with no obvious downsides.  Nicely done.

Thanks for the kind words and review :)

[...]
> > diff --git a/mm/swap.c b/mm/swap.c
> > index 5cc44f0de9877..71607b0ce3d18 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -160,13 +160,36 @@ static void folio_batch_move_lru(struct folio_batch *fbatch, move_fn_t move_fn)
> >   	int i;
> >   	struct lruvec *lruvec = NULL;
> >   	unsigned long flags = 0;
> > +	struct folio_batch free_fbatch;
> > +	bool is_lru_add = (move_fn == lru_add);
> > +
> > +	/*
> > +	 * If we're adding to the LRU, preemptively filter dead folios. Use
> > +	 * this dedicated folio batch for temp storage and deferred cleanup.
> > +	 */
> > +	if (is_lru_add)
> > +		folio_batch_init(&free_fbatch);
> >   
> >   	for (i = 0; i < folio_batch_count(fbatch); i++) {
> >   		struct folio *folio = fbatch->folios[i];
> >   
> >   		/* block memcg migration while the folio moves between lru */
> > -		if (move_fn != lru_add && !folio_test_clear_lru(folio))
> > +		if (!is_lru_add && !folio_test_clear_lru(folio))
> > +			continue;
> > +
> > +		/*
> > +		 * Filter dead folios by moving them from the add batch to the temp
> > +		 * batch for freeing after this loop.
> > +		 *
> > +		 * Since the folio may be part of a huge page, unqueue from
> > +		 * deferred split list to avoid a dangling list entry.
> > +		 */
> > +		if (is_lru_add && folio_ref_freeze(folio, 1)) {
> > +			folio_unqueue_deferred_split(folio);
>
> Would it be better to do this outside the lru lock; it's just that we
> don't have a convenient batched version to do it?  It seems like
> there are a few places that could use a batched version in vmscan.c and
> swap.c.  Not that I think we should hold up this patch to investigate
> that micro-optimisation!  Just something you couldlook at as a
> follow-up.

Good call. I'll leave this patch as-is (unless other feedback), then
pursue the batched version of unqueuing the split in a separate
follow-up patch.

> Reviewed-by: Matthew Wilcox (Oracle) <willy@xxxxxxxxxxxxx>