DAMON development mailing list
 help / color / mirror / Atom feed
* [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid
@ 2026-07-26  1:48 dayou5941
  2026-07-26  2:00 ` sashiko-bot
  2026-07-26 16:50 ` SJ Park
  0 siblings, 2 replies; 4+ messages in thread
From: dayou5941 @ 2026-07-26  1:48 UTC (permalink / raw)
  To: sj, akpm; +Cc: damon, linux-mm, linux-kernel, joshua.hahnjy, liyouhong, stable

From: liyouhong <liyouhong@kylinos.cn>

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: <stable@vger.kernel.org>
Assisted-by: Cursor:grok-4.5
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: liyouhong <liyouhong@kylinos.cn>

---
v3:
- Drop the 2/2 cleanup patch as suggested; keep open-coded putback only

v2:
- Drop the putback helper for easier stable backport
- Open-code putback only on the invalid-nid path
- Keep the original indentation and Cc stable@
- Add a separate cleanup as 2/2 (factor out damon_putback_folio_list)

---
 mm/damon/ops-common.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
index d1842e2b00ef..f5ded45fabd1 100644
--- a/mm/damon/ops-common.c
+++ b/mm/damon/ops-common.c
@@ -394,8 +394,15 @@ 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)) {
+		while (!list_empty(folio_list)) {
+			struct folio *folio = lru_to_folio(folio_list);
+
+			list_del(&folio->lru);
+			folio_putback_lru(folio);
+		}
 		return nr_migrated;
+	}
 
 	noreclaim_flag = memalloc_noreclaim_save();
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid
  2026-07-26  1:48 [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid dayou5941
@ 2026-07-26  2:00 ` sashiko-bot
  2026-07-26 16:43   ` SJ Park
  2026-07-26 16:50 ` SJ Park
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-07-26  2:00 UTC (permalink / raw)
  To: dayou5941; +Cc: damon

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 <liyouhong@kylinos.cn>

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: <stable@vger.kernel.org>
Assisted-by: Cursor:grok-4.5
Reviewed-by: SJ Park <sj@kernel.org>
Signed-off-by: liyouhong <liyouhong@kylinos.cn>

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index e59f77eca83b2..0981a5e3af172 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
[ ... ]

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().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726014815.1280757-1-dayou5941@163.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid
  2026-07-26  2:00 ` sashiko-bot
@ 2026-07-26 16:43   ` SJ Park
  0 siblings, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-07-26 16:43 UTC (permalink / raw)
  To: sashiko-bot; +Cc: SJ Park, dayou5941, damon

On Sun, 26 Jul 2026 02:00:34 +0000 sashiko-bot@kernel.org wrote:

> 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 <liyouhong@kylinos.cn>
> 
> 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: <stable@vger.kernel.org>
> Assisted-by: Cursor:grok-4.5
> Reviewed-by: SJ Park <sj@kernel.org>
> Signed-off-by: liyouhong <liyouhong@kylinos.cn>
> 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > index e59f77eca83b2..0981a5e3af172 100644
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> [ ... ]
> 
> 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().

Known issue.  I'm working on it.

> 
> 
> 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().

Good catch.  Sounds correct.  No blocker for this patch.  I will work on this.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260726014815.1280757-1-dayou5941@163.com?part=1


Thanks,
SJ

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid
  2026-07-26  1:48 [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid dayou5941
  2026-07-26  2:00 ` sashiko-bot
@ 2026-07-26 16:50 ` SJ Park
  1 sibling, 0 replies; 4+ messages in thread
From: SJ Park @ 2026-07-26 16:50 UTC (permalink / raw)
  To: dayou5941
  Cc: SJ Park, akpm, damon, linux-mm, linux-kernel, joshua.hahnjy,
	liyouhong, stable

On Sun, 26 Jul 2026 09:48:15 +0800 dayou5941@163.com wrote:

> From: liyouhong <liyouhong@kylinos.cn>
> 
> 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.

Looks good to me, thank you for catching and fixing this, liyouhong!

Andrew, I think this deserves the hotfix fast track.  The bug can cause memory
pressure, and can be reliably reproduced by the user.  The user should already
have a sysfs write permission, though.  Let me know if you think differently.

This patch is applied to damon/next [1] tree.  If this patch is not added to
mm.git in short term (~1 week?), I will ask mm.git maintainer (Andrew Morton)
to pick this.  So, no action from your side is needed for now.  If it seems I
also forgot doing that or you cannot wait for my action, please feel free to
directly ask that to Andrew.

[1] https://origin.kernel.org/doc/html/latest/mm/damon/maintainer-profile.html#scm-trees


Thanks,
SJ

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-26 16:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-26  1:48 [PATCH v3] mm/damon/ops-common: putback folios on invalid migrate nid dayou5941
2026-07-26  2:00 ` sashiko-bot
2026-07-26 16:43   ` SJ Park
2026-07-26 16:50 ` SJ Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox