Re: [PATCH v2] mm: migrate: requeue destination folio on deferred split queue

[Wei Yang]

On Mon, Jun 22, 2026 at 11:16:39AM +0200, David Hildenbrand (Arm) wrote:
>On 6/20/26 09:27, Wei Yang wrote:
>> On Tue, Mar 10, 2026 at 03:54:19AM -0700, Usama Arif wrote:
>>> During folio migration, __folio_migrate_mapping() removes the source
>>> folio from the deferred split queue, but the destination folio is never
>>> re-queued.  This causes underutilized THPs to escape the shrinker after
>>> NUMA migration, since they silently drop off the deferred split list.
>>>
>>> Fix this by recording whether the source folio was on the deferred split
>>> queue and its partially mapped state before move_to_new_folio() unqueues
>>> it, and re-queuing the destination folio after a successful migration if
>>> it was.
>>>
>>> By the time migrate_folio_move() runs, partially mapped folios without a
>>> pin have already been split by migrate_pages_batch().  So only two cases
>>> remain on the deferred list at this point:
>>>  1. Partially mapped folios with a pin (split failed).
>>>  2. Fully mapped but potentially underused folios.
>>> The recorded partially_mapped state is forwarded to deferred_split_folio()
>>> so that the destination folio is correctly re-queued in both cases.
>>>
>>> Reported-by: Johannes Weiner <hannes@xxxxxxxxxxx>
>>> Fixes: dafff3f4c850 ("mm: split underused THPs")
>>> Signed-off-by: Usama Arif <usama.arif@xxxxxxxxx>
>>> ---
>>> v1 -> v2:
>>> - record whether source folio was on the deferred split queue before
>>>  move_to_folio() (David)
>>> - record partially mapped state and update commit message (Zi)
>>> ---
>>> mm/migrate.c | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/mm/migrate.c b/mm/migrate.c
>>> index ece77ccb2ec0..61013d258eb4 100644
>>> --- a/mm/migrate.c
>>> +++ b/mm/migrate.c
>>> @@ -1360,6 +1360,8 @@ static int migrate_folio_move(free_folio_t put_new_folio, unsigned long private,
>>> 	int rc;
>>> 	int old_page_state = 0;
>>> 	struct anon_vma *anon_vma = NULL;
>>> +	bool src_deferred_split = false;
>>> +	bool src_partially_mapped = false;
>>> 	struct list_head *prev;
>>>
>>> 	__migrate_folio_extract(dst, &old_page_state, &anon_vma);
>>> @@ -1373,6 +1375,12 @@ static int migrate_folio_move(free_folio_t put_new_folio, unsigned long private,
>>> 		goto out_unlock_both;
>>> 	}
>>>
>>> +	if (folio_test_large(src) && folio_test_large_rmappable(src) &&
>>> +	    !data_race(list_empty(&src->_deferred_list))) {
>>> +		src_deferred_split = true;
>>> +		src_partially_mapped = folio_test_partially_mapped(src);
>>> +	}
>> 
>> Hi, Usama
>> 
>> I am afraid there maybe a race between migration and defer_split.
>> 
>>                 A                              B
>>   migrate_pages_batch                   deferred_split_scan
>>     migrate_folio_unmap                   list_del_init(&folio->_deferred_list)
>>       folio_lock/folio_trylock
>> 
>>     migrate_folios_move
>>       migrate_folio_move
>>         list_empty(&src->_deferred_list)
>>                                           folio_trylock()
>>                                           requeue:
>> 
>> In case list_empty() check happens after folio removed from defer_list but
>> before requeued, we will miss this folio.
>
>deferred_split_isolate() would grab a reference through folio_try_get().
>
>How can we migrate a folio with a raised refcount?
>

Thanks, I missed expected_refcount check in __migrate_folio().

>-- 
>Cheers,
>
>David

-- 
Wei Yang
Help you, Help me