mm/damon/vaddr: avoid hw-driven pte updates during damon_hugetlb_mkold()
Subsystems (3)
SJ Park
maintainerPATCH 1/1 mm/damon/vaddr: avoid hw-driven pte updates during damon_hugetlb_mkold()
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/[email protected]
Changes from RFC
- RFC: https://lore.kernel.org/[email protected]
- Drop RFC tag.
--- mm/damon/vaddr.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
mm/damon/vaddr.c +14 -7
@@ -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
Signature · 1 line
2.47.3
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Modifying contiguous huge PTEs by clearing and re-setting without an intermediate TLB flush…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying contiguous huge PTEs by clearing and re-setting without an intermediate TLB flush violates Break-Before-Make (BBM) rules on architectures like arm64, potentially causing fatal TLB conflict aborts. --
commit 1fee0c4237cb61938cdabe61cd4adf713c7ddbec
Author: SJ Park <[email protected]>
mm/damon/vaddr: avoid hw-driven pte updates during damon_hugetlb_mkold()
This commit attempts to prevent hardware-driven PTE updates (like dirty bits) from being lost during DAMON's mkold operation on hugetlb pages. It does this by replacing huge_ptep_get() with huge_ptep_get_and_clear() before calling set_huge_pte_at().
Quoted 4 lines
mm/damon/vaddr.c +0
-0
[ ... ]
Quoted 4 lines
mm/damon/vaddr.c +0
-0@@ -283,22 +283,29 @@ static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr, }#ifdef CONFIG_HUGETLB_PAGE
mm/damon/vaddr.c static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr, · line 295
Quoted 10 lines
mm/damon/vaddr.c +10
-0+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);
[Severity: High]
Does this modification of contiguous huge PTEs violate Break-Before-Make (BBM) rules on architectures like arm64?
By calling huge_ptep_get_and_clear() followed by set_huge_pte_at() without an intermediate TLB flush, the hardware might cache conflicting contiguous TLB entries, which could cause fatal TLB conflict aborts. On arm64, huge_ptep_get_and_clear() clears the constituent PTEs but does not flush the TLB. When set_huge_pte_at() is subsequently called, it skips its internal BBM TLB flush because the PTEs were already cleared and its internal validity check evaluates to false.
Quoted 4 lines
mm/damon/vaddr.c +3
-0+return true; +} + static void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm,
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
SJ Park
maintainerQuoted 43 lines · On Mon, 07 Sep 2026 17:16:13 +0000 [email protected] wrote:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying contiguous huge PTEs by clearing and re-setting without an intermediate TLB flush violates Break-Before-Make (BBM) rules on architectures like arm64, potentially causing fatal TLB conflict aborts. --commit
1fee0c4237cb61938cdabe61cd4adf713c7ddbec
Author: SJ Park <[email protected]>
mm/damon/vaddr: avoid hw-driven pte updates duringdamon_hugetlb_mkold()This commit attempts to prevent hardware-driven PTE updates (like dirty bits) from being lost during DAMON's mkold operation on hugetlb pages. It does this by replacing
huge_ptep_get()withhuge_ptep_get_and_clear()before callingset_huge_pte_at().Quoted 4 lines
mm/damon/vaddr.c +0
-0[ ... ]
Quoted 4 lines
mm/damon/vaddr.c +0
-0@@ -283,22 +283,29 @@ static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr, }#ifdef CONFIG_HUGETLB_PAGE mm/damon/vaddr.c static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr, · line 295
Quoted 10 lines
mm/damon/vaddr.c +10
-0+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);[Severity: High]
Does this modification of contiguous huge PTEs violate Break-Before-Make (BBM) rules on architectures like arm64?
Quoted 7 lines
By calling
huge_ptep_get_and_clear()followed byset_huge_pte_at()without an intermediate TLB flush, the hardware might cache conflicting contiguous TLB entries, which could cause fatal TLB conflict aborts. On arm64,huge_ptep_get_and_clear()clears the constituent PTEs but does not flush the TLB. Whenset_huge_pte_at()is subsequently called, it skips its internal BBM TLB flush because the PTEs were already cleared and its internal validity check evaluates to false.
But we are changing only accessed bit. I think BBM is not required for this? Please correct me if I'm wrong.
Quoted 8 lines
Quoted 5 lines
mm/damon/vaddr.c +3
-0+return true; +} + static void damon_hugetlb_mkold(pte_t *pte, struct mm_struct *mm,Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Thanks,
SJ
SJ Park
maintainerOn Mon, 7 Sep 2026 10:03:56 -0700 SJ Park <[email protected]> wrote:
[...]
mm/damon/vaddr.c static int damon_mkold_pmd_entry(pmd_t *pmd, unsigned long addr, · line 296
Quoted 11 lines
mm/damon/vaddr.c +11
-0+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/[email protected]
Thanks,
SJ
[...]