Re: [PATCH v2] mm/damon/vaddr: use a page-aligned address for the sampling walks

[SJ Park]

On Tue, 1 Sep 2026 13:25:36 -0700 Nathan Gao <zcgao@xxxxxxxxxx> wrote:

> On Mon, 31 Aug 2026 18:35:50 -0700 SJ Park <sj@xxxxxxxxxx> wrote:
> 
> > On Mon, 31 Aug 2026 15:11:51 -0700 Nathan Gao <zcgao@xxxxxxxxxx> wrote:
[...]
> > > 
> > > Otherwise the page is writable, the PTE_AF clearing succeeds silently
> > > and the damage only surfaces later, in whatever happened to own the
> > > page, so the backtrace is unrelated to DAMON and differs between runs.
> > 
> > Urgh, this must have been a painful debugging.  Sorry about that, and
> > appreciate your great work on this!
> > 
> 
> No problem at all!  Had fun digging into this. 

Thank you for that kind ack!

> 
> > > 
> > > Align the address down to a page boundary in damon_va_mkold() and
> > > damon_va_young(), the two users that pass it into a page table walk. It
> > > is the address of the page to sample, so this matches its intended
> > > meaning. r->sampling_addr itself is left as is, so the sampling and
> > > region bookkeeping semantics are unchanged.
> > 
> > I'm still wondering if it makes sense to restore unaligned address support in
> > contpte_test_and_clear_young_ptes() as a long term fix.
> > 
> 
> I'd appreciate Baolin's thoughts here.  If it turns out callers are
> expected to do the alignment, it would be better to have a WARN to expose
> the issue.

I agree.  If we conclude contpte helpers are not the right place to warn, we
should do that at least in DAMON internal helpers.

[...]
> > > --- a/mm/damon/vaddr.c
> > > +++ b/mm/damon/vaddr.c
> > > @@ -349,6 +349,9 @@ static void damon_va_mkold(struct mm_struct *mm, unsigned long addr)
> > >  		.hugetlb_entry = damon_mkold_hugetlb_entry,
> > >  	};
> > >  
> > > +	/* Arch helpers can derive a page range from @addr; align it down. */
> > > +	addr = PAGE_ALIGN_DOWN(addr);
> > > +
> > >  	damon_va_walk_page_range(mm, addr, addr + 1, &damon_mkold_ops, NULL);
> > 
> > Could we do the alignment just before passing the addr to
> > contpte_test_and_clear_young_ptes(), which is the exact function that disallows
> > the unaligned address?
> > 
> 
> Sent a v3.  It moves the alignment into damon_ptep_mkold(), the only place
> in DAMON that reaches contpte_test_and_clear_young_ptes(), so it is as
> close to that function as DAMON can get:
> 
> https://lore.kernel.org/all/20260901201001.33271-1-zcgao@xxxxxxxxxx/

Thank you, I left comments.  Please forgive me being picky there.


Thanks,
SJ

[...]