Re: [PATCH v2 1/3] mm/damon: move damon_hugetlb_mkold() from vaddr to ops-common

[SJ Park <sj@kernel.org>]

On Wed, 02 Sep 2026 03:14:22 +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 read-modify-write of huge PTE in damon_hugetlb_mkold clobbers hardware-updated dirty bits.
> - [High] Unaligned addresses in DAMON mkold functions cause incorrect secondary MMU aging.
> --
> 
> commit c27851bf8f97112ca3d3c8258d2ecf636424ff88
> Author: Krishna Iyer <kiyer@crusoe.ai>
> 
> mm/damon: move damon_hugetlb_mkold() from vaddr to ops-common
> 
> This commit moves the damon_hugetlb_mkold function from the virtual address
> space monitoring operations set to the common operations set. The physical
> address space monitoring operations set will now be able to use it to
> support access monitoring of hugetlb-backed memory.
> 
> > diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> > index fbda70d8ea4d0..f5fe92b825bbc 100644
> > --- a/mm/damon/ops-common.c
> > +++ b/mm/damon/ops-common.c
> [ ... ]
> > @@ -98,6 +99,35 @@ void damon_pmdp_mkold(pmd_t *pmd, struct vm_area_struct *vma, unsigned long addr
> >  #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
> >  }
> >  
> > +#ifdef CONFIG_HUGETLB_PAGE
> > +void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm,
> > +		struct vm_area_struct *vma, unsigned long addr)
> > +{
> > +	bool referenced = false;
> > +	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);
> > +	}
> 
> [Severity: Critical]
> This is a pre-existing issue, but does this non-atomic sequence overwrite
> the PTE and silently clobber concurrent hardware updates to the dirty bit?
> 
> In damon_hugetlb_mkold(), the entry is read via huge_ptep_get(), manually
> cleared of the young bit with pte_mkold(), and unconditionally written back
> with set_huge_pte_at(). A hardware update to the dirty bit occurring between
> the read and the write could be lost. If the dirty bit is overwritten with a
> stale clean copy, data could be reclaimed and lost instead of being written
> back.

I'm separatively working on this.

> 
> > +
> > +	if (mmu_notifier_clear_young(mm, addr,
> > +				     addr + huge_page_size(hstate_vma(vma))))
> 
> [Severity: High]
> This is a pre-existing issue, but could passing an unaligned addr to
> mmu_notifier_clear_young() cause incorrect secondary MMU aging?
> 
> When mmu_notifier_clear_young() receives a range that crosses page
> boundaries due to an unaligned start address, the secondary MMU (such as
> KVM) may age multiple adjacent host pages instead of just the target page.
> This would corrupt working set statistics for adjacent pages and lead to
> incorrect memory management, like reclaiming hot pages.
> 
> This unaligned addr behavior appears to originate from damon_rand() and is
> also present in damon_ptep_mkold() and damon_pmdp_mkold().

I will separatively work on this.

FYI, I think Sashiko maybe reported this kind of issue before, but I ignored it
by misunderstanding the point.  Now I understand it better and agree this is an
issue that better to be fixed.

> 
> > +		referenced = true;
> > +
> > +	if (referenced)
> > +		folio_set_young(folio);
> > +
> > +	folio_set_idle(folio);
> > +	folio_put(folio);
> > +}
> > +#endif	/* CONFIG_HUGETLB_PAGE */
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260902025700.17975-1-kiyer@crusoe.ai?part=1


Thanks,
SJ