Re: [PATCH 3/6] mm/damon/paddr: support hugetlb folios in access monitoring

[SJ Park <sj@kernel.org>]

On Sun, 30 Aug 2026 05:29:35 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Non-atomic huge PTE modification in damon_hugetlb_mkold() causes a TOCTOU race with hardware page table walkers, silently dropping dirty bits.
> - [High] Incorrect physical address advancement when starting from a tail page of a large folio.
> --
> 
> commit fa7cc985c2891991281afce141ed40264af354fe
> Author: Krishna Iyer <kiyer@crusoe.ai>
> 
>     mm/damon/paddr: support hugetlb folios in access monitoring
>     
>     DAMON's physical address space monitoring is blind to hugetlb-backed
>     memory. Every access check starts at damon_get_folio(), which rejects
>     folios that are not on the LRU lists. This commit adds
>     damon_get_folio_incl_hugetlb() to additionally accept hugetlb folios.
> 
> [ ... ]
> 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > index 62004206ca310..ece101d346844 100644
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> 
> [ ... ]
> 
> [Severity: Critical]
> This is a pre-existing issue, but does this non-atomic huge PTE modification
> drop hardware dirty bits?
> 
> In damon_hugetlb_mkold() in mm/damon/ops-common.c:
> 
> 	pte_t entry = huge_ptep_get(mm, addr, pte);
> 	struct folio *folio = pfn_folio(pte_pfn(entry));
> 	unsigned long psize = huge_page_size(hstate_vma(vma));
> 
> 	folio_get(folio);
> 
> 	if (pte_young(entry)) {
> 		referenced = true;
> 		entry = pte_mkold(entry);
> 		set_huge_pte_at(mm, addr, pte, entry, psize);
> 	}
> 
> If a hardware page table walker sets the dirty bit concurrently between the
> read and the write, does the non-atomic writeback via set_huge_pte_at()
> silently overwrite and clear the dirty bit?

I'm planning to work on fixing this.

> 
> [Severity: High]
> This isn't a bug introduced by this patch, but does this physical address
> advancement skip memory pages?
> 
> In damon_pa_pageout() in mm/damon/paddr.c:
> 
> 	addr = damon_pa_phys_addr(r->ar.start, addr_unit);
> 	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
> 		folio = damon_get_folio(PHYS_PFN(addr));
> ...
> put_folio:
> 		addr += folio_size(folio);
> 		folio_put(folio);
> 	}
> 
> If the region's start address is not naturally aligned to the folio boundary
> and lands on a tail page of a large folio, does adding the full folio size
> overshoot the start of the next folio?
> 
> The same pattern appears in damon_pa_stat() where it also over-accounts the
> filter size:
> 
> 		if (damos_pa_filter_out(s, folio))
> 			goto put_folio;
> 		else
> 			*sz_filter_passed += folio_size(folio) / addr_unit;
> ...
> put_folio:
> 		addr += folio_size(folio);
> 		folio_put(folio);

Known issue.  Fix is cooking:
https://lore.kernel.org/20260802162631.90304-1-sj@kernel.org

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260830051407.50008-1-kiyer@crusoe.ai?part=3


Thanks,
SJ