[PATCH] mm/damon: use a page-aligned sampling address

[PATCH] mm/damon: use a page-aligned sampling address

[Nathan Gao]

__damon_va_prepare_access_check() picks a random byte address within the
region and stores it in r->sampling_addr. There are two users of
r->sampling_addr in vaddr.c that pass it into a page table walk, and
both use it as the address of a page.

  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)

  damon_va_young(mm, r->sampling_addr, &folio_sz)
    damon_va_walk_page_range(mm, addr, addr + 1)
      damon_young_pmd_entry()
        ptep_get(pte)
        mmu_notifier_test_young(walk->mm, addr)

test_and_clear_young_ptes(), which backs ptep_test_and_clear_young() on
arm64, documents @addr as "Address the first page is mapped at".

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. Seen
on an arm64 guest running the DAMON selftests as random slab and page
table corruption.

Align the sampled address down to a page boundary. It is the address of
the page to sample, so this matches its intended meaning and fixes both
users in vaddr.c.

Fixes: 3f49584b262c ("mm/damon: implement primitives for the virtual memory address spaces")
Cc: stable@vger.kernel.org
Signed-off-by: Nathan Gao <zcgao@amazon.com>
---
 mm/damon/vaddr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index 2c1c1952c008d..e26e426a56af2 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -360,7 +360,8 @@ static void __damon_va_prepare_access_check(struct mm_struct *mm,
 					struct damon_region *r,
 					struct damon_ctx *ctx)
 {
-	r->sampling_addr = damon_rand(ctx, r->ar.start, r->ar.end);
+	r->sampling_addr = PAGE_ALIGN_DOWN(damon_rand(ctx, r->ar.start,
+						      r->ar.end));
 
 	damon_va_mkold(mm, r->sampling_addr);
 }
-- 
2.50.1

Re: [PATCH] mm/damon: use a page-aligned sampling address

[SJ Park]

Hello Nathan,

On Thu, 27 Aug 2026 12:38:21 -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. There are two users of
> r->sampling_addr in vaddr.c that pass it into a page table walk, and
> both use it as the address of a page.
> 
>   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)
> 
>   damon_va_young(mm, r->sampling_addr, &folio_sz)
>     damon_va_walk_page_range(mm, addr, addr + 1)
>       damon_young_pmd_entry()
>         ptep_get(pte)
>         mmu_notifier_test_young(walk->mm, addr)
> 
> test_and_clear_young_ptes(), which backs ptep_test_and_clear_young() on
> arm64, documents @addr as "Address the first page is mapped at".
> 
> For arm64, before commit 6f0e1142173a ("arm64: mm: support batch
> clearing of the young flag for large folios"),

The @addr documentation is also introduced by this commit.  This commit is
authored at 2026-02-09.

> 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)

So, there was no issue before the commit.

> 
> 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. Seen
> on an arm64 guest running the DAMON selftests as random slab and page
> table corruption.

Thank you for sharing the finding with us!

> 
> Align the sampled address down to a page boundary. It is the address of
> the page to sample, so this matches its intended meaning and fixes both
> users in vaddr.c.

This indeed sounds like can fix the issue to me.  However, was it a clear rule
that we should pass only contepte-aligned addrss to
ptep_test_and_clear_young()?  And is DAMON the only ptep_test_and_clear_young()
caller that is mistakenly passing the unaligned address?

If not, it might make sense to make contpte_test_and_clear_young_ptes() support
unaligned adress again in my opinion.  May I ask your opinion, Baolin?

> 
> Fixes: 3f49584b262c ("mm/damon: implement primitives for the virtual memory address spaces")

I think 6f0e1142173a ("arm64: mm: support batch clearing of the young flag for
large folios") would be mroe correct 'Fixes:', if there was no issue before the
commit.

> Cc: s6f0e1142173a ("arm64: mm: support batch
> clearing of the young flag for large folios"),table@vger.kernel.org
> Signed-off-by: Nathan Gao <zcgao@amazon.com>
> ---
>  mm/damon/vaddr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
> index 2c1c1952c008d..e26e426a56af2 100644
> --- a/mm/damon/vaddr.c
> +++ b/mm/damon/vaddr.c
> @@ -360,7 +360,8 @@ static void __damon_va_prepare_access_check(struct mm_struct *mm,
>  					struct damon_region *r,
>  					struct damon_ctx *ctx)
>  {
> -	r->sampling_addr = damon_rand(ctx, r->ar.start, r->ar.end);
> +	r->sampling_addr = PAGE_ALIGN_DOWN(damon_rand(ctx, r->ar.start,
> +						      r->ar.end));

If we need to have the fix in DAMON, this kind of change would be needed.

However, what happens if the address is backed by large folios?

Before the commit 6f0e1142173a, also, it was aligning to CONT_PTE_SIZE.  Should
we do same?

Also, I think we should pass aligned address to only the functions that
require alignement.  Making the alignment to the sampling address in general
sounds too much to me.  Particularly, we are working on supporting new page
access check primitives other than PTE Accessed bit, like AMd IBS.  In the
case, we might support <PAGE_SIZE granularity monitoring.  Aligning sampling
address in general will make it more complicated.

>  
>  	damon_va_mkold(mm, r->sampling_addr);
>  }
> -- 
> 2.50.1


Thanks,
SJ

Re: [PATCH] mm/damon: use a page-aligned sampling address

[Nathan Gao]

Hi SJ,

Thanks for your review!

On Thu, 27 Aug 2026 17:22:11 -0700 SJ Park <sj@kernel.org> wrote:

> Hello Nathan,
> 
> On Thu, 27 Aug 2026 12:38:21 -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. There are two users of
> > r->sampling_addr in vaddr.c that pass it into a page table walk, and
> > both use it as the address of a page.
> > 
> >   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)
> > 
> >   damon_va_young(mm, r->sampling_addr, &folio_sz)
> >     damon_va_walk_page_range(mm, addr, addr + 1)
> >       damon_young_pmd_entry()
> >         ptep_get(pte)
> >         mmu_notifier_test_young(walk->mm, addr)
> > 
> > test_and_clear_young_ptes(), which backs ptep_test_and_clear_young() on
> > arm64, documents @addr as "Address the first page is mapped at".
> > 
> > For arm64, before commit 6f0e1142173a ("arm64: mm: support batch
> > clearing of the young flag for large folios"),
> 
> The @addr documentation is also introduced by this commit.  This commit is
> authored at 2026-02-09.
> 
> > 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)
> 
> So, there was no issue before the commit.
> 

Right. Before 6f0e1142173a, unaligned addresses were tolerated but I don't
think this is guaranteed.

> > 
> > Align the sampled address down to a page boundary. It is the address of
> > the page to sample, so this matches its intended meaning and fixes both
> > users in vaddr.c.
> 
> This indeed sounds like can fix the issue to me.  However, was it a clear rule
> that we should pass only contepte-aligned addrss to
> ptep_test_and_clear_young()?  And is DAMON the only ptep_test_and_clear_young()
> caller that is mistakenly passing the unaligned address?
> 

It is not spelled out as an explicit rule, but the documented "Address
the first page is mapped at" implies it, and these callers are using
aligned addresses:

 mm/page_idle.c: page_idle_clear_pte_refs_one()
 fs/proc/task_mmu.c: clear_refs_pte_range()

> If not, it might make sense to make contpte_test_and_clear_young_ptes() support
> unaligned adress again in my opinion.  May I ask your opinion, Baolin?
> 
> > 
> > Fixes: 3f49584b262c ("mm/damon: implement primitives for the virtual memory address spaces")
> 
> I think 6f0e1142173a ("arm64: mm: support batch clearing of the young flag for
> large folios") would be mroe correct 'Fixes:', if there was no issue before the
> commit.
> 

Will use that in v2.

> > -	r->sampling_addr = damon_rand(ctx, r->ar.start, r->ar.end);
> > +	r->sampling_addr = PAGE_ALIGN_DOWN(damon_rand(ctx, r->ar.start,
> > +						      r->ar.end));
> 
> If we need to have the fix in DAMON, this kind of change would be needed.
> 
> However, what happens if the address is backed by large folios?
> 
> Before the commit 6f0e1142173a, also, it was aligning to CONT_PTE_SIZE.  Should
> we do same?

Passing a page-aligned address restores the pre-6f0e1142173a behavior.
Before the change, the helper aligned ptep down to the block start and
walked a fixed CONT_PTES entries, regardless of addr.  After the
change, the walk covers [ALIGN_DOWN(addr, CONT_PTE_SIZE),
ALIGN(addr + nr * PAGE_SIZE, CONT_PTE_SIZE)).  With a sub-page offset,
addr + PAGE_SIZE lands just past the block boundary, so the round-up
extends the walk a whole block further.  With a page-aligned addr,
addr + PAGE_SIZE is at most the block end, so the round-up lands
exactly on the block end and the walk covers the same CONT_PTES
entries as before the commit.

We also can't align to CONT_PTE_SIZE in DAMON since it's defined only under
arch/arm64/:

  #define CONT_PTES		(1 << (CONT_PTE_SHIFT - PAGE_SHIFT))
  #define CONT_PTE_SIZE		(CONT_PTES * PAGE_SIZE)


> Also, I think we should pass aligned address to only the functions that
> require alignement.  Making the alignment to the sampling address in general
> sounds too much to me.  Particularly, we are working on supporting new page
> access check primitives other than PTE Accessed bit, like AMd IBS.  In the
> case, we might support <PAGE_SIZE granularity monitoring.  Aligning sampling
> address in general will make it more complicated.

Makes sense. I will keep sampling_addr as is and align inside damon_va_mkold()
and damon_va_young() in v2.

Thanks,
Nathan

Re: [PATCH] mm/damon: use a page-aligned sampling address

[SJ Park]

On Fri, 28 Aug 2026 18:04:55 -0700 Nathan Gao <zcgao@amazon.com> wrote:

> Hi SJ,
> 
> Thanks for your review!
> 
> On Thu, 27 Aug 2026 17:22:11 -0700 SJ Park <sj@kernel.org> wrote:
> 
> > Hello Nathan,
> > 
> > On Thu, 27 Aug 2026 12:38:21 -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. There are two users of
> > > r->sampling_addr in vaddr.c that pass it into a page table walk, and
> > > both use it as the address of a page.
> > > 
> > >   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)
> > > 
> > >   damon_va_young(mm, r->sampling_addr, &folio_sz)
> > >     damon_va_walk_page_range(mm, addr, addr + 1)
> > >       damon_young_pmd_entry()
> > >         ptep_get(pte)
> > >         mmu_notifier_test_young(walk->mm, addr)
> > > 
> > > test_and_clear_young_ptes(), which backs ptep_test_and_clear_young() on
> > > arm64, documents @addr as "Address the first page is mapped at".
> > > 
> > > For arm64, before commit 6f0e1142173a ("arm64: mm: support batch
> > > clearing of the young flag for large folios"),
> > 
> > The @addr documentation is also introduced by this commit.  This commit is
> > authored at 2026-02-09.

I was wrong.  The documentation was introduced by commit 6d7237dda44f ("mm: add
a batched helper to clear the young flag for large folios"), which was authored
by Baolin on 2026-03-06.

> > 
> > > 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)
> > 
> > So, there was no issue before the commit.
> > 
> 
> Right. Before 6f0e1142173a, unaligned addresses were tolerated but I don't
> think this is guaranteed.
> 
> > > 
> > > Align the sampled address down to a page boundary. It is the address of
> > > the page to sample, so this matches its intended meaning and fixes both
> > > users in vaddr.c.
> > 
> > This indeed sounds like can fix the issue to me.  However, was it a clear rule
> > that we should pass only contepte-aligned addrss to
> > ptep_test_and_clear_young()?  And is DAMON the only ptep_test_and_clear_young()
> > caller that is mistakenly passing the unaligned address?
> > 
> 
> It is not spelled out as an explicit rule, but the documented "Address
> the first page is mapped at" implies it,

You mean the comment on test_and_clear_young_ptes(), right?  But as I mentioned
above, the comment was introduced by Baolin's patch that was authored on
2026-03-06.  I'd still appreciate Baolin's opinion.

> and these callers are using
> aligned addresses:
> 
>  mm/page_idle.c: page_idle_clear_pte_refs_one()
>  fs/proc/task_mmu.c: clear_refs_pte_range()
> 
> > If not, it might make sense to make contpte_test_and_clear_young_ptes() support
> > unaligned adress again in my opinion.  May I ask your opinion, Baolin?
> > 
> > > 
> > > Fixes: 3f49584b262c ("mm/damon: implement primitives for the virtual memory address spaces")
> > 
> > I think 6f0e1142173a ("arm64: mm: support batch clearing of the young flag for
> > large folios") would be mroe correct 'Fixes:', if there was no issue before the
> > commit.
> > 
> 
> Will use that in v2.
> 
> > > -	r->sampling_addr = damon_rand(ctx, r->ar.start, r->ar.end);
> > > +	r->sampling_addr = PAGE_ALIGN_DOWN(damon_rand(ctx, r->ar.start,
> > > +						      r->ar.end));
> > 
> > If we need to have the fix in DAMON, this kind of change would be needed.
> > 
> > However, what happens if the address is backed by large folios?
> > 
> > Before the commit 6f0e1142173a, also, it was aligning to CONT_PTE_SIZE.  Should
> > we do same?
> 
> Passing a page-aligned address restores the pre-6f0e1142173a behavior.
> Before the change, the helper aligned ptep down to the block start and
> walked a fixed CONT_PTES entries, regardless of addr.  After the
> change, the walk covers [ALIGN_DOWN(addr, CONT_PTE_SIZE),
> ALIGN(addr + nr * PAGE_SIZE, CONT_PTE_SIZE)).  With a sub-page offset,
> addr + PAGE_SIZE lands just past the block boundary, so the round-up
> extends the walk a whole block further.  With a page-aligned addr,
> addr + PAGE_SIZE is at most the block end, so the round-up lands
> exactly on the block end and the walk covers the same CONT_PTES
> entries as before the commit.
> 
> We also can't align to CONT_PTE_SIZE in DAMON since it's defined only under
> arch/arm64/:
> 
>   #define CONT_PTES		(1 << (CONT_PTE_SHIFT - PAGE_SHIFT))
>   #define CONT_PTE_SIZE		(CONT_PTES * PAGE_SIZE)

DAMON cares only exactly the byte of the address, so I agree this would work
for DAMON and be safe.  But, still the behavior is not exactly same to
pre-6f0e1142173a, isn't it?  I'm not really sure if this is really the correct
use of the function.  Again, I'd appreciate Baolin's comment.

> 
> 
> > Also, I think we should pass aligned address to only the functions that
> > require alignement.  Making the alignment to the sampling address in general
> > sounds too much to me.  Particularly, we are working on supporting new page
> > access check primitives other than PTE Accessed bit, like AMd IBS.  In the
> > case, we might support <PAGE_SIZE granularity monitoring.  Aligning sampling
> > address in general will make it more complicated.
> 
> Makes sense. I will keep sampling_addr as is and align inside damon_va_mkold()
> and damon_va_young() in v2.

Regardless of Baolin's comment, let's fix this issue.  So the v2 would be
appreciated.  In the v2, could you also add more details about how the issue
can be reproduced, and the user impact?  You mentioned you found memory
corruption from DAMON selftets.  It would be nice if you could make it more
detailed, such as what selftest reproduces the issue and what symptoms it
showed you.

Nevertheless I'm also wondering if supporting unaligned address again, like
below also works.

'''
--- a/arch/arm64/mm/contpte.c
+++ b/arch/arm64/mm/contpte.c
@@ -519,9 +519,12 @@ bool contpte_test_and_clear_young_ptes(struct vm_area_struct *vma,
         * of the same large folio in a single VMA and a single page table.
         */

-       unsigned long end = addr + nr * PAGE_SIZE;
+       unsigned long end;
        bool young = false;

+       ptep = contpte_align_down(ptep);
+       addr = ALIGN_DOWN(addr, CONT_PTE_SIZE);
+       end = addr + nr * PAGE_SIZE;
        ptep = contpte_align_addr_ptep(&addr, &end, ptep, nr);
        for (; addr != end; ptep++, addr += PAGE_SIZE)
                young |= __ptep_test_and_clear_young(vma, addr, ptep);
'''

Nathan, what do you think?  If it makes sense to you, could you also test this?


Thanks,
SJ

[...]