[PATCH v2 0/3] hold PTL for several .hugetlb_entry callbacks

[PATCH v2 0/3] hold PTL for several .hugetlb_entry callbacks

[Jinjiang Tu]

Changelog since v1:
 * update the subject pacth #1
 * add patch #2 and #3 to hold PTL for some other .hugetlb_entry callbacks

Patch #1 fixes a race between smaps_hugetlb_range() and migration by holding
PTL in smaps_hugetlb_range().

To avoid operating on stale pages, add PTL for several .hugetlb_entry
callbacks too. We don't hold PTL in __s390_enable_skey_hugetlb() and
prot_none_hugetlb_entry(), since the mmap write lock is held. We don't
hold PTL in hwpoison_hugetlb_range() too, since it only reads the pte and
doesn't operate the page.

Jinjiang Tu (3):
  mm/smaps: fix race between smaps_hugetlb_range and migration
  fs/proc/task_mmu: hold PTL in pagemap_hugetlb_range and
    gather_hugetlb_stats
  mm/mincore: hold PTL in mincore_hugetlb

 fs/proc/task_mmu.c | 20 ++++++++++++++++----
 mm/mincore.c       |  3 +++
 2 files changed, 19 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH v2 1/3] mm/smaps: fix race between smaps_hugetlb_range and migration

[Jinjiang Tu]

smaps_hugetlb_range() handles the pte without holdling ptl, and may be
concurrenct with migration, leaing to BUG_ON in pfn_swap_entry_to_page().
The race is as follows.

smaps_hugetlb_range              migrate_pages
  huge_ptep_get
                                   remove_migration_ptes
				   folio_unlock
  pfn_swap_entry_folio
    BUG_ON

To fix it, hold ptl lock in smaps_hugetlb_range().

Fixes: 25ee01a2fca0 ("mm: hugetlb: proc: add hugetlb-related fields to /proc/PID/smaps")
Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 fs/proc/task_mmu.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 751479eb128f..0102ab3aaec1 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1020,10 +1020,13 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
 {
 	struct mem_size_stats *mss = walk->private;
 	struct vm_area_struct *vma = walk->vma;
-	pte_t ptent = huge_ptep_get(walk->mm, addr, pte);
 	struct folio *folio = NULL;
 	bool present = false;
+	spinlock_t *ptl;
+	pte_t ptent;
 
+	ptl = huge_pte_lock(hstate_vma(vma), walk->mm, pte);
+	ptent = huge_ptep_get(walk->mm, addr, pte);
 	if (pte_present(ptent)) {
 		folio = page_folio(pte_page(ptent));
 		present = true;
@@ -1042,6 +1045,7 @@ static int smaps_hugetlb_range(pte_t *pte, unsigned long hmask,
 		else
 			mss->private_hugetlb += huge_page_size(hstate_vma(vma));
 	}
+	spin_unlock(ptl);
 	return 0;
 }
 #else
-- 
2.43.0

[PATCH v2 2/3] fs/proc/task_mmu: hold PTL in pagemap_hugetlb_range and gather_hugetlb_stats

[Jinjiang Tu]

Hold PTL in pagemap_hugetlb_range() and gather_hugetlb_stats() to avoid
operating on stale page, as pagemap_pmd_range() and gather_pte_stats() have
done.

Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 fs/proc/task_mmu.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 0102ab3aaec1..68077aef6c5b 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -1893,12 +1893,14 @@ static int pagemap_hugetlb_range(pte_t *ptep, unsigned long hmask,
 	struct pagemapread *pm = walk->private;
 	struct vm_area_struct *vma = walk->vma;
 	u64 flags = 0, frame = 0;
+	spinlock_t *ptl;
 	int err = 0;
 	pte_t pte;
 
 	if (vma->vm_flags & VM_SOFTDIRTY)
 		flags |= PM_SOFT_DIRTY;
 
+	ptl = huge_pte_lock(hstate_vma(vma), walk->mm, ptep);
 	pte = huge_ptep_get(walk->mm, addr, ptep);
 	if (pte_present(pte)) {
 		struct folio *folio = page_folio(pte_page(pte));
@@ -1926,11 +1928,12 @@ static int pagemap_hugetlb_range(pte_t *ptep, unsigned long hmask,
 
 		err = add_to_pagemap(&pme, pm);
 		if (err)
-			return err;
+			break;
 		if (pm->show_pfn && (flags & PM_PRESENT))
 			frame++;
 	}
 
+	spin_unlock(ptl);
 	cond_resched();
 
 	return err;
@@ -3004,17 +3007,22 @@ static int gather_pte_stats(pmd_t *pmd, unsigned long addr,
 static int gather_hugetlb_stats(pte_t *pte, unsigned long hmask,
 		unsigned long addr, unsigned long end, struct mm_walk *walk)
 {
-	pte_t huge_pte = huge_ptep_get(walk->mm, addr, pte);
+	pte_t huge_pte;
 	struct numa_maps *md;
 	struct page *page;
+	spinlock_t *ptl;
 
+	ptl = huge_pte_lock(hstate_vma(walk->vma), walk->mm, pte);
+	huge_pte = huge_ptep_get(walk->mm, addr, pte);
 	if (!pte_present(huge_pte))
-		return 0;
+		goto out;
 
 	page = pte_page(huge_pte);
 
 	md = walk->private;
 	gather_stats(page, md, pte_dirty(huge_pte), 1);
+out:
+	spin_unlock(ptl);
 	return 0;
 }
 
-- 
2.43.0

[PATCH v2 3/3] mm/mincore: hold PTL in mincore_hugetlb

[Jinjiang Tu]

Hold PTL in mincore_hugetlb() to avoid operating on stale page, as
mincore_pte_range() have done.

Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 mm/mincore.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/mincore.c b/mm/mincore.c
index 42d6c9c8da86..10dabefc3acc 100644
--- a/mm/mincore.c
+++ b/mm/mincore.c
@@ -29,7 +29,9 @@ static int mincore_hugetlb(pte_t *pte, unsigned long hmask, unsigned long addr,
 #ifdef CONFIG_HUGETLB_PAGE
 	unsigned char present;
 	unsigned char *vec = walk->private;
+	spinlock_t *ptl;
 
+	ptl = huge_pte_lock(hstate_vma(walk->vma), walk->mm, pte);
 	/*
 	 * Hugepages under user process are always in RAM and never
 	 * swapped out, but theoretically it needs to be checked.
@@ -38,6 +40,7 @@ static int mincore_hugetlb(pte_t *pte, unsigned long hmask, unsigned long addr,
 	for (; addr != end; vec++, addr += PAGE_SIZE)
 		*vec = present;
 	walk->private = vec;
+	spin_unlock(ptl);
 #else
 	BUG();
 #endif
-- 
2.43.0

Re: [PATCH v2 0/3] hold PTL for several .hugetlb_entry callbacks

[David Hildenbrand]

On 24.07.25 11:09, Jinjiang Tu wrote:
> Changelog since v1:
>   * update the subject pacth #1
>   * add patch #2 and #3 to hold PTL for some other .hugetlb_entry callbacks
> 
> Patch #1 fixes a race between smaps_hugetlb_range() and migration by holding
> PTL in smaps_hugetlb_range().
> 
> To avoid operating on stale pages, add PTL for several .hugetlb_entry
> callbacks too. We don't hold PTL in __s390_enable_skey_hugetlb() and
> prot_none_hugetlb_entry(), since the mmap write lock is held.

There could be rmap walkers only holding the rmap lock (but I recall 
that s390x does not support migration of hugetlb folios, not sure).

In any, case, ok to leave that alone for now.

We don't
> hold PTL in hwpoison_hugetlb_range() too, since it only reads the pte and
> doesn't operate the page.

Not sure ... we should just handle it like the PTE/PMD case and not play 
any tricks.


-- 
Cheers,

David / dhildenb

Re: [PATCH v2 1/3] mm/smaps: fix race between smaps_hugetlb_range and migration

[David Hildenbrand]

On 24.07.25 11:09, Jinjiang Tu wrote:
> smaps_hugetlb_range() handles the pte without holdling ptl, and may be
> concurrenct with migration, leaing to BUG_ON in pfn_swap_entry_to_page().
> The race is as follows.
> 
> smaps_hugetlb_range              migrate_pages
>    huge_ptep_get
>                                     remove_migration_ptes
> 				   folio_unlock
>    pfn_swap_entry_folio
>      BUG_ON
> 
> To fix it, hold ptl lock in smaps_hugetlb_range().
> 
> Fixes: 25ee01a2fca0 ("mm: hugetlb: proc: add hugetlb-related fields to /proc/PID/smaps")
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 2/3] fs/proc/task_mmu: hold PTL in pagemap_hugetlb_range and gather_hugetlb_stats

[David Hildenbrand]

On 24.07.25 11:09, Jinjiang Tu wrote:
> Hold PTL in pagemap_hugetlb_range() and gather_hugetlb_stats() to avoid
> operating on stale page, as pagemap_pmd_range() and gather_pte_stats() have
> done.
> 
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 3/3] mm/mincore: hold PTL in mincore_hugetlb

[David Hildenbrand]

On 24.07.25 11:09, Jinjiang Tu wrote:
> Hold PTL in mincore_hugetlb() to avoid operating on stale page, as
> mincore_pte_range() have done.
> 
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---

Yeah, likely nothing broken but matches the ordinary PMD handling.

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH v2 0/3] hold PTL for several .hugetlb_entry callbacks

[Jinjiang Tu]

在 2025/7/25 0:26, David Hildenbrand 写道:
> On 24.07.25 11:09, Jinjiang Tu wrote:
>> Changelog since v1:
>>   * update the subject pacth #1
>>   * add patch #2 and #3 to hold PTL for some other .hugetlb_entry 
>> callbacks
>>
>> Patch #1 fixes a race between smaps_hugetlb_range() and migration by 
>> holding
>> PTL in smaps_hugetlb_range().
>>
>> To avoid operating on stale pages, add PTL for several .hugetlb_entry
>> callbacks too. We don't hold PTL in __s390_enable_skey_hugetlb() and
>> prot_none_hugetlb_entry(), since the mmap write lock is held.
>
> There could be rmap walkers only holding the rmap lock (but I recall 
> that s390x does not support migration of hugetlb folios, not sure).
>
> In any, case, ok to leave that alone for now.
>
> We don't
>> hold PTL in hwpoison_hugetlb_range() too, since it only reads the pte 
>> and
>> doesn't operate the page.
>
> Not sure ... we should just handle it like the PTE/PMD case and not 
> play any tricks.
>
Indeed, I will update hwpoison_hugetlb_range() too.

[PATCH] mm/memory-failure: hold PTL in hwpoison_hugetlb_range

[Jinjiang Tu]

Hold PTL in hwpoison_hugetlb_range() to avoid operating on stale page, as
hwpoison_pte_range() have done.

Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
---
 mm/memory-failure.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index b91a33fb6c69..0ad3d55d1d09 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -837,11 +837,17 @@ static int hwpoison_hugetlb_range(pte_t *ptep, unsigned long hmask,
 			    struct mm_walk *walk)
 {
 	struct hwpoison_walk *hwp = walk->private;
-	pte_t pte = huge_ptep_get(walk->mm, addr, ptep);
 	struct hstate *h = hstate_vma(walk->vma);
+	spinlock_t *ptl;
+	pte_t pte;
+	int ret;
 
-	return check_hwpoisoned_entry(pte, addr, huge_page_shift(h),
-				      hwp->pfn, &hwp->tk);
+	ptl = huge_pte_lock(h, walk->mm, ptep);
+	pte = huge_ptep_get(walk->mm, addr, ptep);
+	ret = check_hwpoisoned_entry(pte, addr, huge_page_shift(h),
+					hwp->pfn, &hwp->tk);
+	spin_unlock(ptl);
+	return ret;
 }
 #else
 #define hwpoison_hugetlb_range	NULL
-- 
2.43.0

Re: [PATCH] mm/memory-failure: hold PTL in hwpoison_hugetlb_range

[David Hildenbrand]

On 25.07.25 05:31, Jinjiang Tu wrote:
> Hold PTL in hwpoison_hugetlb_range() to avoid operating on stale page, as
> hwpoison_pte_range() have done.
> 
> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH] mm/memory-failure: hold PTL in hwpoison_hugetlb_range

[Andrew Morton]

On Fri, 25 Jul 2025 11:31:12 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:

> Hold PTL in hwpoison_hugetlb_range() to avoid operating on stale page, as
> hwpoison_pte_range() have done.

Does this fix any known runtime issues?  Or is it expected to?

In other words, do we have reason to backport this fix into earlier
kernels?

Re: [PATCH] mm/memory-failure: hold PTL in hwpoison_hugetlb_range

[Jinjiang Tu]

[-- Attachment #1: Type: text/plain, Size: 534 bytes --]


在 2025/7/26 7:29, Andrew Morton 写道:
> On Fri, 25 Jul 2025 11:31:12 +0800 Jinjiang Tu<tujinjiang@huawei.com> wrote:
>
>> Hold PTL in hwpoison_hugetlb_range() to avoid operating on stale page, as
>> hwpoison_pte_range() have done.
> Does this fix any known runtime issues?  Or is it expected to?

It doesn't. There is no known issues, PTL is expected to hold like PTE handling to avoid poissible issues.

So, there is no need to backport it.

>
> In other words, do we have reason to backport this fix into earlier
> kernels?
>
>

[-- Attachment #2: Type: text/html, Size: 1303 bytes --]