Re: [PATCH] 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] Incorrect Loop Advancement / Bounds Oversight (PFN advancement bug)
- [High] Missing yield in unbounded list putback loop
--

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

This commit extracts the LRU putback logic into a helper function and
uses it to return folios to the LRU when damon_migrate_pages() is called
with an invalid target_nid. This prevents folios from remaining isolated
from the LRU permanently.

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index e59f77eca83b2..1286fe54b0d9d 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -330,12 +330,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);
> +	}
> +}

[Severity: High]
This is a pre-existing issue, but can this tight loop cause CPU soft lockups?

If a DAMON scheme isolates a massive region that fails to migrate, or is
given an invalid migration NID, the isolated folio list could scale to
millions of folios. Because this loop takes the LRU lock for each folio
without a yield point, could it monopolize the CPU and cause severe latency
spikes? Should there be a cond_resched() in this loop?

[ ... ]

[Severity: High]
This is a pre-existing issue, but do these unconditionally sized advancements
elsewhere in the DAMON codebase risk skipping uninspected memory regions?

In mm/damon/paddr.c:damon_pa_pageout():

    addr += folio_size(folio);

And similarly in mm/damon/vaddr.c:damos_va_migrate_pmd_entry() and
damos_va_stat_pmd_entry():

    nr = folio_nr_pages(folio);

When a DAMON region starts in the middle of a large folio (a tail page), or
a PTE walk encounters a partially mapped large folio, advancing the pointer
by the total folio size or total number of pages overshoots the bounds. Does
this silently skip adjacent, unrelated PTEs and physical addresses?

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