Re: [PATCH v3] mm/damon/ops-common: use a page-aligned address in damon_ptep_mkold()

[Baolin Wang]

On 9/2/26 8:11 AM, SJ Park wrote:
> On Tue, 1 Sep 2026 13:10:01 -0700 Nathan Gao <zcgao@xxxxxxxxxx> wrote:
>
> > __damon_va_prepare_access_check() picks a random byte address within the
> > region and stores it in r->sampling_addr. damon_va_mkold() passes it into
> > a page table walk, which hands it to damon_ptep_mkold() as the address of
> > the page to sample:
> >
> >    damon_va_mkold(mm, r->sampling_addr)
> >      damon_va_walk_page_range(mm, addr, addr + 1)
> >        damon_mkold_pmd_entry()
> >          damon_ptep_mkold(pte, vma, addr)
> >            ptep_test_and_clear_young(vma, addr, pte)
> >            mmu_notifier_clear_young(mm, addr, addr + PAGE_SIZE)
> >
> > For arm64, before commit 6f0e1142173a ("arm64: mm: support batch
> > clearing of the young flag for large folios"), the contpte helper walked
> > exactly CONT_PTES entries from the aligned-down page table pointer and
> > used @addr only to pass down to each entry, so an unaligned value was
> > harmless:
> >
> >          ptep = contpte_align_down(ptep);
> >          addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
> >          for (i = 0; i < CONT_PTES; i++, ptep++, addr += PAGE_SIZE)
> >
> > Now the range to walk is derived from @addr instead: end = addr +
> > nr * PAGE_SIZE, rounded up to CONT_PTE_SIZE. For a sample in the last
> > page of a contpte block, the sub-page offset puts end just past the
> > block boundary, so the round-up lands a whole block further and the
> > walk clears PTE_AF in CONT_PTES entries beyond the sampled block. For
> > the last block in a page table page, those entries are past the end of
> > that page, so the walk writes into the page that follows.
>
> I just wanted to call out again that I'm wondering if we could restore the
> unaligned address support in the helper.  E.g., as a very dirty hack that I can
> imagine off the top of my head,
>
> '''
> --- a/arch/arm64/mm/contpte.c
> +++ b/arch/arm64/mm/contpte.c
> @@ -30,6 +30,7 @@ static inline pte_t *contpte_align_addr_ptep(unsigned long *start,
>                                               unsigned long *end, pte_t *ptep,
>                                               unsigned int nr)
>   {
> +       *start = PAGE_ALIGN_DOWN(*start);
>          /*
>           * Note: caller must ensure these nr PTEs are consecutive (present)
>           * PTEs that map consecutive pages of the same large folio within a
> '''
>
> I and Nathan have no strong clue, so we are looking for Baolin and others'
> opinion.
>
> While waiting for the opinions, I and Nathan agree we should stop bleeding with
> a pinpoint hotfix change in DAMON.

Thanks for the reporting.

IMO, we could let the arch low-level functions handle the alignment of 
addr, but that would also require changing functions like 
contpte_clear_young_dirty_ptes(), contpte_set_ptes() and so on, which 
would cause a lot of churn? (they also assume that addr is page aligned).

Since the arch low-level functions basically assume that addr is 
page-size aligned, and the addr handling in the mm core is also mostly 
page-size aligned, I think the caller guaranteeing that addr is 
page-size aligned is a reasonable fix. So:

Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>