Re:Re: [PATCH] mm/damon/ops-common: putback folios on invalid migrate nid

Thanks for the review.


At 2026-07-23 21:46:22, "SJ Park" <[email protected]> wrote:
>On Thu, 23 Jul 2026 14:12:46 +0800 [email protected] wrote:
>
>> From: liyouhong <[email protected]>
>> 
>> damon_pa_migrate() and damos_va_migrate() isolate folios into a local list
>> and then call damon_migrate_pages().  When target_nid is invalid (including
>> the scheme default NUMA_NO_NODE / -1), damon_migrate_pages() returns early
>> without putting the folios back to the LRU.
>> 
>> Callers then discard the list head while those folios remain isolated with
>> an extra reference taken by folio_isolate_lru().  The pages stay off the
>> LRU for as long as the mapping exists (anon active+inactive counts drop
>> while RSS does not), and the leftover references can pin the pages after
>> the mapping is gone.
>
>Nice catch!
>
>Sounds like this is a very bad bug.  Users can trigger it (though it is
>arguably weird or poor setup at least) and the consequence is quote bad:
>visible but silent memory exhaustion that cannot be restored without reboot.  I
>believe this deserves the hot fixes fast track.  Let me know if I'm missing
>something.


>






Users can hit it by creating a migrate_{hot,cold} scheme without changing
target_nid (the sysfs default is NUMA_NO_NODE / -1), or by pointing it at a
non-memory node.  Each apply cycle isolates folios, then
damon_migrate_pages() returns without putback and the callers discard the
list heads.  The pages leave the LRU while the process RSS stays up; after
the mapping is gone, the leftover isolate refs keep them pinned until
reboot.  I do not think you are missing anything.


>> 
>> Factor the existing putback loop into damon_putback_folio_list() and use it
>> on the invalid-nid path as well, so ignored migration requests still return
>> folios to the LRU.
>
>As this is a hotfix, I'd prefer skipping putback loop facotring out part, for
>simplicity of backporting.
>
>> 
>> Fixes: 7e6c3130690a ("mm/damon/ops-common: ignore migration request to invalid nodes")
>
>Let's Cc: stable@.
>
>> Signed-off-by: liyouhong <[email protected]>
>> ---
>>  mm/damon/ops-common.c | 22 +++++++++++++++-------
>>  1 file changed, 15 insertions(+), 7 deletions(-)
>> 
>> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
>> index d1842e2b00ef..9a1e8aec5444 100644
>> --- a/mm/damon/ops-common.c
>> +++ b/mm/damon/ops-common.c
>> @@ -331,12 +331,22 @@ static unsigned int __damon_migrate_folio_list(
>>  	return nr_succeeded;
>>  }
>>  
>> +static void damon_putback_folio_list(struct list_head *folio_list)
>> +{
>> +	struct folio *folio;
>> +
>> +	while (!list_empty(folio_list)) {
>> +		folio = lru_to_folio(folio_list);
>> +		list_del(&folio->lru);
>> +		folio_putback_lru(folio);
>> +	}
>> +}
>> +
>>  static unsigned int damon_migrate_folio_list(struct list_head *folio_list,
>>  						struct pglist_data *pgdat,
>>  						int target_nid)
>>  {
>>  	unsigned int nr_migrated = 0;
>> -	struct folio *folio;
>>  	LIST_HEAD(ret_folios);
>>  	LIST_HEAD(migrate_folios);
>>  
>> @@ -374,11 +384,7 @@ static unsigned int damon_migrate_folio_list(struct list_head *folio_list,
>>  
>>  	list_splice(&ret_folios, folio_list);
>>  
>> -	while (!list_empty(folio_list)) {
>> -		folio = lru_to_folio(folio_list);
>> -		list_del(&folio->lru);
>> -		folio_putback_lru(folio);
>> -	}
>> +	damon_putback_folio_list(folio_list);
>
>I agree this is good cleanup.  For simple porting to stable@ series, however,
>let's not touch this function.
>
>>  
>>  	return nr_migrated;
>>  }
>> @@ -394,8 +400,10 @@ unsigned long damon_migrate_pages(struct list_head *folio_list, int target_nid)
>>  		return nr_migrated;
>>  
>>  	if (target_nid < 0 || target_nid >= MAX_NUMNODES ||
>> -			!node_state(target_nid, N_MEMORY))
>> +	    !node_state(target_nid, N_MEMORY)) {
>
>Please keep the original indentation.
>
>> +		damon_putback_folio_list(folio_list);
>>  		return nr_migrated;
>> +	}
>
>Let's open-code the putback here.  We could factor out that later.








v2 follows your suggestions: drop the putback helper so the change is
easier to backport, open-code the putback only on the invalid-nid path,
keep the original indentation, and Cc stable@.




Best regards,
liyouhong