mm/damon/ops-common: putback folios on invalid migrate nid

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Invalid address and PTE advancement after page-to-folio conversion for large folios in DAMON scanning loops.
- [High] DAMON migration causes NR_ISOLATED_ANON and NR_ISOLATED_FILE counter underflow.
--

--- Patch [1]: [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid --- commit 1f243815b710aba6f1b966b6f98b83cb7c51edeb
Author: liyouhong <[email protected]>

mm/damon/ops-common: putback folios on invalid migrate nid

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.

Put the folios back on the invalid-nid path so ignored migration requests still return them to the LRU.

Fixes 7e6c3130690a ("mm/damon/ops-common: ignore migration request to invalid nodes")

Cc <[email protected]>

Assisted-by Cursor:grok-4.5

Reviewed-by SJ Park <[email protected]>

Signed-off-by liyouhong <[email protected]>

mm/damon/ops-common.c

Quoted 4 lines

mm/damon/ops-common.c +0 -0

index e59f77eca83b2..0981a5e3af172 100644

[ ... ]

While reviewing this migration fix, I noticed a couple of pre-existing issues in the DAMON migration code:

mm/damon/vaddr.c:damos_va_migrate_pmd_entry() {
[ ... ]

	for (; addr < next; pte += nr, addr += nr * PAGE_SIZE) {
		nr = 1;
		ptent = ptep_get(pte);

		if (pte_none(ptent) || !pte_present(ptent))
			continue;
		folio = vm_normal_folio(walk->vma, addr, ptent);
		if (!folio)
			continue;
		if (damos_va_filter_out(s, folio, walk->vma, addr, pte, NULL))
			continue;
		damos_va_migrate_dests_add(folio, walk->vma, addr, dests,
				migration_lists);
		nr = folio_nr_pages(folio);
	}

[ ... ]
}

[Severity: High]
This isn't a bug introduced by this patch, but does this iteration overshoot the end of large folios?

If the current PTE corresponds to a tail page (meaning we are at an offset into the folio), adding the full folio_nr_pages() to both pte and addr seems like it will advance past the end of the folio, skipping subsequent, unrelated memory.

This same pattern appears in damos_va_stat_pmd_entry(), damon_pa_migrate(), and damon_pa_stat().

mm/damon/vaddr.c:damos_va_migrate_dests_add() {
[ ... ]
isolate:

	if (!folio_isolate_lru(folio))
		return;

	list_add(&folio->lru, &migration_lists[i]);
}

[Severity: High]
This is a pre-existing issue, but does this isolation miss incrementing the isolated VM stats?

Folios are isolated here and added to the migration list, but I don't see NR_ISOLATED_ANON or NR_ISOLATED_FILE being updated.

Later, when the core migrate_pages() code processes these folios via damon_migrate_pages():

mm/migrate.c:migrate_folio_done() {
[ ... ]

	if (likely(!page_has_movable_ops(&src->page)) && reason != MR_DEMOTION)
		mod_node_page_state(folio_pgdat(src), NR_ISOLATED_ANON +
				    folio_is_file_lru(src), -folio_nr_pages(src));

[ ... ]
}

Will this decrement cause the node page state counters to underflow?

Since the counters weren't incremented originally during DAMON's isolation, it looks like a successful migration will drop the isolated stat and wrap it around. This could hang memory reclaim since too_many_isolated() might perpetually return true.

This also seems to apply to damon_pa_migrate().

Signature · 2 lines

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

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.

Put the folios back on the invalid-nid path so ignored migration requests still return them to the LRU.