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

["JP Kobryn (Meta)" <jp.kobryn@linux.dev>]

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@infradead.org>