Re: [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT

On Sun, 02 Aug 2026 16:36:30 +0000 [email protected] wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unsafe blind PTE batching skips intermediate mappings and misses per-PTE Accessed/Young bits for large folios.
> --
> 
> --- Patch [5]: [RFC PATCH v1.1 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT ---
> commit d6ffeea9e85e0fc3aa2c2575b8113904f9da4123
> Author: SJ Park <[email protected]>
> 
> mm/damon/vaddr: respect folio end for DAMOS_STAT
> 
> This commit fixes an issue in DAMOS_STAT page-level monitoring where the
> address advancement could incorrectly skip folios if a region starts in the
> middle of a large folio. It ensures the address is advanced exactly to the
> start of the next folio by calculating the correct page index.
> 
> [1] https://lore.kernel.org/[email protected]
> 
> > diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> > index 0648400b2d65b..4b0b5edf67952 100644
> > --- a/mm/damon/vaddr.c
> > +++ b/mm/damon/vaddr.c
> [ ... ]
> > @@ -844,7 +846,8 @@ static int damos_va_stat_pmd_entry(pmd_t *pmd, unsigned long addr,
> >  
> >  		if (!damos_va_filter_out(s, folio, vma, addr, pte, NULL))
> >  			*sz_filter_passed += folio_size(folio);
> > -		nr = folio_nr_pages(folio);
> > +		page_idx = folio_page_idx(folio, pte_page(ptent));
> > +		nr = folio_nr_pages(folio) - page_idx;
> 
> [Severity: High]
> This is a pre-existing issue, but does this blind PTE batching skip
> intermediate mappings and miss per-PTE Accessed/Young bits for large folios?
> 
> When damos_va_filter_out() tests the young filter, it only checks the
> accessed bit of the first pte. By unconditionally advancing pte += nr without
> aggregating the accessed bits of the remaining subpages, could this miss
> hardware accesses on PTE-mapped large folios (like mTHP) and cause DAMON to
> falsely classify actively used memory as cold?
> 
> Additionally, if userspace partially unmaps or remaps portions of this large
> folio (for example, via MADV_DONTNEED or mmap), the page table might contain
> different folios or empty entries in the skipped range. By blindly advancing
> the PTE walker based purely on the first folio's size, does this ignore those
> mixed mappings and fail to monitor the substituted folios?

Handling all the cases would be too complicated for backporting.  I will
revisit this later.

> 
> >  		s->last_applied = folio;
> >  	}
> >  	pte_unmap_unlock(start_pte, ptl);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
> 


Thanks,
SJ