[PATCH] mm/damon/vaddr: avoid hw-driven pte updates during damon_hugetlb_mkold()

[PATCH] mm/damon/vaddr: avoid hw-driven pte updates during damon_hugetlb_mkold()

[SJ Park]

damon_hugetlb_mkold() reads the page table entry into a local variable,
unsets the accessed bit in the variable, and updates the page table
entry with the updated variable value.  If hardware updates the same
page table entry in parallel, the hw updates could be lost.  For
example, hardware-updated dirty bits might be lost.

Avoid the parallel updates by clearing the page table entry when reading
it together, using huge_ptep_get_and_clear().  If a parallel write to
the memory is made after the clearing, the hw will see the page table
entry is cleared, trigger page fault and wait until it is handled.  The
page fault handling will wait for damon_hugetlb_mkold() due to the page
table lock.

Because hugetlbfs is an in-memory file system and hugetlb pages cannot
be reclaimed, no critical issue is expected to my best knowledge.  But
definitely this is a nasty bug that should be fixed sooner rather than
later.

The issue was discovered [1] by Sashiko.

[1] https://lore.kernel.org/20260830160545.98969-1-sj@kernel.org

Changes from RFC
- RFC: https://lore.kernel.org/20260906195417.103263-1-sj@kernel.org
- Drop RFC tag.

Fixes: 49f4203aae06 ("mm/damon: add access checking for hugetlb pages")
Cc: <stable@vger.kernel.org> # 5.17.x
Signed-off-by: SJ Park <sj@kernel.org>
---
 mm/damon/vaddr.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/mm/damon/vaddr.c b/mm/damon/vaddr.c
index f884d3f78f30a..91a0d441c1f94 100644
--- a/mm/damon/vaddr.c
+++ b/mm/damon/vaddr.c
@@ -283,22 +283,29 @@ static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr,
 }
 
 #ifdef CONFIG_HUGETLB_PAGE
+static bool damon_hugetlb_ptep_mkold(pte_t *pte, struct mm_struct *mm,
+		struct vm_area_struct *vma, unsigned long addr, pte_t *entry)
+{
+	unsigned long psize = huge_page_size(hstate_vma(vma));
+
+	if (!pte_young(*entry))
+		return false;
+	*entry = huge_ptep_get_and_clear(mm, addr, pte, psize);
+	*entry = pte_mkold(*entry);
+	set_huge_pte_at(mm, addr, pte, *entry, psize);
+	return true;
+}
+
 static void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm,
 				struct vm_area_struct *vma, unsigned long addr)
 {
 	bool referenced = false;
 	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);
-	}
-
+	referenced = damon_hugetlb_ptep_mkold(pte, mm, vma, addr, &entry);
 	if (mmu_notifier_clear_young(mm, addr,
 				     addr + huge_page_size(hstate_vma(vma))))
 		referenced = true;

base-commit: 13eba1898080a8a54c60710322909fb0a4d35e84
-- 
2.47.3

Re: [PATCH] mm/damon/vaddr: avoid hw-driven pte updates during damon_hugetlb_mkold()

[SJ Park]

On Mon,  7 Sep 2026 10:03:56 -0700 SJ Park <sj@kernel.org> wrote:

[...]
> +static bool damon_hugetlb_ptep_mkold(pte_t *pte, struct mm_struct *mm,
> +		struct vm_area_struct *vma, unsigned long addr, pte_t *entry)
> +{
> +	unsigned long psize = huge_page_size(hstate_vma(vma));
> +
> +	if (!pte_young(*entry))
> +		return false;
> +	*entry = huge_ptep_get_and_clear(mm, addr, pte, psize);
> +	*entry = pte_mkold(*entry);
> +	set_huge_pte_at(mm, addr, pte, *entry, psize);
> +	return true;

Sashiko says [1] this might violate BBM.  But this is only for accessed bit
change.  So I don't think BBM is required here.  Also, we intentionally do not
flush TLB because this is just a best-effort monitoring.  Please correct me if
I'm wrong.

[1] https://lore.kernel.org/20260907171613.A09B11F00A3A@smtp.kernel.org


Thanks,
SJ

[...]