Re: [PATCH v2 2/3] mm/damon/ops-common: handle hugetlb folios in folio mkold/young rmap walkers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Non-atomic clearing of huge PTE young bit silently overwrites hardware dirty bit
--

commit 04cc6d81cf1593c6fb092b306da28db5569a3f11
Author: Krishna Iyer <kiyer@crusoe.ai>

mm/damon/ops-common: handle hugetlb folios in folio mkold/young rmap walkers

This commit adds hugetlb branches to the damon_folio_mkold_one and
damon_folio_young_one rmap walkers. It reuses damon_hugetlb_mkold to properly
age huge PTEs and introduces a new helper damon_hugetlb_young for checking
their accessed state.

> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index f5fe92b825bbc..373b25db59505 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -193,10 +193,15 @@ static bool damon_folio_mkold_one(struct folio *folio,
>  
>  	while (page_vma_mapped_walk(&pvmw)) {
>  		addr = pvmw.address;
> -		if (pvmw.pte)
> -			damon_ptep_mkold(pvmw.pte, vma, addr);
> -		else
> +		if (pvmw.pte) {
> +			if (folio_test_hugetlb(folio))
> +				damon_hugetlb_mkold(pvmw.pte, vma->vm_mm, vma,
> +						addr);

[Severity: High]
This is a pre-existing issue, but does expanding the use of
damon_hugetlb_mkold here expose us to hardware dirty bit overwrites?

Looking at damon_hugetlb_mkold in mm/damon/ops-common.c, it performs a
non-atomic read-modify-write on the huge PTE:

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

If the hardware sets the dirty bit after software reads the PTE via
huge_ptep_get, but before it writes it back via set_huge_pte_at, the kernel
will silently discard the newly dirtied state. This can lead to data
corruption as the page might be dropped assuming it is clean.

Since DAMON monitoring can be configured to monitor hugetlb VMAs, triggering
this path, can this non-atomic clearing be safely avoided or addressed here?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902025700.17975-1-kiyer@crusoe.ai?part=2