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

On Thu, 23 Jul 2026 06:24:27 +0000 [email protected] wrote:

> 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?

Users could set the maximum size of regions with max_nr_regions parameter.  We
could revisit this if it turns out the knob is not really useful, from the real
world.

> 
> [ ... ]
> 
> [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?

Known issue.  I'm working on this.

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


Thanks,
SJ