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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 9/2/26 8:11 AM, SJ Park wrote:
> On Tue, 1 Sep 2026 13:10:01 -0700 Nathan Gao <zcgao@amazon.com> 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@linux.alibaba.com>