[PATCH v2 0/4] mm: gup: batch PTE-mapped large folios in gup_fast_pte_range()

[PATCH v2 0/4] mm: gup: batch PTE-mapped large folios in gup_fast_pte_range()

[Yeoreum Yun]

GUP-fast grabs a PTE-mapped large folio one page at a time.  Every PTE
costs a try_grab_folio_fast() (a refcount cmpxchg, plus the pincount
atomic, a full barrier and a node stat update for FOLL_PIN), a
gup_fast_folio_allowed() and a folio_set_referenced(), so a 64 kB mTHP
pays sixteen of each and a PTE-mapped 2 MB THP pays 512.

The users are the hot ones: iov_iter_extract_user_pages() pins every
O_DIRECT buffer through pin_user_pages_fast(), io_uring registers its
buffers through it, and so does RDMA memory registration.
With mTHP those buffers are PTE-mapped large folios.

After the head page has been grabbed and verified, extend the run to the
following PTEs that map the next pages of the same folio with the
same protection bits, take the remaining references for the
run with try_grab_folio(), and fill pages[] from the run.
The PTEs of the run are read while the reference on the folio is held,
so the folio can neither be collapsed, freed nor split underneath the scan.

A read-only run of an anonymous folio stops at the first page that
gup_must_unshare() rejects, so the per-page exclusivity guarantee for FOLL_PIN
is kept.

folio_pte_batch_flags() does the scan.  It reads the entries with
ptep_get(), which is not safe without the page table lock on
CONFIG_GUP_GET_PXX_LOW_HIGH (x86 PAE, mips32, sh) or with arm64 contpte,
and GUP-fast reads every PTE with ptep_get_lockless() for that reason.
Add FPB_LOCKLESS to make the helper do the same; it is inlined per call
site, so the other callers do not change.

The order-0 path keeps its instruction sequence.  The folio flags are
tested before folio_set_referenced() on purpose: a load of the flags
right after that locked instruction cost about 25% on order-0 pages in
the gup_test benchmark below, and reading them first brings that path
back to the baseline.

gup_test ioctl over a 256 MB anonymous region, FOLL_WRITE, 65536 pages
per call, median of 15 runs, two baseline boots / one patched boot, on
an x86-64 VM:

                       get_user_pages_fast()   pin_user_pages_fast()
                       before       after      before       after
  4 kB pages           849/789 us   783 us     1174/1138 us 1094 us
  64 kB mTHP           812/811 us   126 us     1215/1113 us  188 us
  1 MB mTHP            799/774 us    59 us     1185/1077 us   64 us
  2 MB THP, PTE-mapped 803/786 us    57 us     1159/1120 us   58 us
  2 MB THP, PMD-mapped  42/40 us     40 us       46/41 us     42 us

fio O_DIRECT randread of a null_blk device (CPU bound) with the I/O
buffer in 64 kB mTHP, median of 15 five-second runs:

  bs=1M psync                 126.2/125.6 GB/s  ->  220.6 GB/s
  bs=1M io_uring, iodepth 16   71.2/71.6 GB/s   ->  112.1 GB/s
  bs=64k io_uring, iodepth 16  50.2/50.7 GB/s   ->   60.1 GB/s

IORING_REGISTER_BUFFERS of 1 GB, median of 15:

64 kB mTHP           6179/5998 us  ->  1599 us
  1 MB mTHP            5790/5801 us  ->   893 us
  4 kB pages           7173/6741 us  ->  6763 us

---
Changes in v2:
- fix exceptional case for shstk on x86.
- remove redendant check for pmd change in each pte barch.
- Link to v1: https://lore.kernel.org/all/20260917083747.786-1-aa9736195201@gmail.com/

---
Yeoreum Yun (3):
      mm: gup: introduce gup_unshared_pages()
      mm: internal: fix inconsistent in folio_pte_batch_flags() for shadow stack
      mm: gup: batch PTE-mapped large folios in gup_fast_pte_range()

Yuan-Hao Hsu (1):
      mm: internal: introduce FPB_LOCKLESS for folio_pte_batch_flags()

 mm/gup.c      | 113 ++++++++++++++++++++++++++++++++++++++++++++++------------
 mm/internal.h |  70 ++++++++++++++----------------------
 2 files changed, 117 insertions(+), 66 deletions(-)
---
base-commit: 34f88a114888c9dba49e45f2b0c2a3e83c4c871c
change-id: 20261015-refactor_gup_fast-b55e3f593dc2

Best regards,
-- 
Sincerely,
Yeoreum Yun

[PATCH v2 1/4] mm: gup: introduce gup_unshared_pages()

[Yeoreum Yun]

This is preparatory patch to make gup_fast_pte_range() batchable.

Introduce gup_unshared_pages() to count consecutive pages within a folio
that do not require unsharing, and use it to implement gup_must_unshare().

Also add a folio argument to gup_must_unshare() so callers can reuse an
existing folio and avoid the compound_head() call implicit in PageAnon().

There is no functional change.

Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
 mm/gup.c      | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
 mm/internal.h | 49 ++++++------------------------------------
 2 files changed, 70 insertions(+), 48 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 7d43ce717b246..49681f63950ec 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -633,6 +633,59 @@ static struct page *no_page_table(struct vm_area_struct *vma,
 	return NULL;
 }
 
+unsigned long gup_unshared_pages(struct vm_area_struct *vma, unsigned int flags,
+				 struct folio *folio, struct page *page,
+				 unsigned long nr_pages)
+{
+	unsigned int i;
+
+	/*
+	 * FOLL_WRITE is implicitly handled correctly as the page table entry
+	 * has to be writable -- and if it references (part of) an anonymous
+	 * folio, that part is required to be marked exclusive.
+	 */
+	if ((flags & (FOLL_WRITE | FOLL_PIN)) != FOLL_PIN)
+		return nr_pages;
+	/*
+	 * Note: PageAnon(page) is stable until the page is actually getting
+	 * freed.
+	 */
+	if (!folio_test_anon(folio)) {
+		/*
+		 * We only care about R/O long-term pining: R/O short-term
+		 * pinning does not have the semantics to observe successive
+		 * changes through the process page tables.
+		 */
+		if (!(flags & FOLL_LONGTERM))
+			return nr_pages;
+
+		/* We really need the vma ... */
+		if (!vma || vma_is_cow_mapping(vma))
+			return 0;
+
+		/*
+		 * ... because we only care about writable private ("COW")
+		 * mappings where we have to break COW early.
+		 */
+		return nr_pages;
+	}
+
+	/* Paired with a memory barrier in folio_try_share_anon_rmap_*(). */
+	if (IS_ENABLED(CONFIG_HAVE_GUP_FAST))
+		smp_rmb();
+
+	/*
+	 * Note that KSM pages cannot be exclusive, and consequently,
+	 * cannot get pinned.
+	 */
+	for (i = 0; i < nr_pages; i++) {
+		if (!PageAnonExclusive(page + i))
+			return i;
+	}
+
+	return nr_pages;
+}
+
 #ifdef CONFIG_PGTABLE_HAS_HUGE_LEAVES
 /* FOLL_FORCE can write to even unwritable PUDs in COW mappings. */
 static inline bool can_follow_write_pud(pud_t pud, struct page *page,
@@ -668,7 +721,8 @@ static struct page *follow_huge_pud(struct vm_area_struct *vma,
 	pfn += (addr & ~PUD_MASK) >> PAGE_SHIFT;
 	page = pfn_to_page(pfn);
 
-	if (!pud_write(pud) && gup_must_unshare(vma, flags, page))
+	if (!pud_write(pud) &&
+	    gup_must_unshare(vma, flags, page_folio(page), page))
 		return ERR_PTR(-EMLINK);
 
 	ret = try_grab_folio(page_folio(page), 1, flags);
@@ -722,7 +776,8 @@ static struct page *follow_huge_pmd(struct vm_area_struct *vma,
 	if (pmd_protnone(*pmd) && !gup_can_follow_protnone(vma, flags))
 		return NULL;
 
-	if (!pmd_write(pmdval) && gup_must_unshare(vma, flags, page))
+	if (!pmd_write(pmdval) &&
+	    gup_must_unshare(vma, flags, page_folio(page), page))
 		return ERR_PTR(-EMLINK);
 
 	VM_WARN_ON_ONCE_PAGE((flags & FOLL_PIN) && PageAnon(page) &&
@@ -846,7 +901,7 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
 	}
 	folio = page_folio(page);
 
-	if (!pte_write(pte) && gup_must_unshare(vma, flags, page)) {
+	if (!pte_write(pte) && gup_must_unshare(vma, flags, folio, page)) {
 		page = ERR_PTR(-EMLINK);
 		goto out;
 	}
@@ -2892,7 +2947,7 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
 			goto pte_unmap;
 		}
 
-		if (!pte_write(pte) && gup_must_unshare(NULL, flags, page)) {
+		if (!pte_write(pte) && gup_must_unshare(NULL, flags, folio, page)) {
 			gup_put_folio(folio, 1, flags);
 			goto pte_unmap;
 		}
@@ -2969,7 +3024,8 @@ static unsigned long gup_fast_pmd_leaf(pmd_t orig, pmd_t *pmdp,
 		gup_put_folio(folio, nr_pages, flags);
 		return 0;
 	}
-	if (!pmd_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
+	if (!pmd_write(orig) &&
+	    gup_must_unshare(NULL, flags, folio, &folio->page)) {
 		gup_put_folio(folio, nr_pages, flags);
 		return 0;
 	}
@@ -3011,7 +3067,8 @@ static unsigned long gup_fast_pud_leaf(pud_t orig, pud_t *pudp,
 		return 0;
 	}
 
-	if (!pud_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
+	if (!pud_write(orig) &&
+	    gup_must_unshare(NULL, flags, folio, &folio->page)) {
 		gup_put_folio(folio, nr_pages, flags);
 		return 0;
 	}
diff --git a/mm/internal.h b/mm/internal.h
index c53c369bf8b5d..35016b1afc8bf 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1282,6 +1282,10 @@ int migrate_device_coherent_folio(struct folio *folio);
 int __must_check try_grab_folio(struct folio *folio, int refs,
 				unsigned int flags);
 
+unsigned long gup_unshared_pages(struct vm_area_struct *vma, unsigned int flags,
+				 struct folio *folio, struct page *page,
+				 unsigned long nr_pages);
+
 /*
  * mm/huge_memory.c
  */
@@ -1354,51 +1358,12 @@ enum {
  * to fallback to the slow path just to lookup the vma.
  */
 static inline bool gup_must_unshare(struct vm_area_struct *vma,
-				    unsigned int flags, struct page *page)
+				    unsigned int flags,
+				    struct folio *folio, struct page *page)
 {
-	/*
-	 * FOLL_WRITE is implicitly handled correctly as the page table entry
-	 * has to be writable -- and if it references (part of) an anonymous
-	 * folio, that part is required to be marked exclusive.
-	 */
-	if ((flags & (FOLL_WRITE | FOLL_PIN)) != FOLL_PIN)
-		return false;
-	/*
-	 * Note: PageAnon(page) is stable until the page is actually getting
-	 * freed.
-	 */
-	if (!PageAnon(page)) {
-		/*
-		 * We only care about R/O long-term pining: R/O short-term
-		 * pinning does not have the semantics to observe successive
-		 * changes through the process page tables.
-		 */
-		if (!(flags & FOLL_LONGTERM))
-			return false;
-
-		/* We really need the vma ... */
-		if (!vma)
-			return true;
-
-		/*
-		 * ... because we only care about writable private ("COW")
-		 * mappings where we have to break COW early.
-		 */
-		return vma_is_cow_mapping(vma);
-	}
-
-	/* Paired with a memory barrier in folio_try_share_anon_rmap_*(). */
-	if (IS_ENABLED(CONFIG_HAVE_GUP_FAST))
-		smp_rmb();
-
-	/*
-	 * Note that KSM pages cannot be exclusive, and consequently,
-	 * cannot get pinned.
-	 */
-	return !PageAnonExclusive(page);
+	return gup_unshared_pages(vma, flags, folio, page, 1) != 1;
 }
 
-
 static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma)
 {
 	/*

-- 
2.43.0

[PATCH v2 2/4] mm: internal: fix inconsistent in folio_pte_batch_flags() for shadow stack

[Yeoreum Yun]

In x86, shadow stack PTE is marked with PAGE_RW=0 and PAGE_DIRTY=1.
If the shadow stack pte entry is consequence with the read-only pte
like PAGE_RW=0 and PAGE_DIRTY=0, folio_pte_batch_flags()
with FPB_RESPECT_WRITE without FPB_RESPECT_DIRTY considers both are
the same.

To address this, save the first pte's writable and compare subsequence
pte's writablity with pte_write() when FPB_RESPECT_WRITE is set
without FPB_RESPECT_DIRTY.

Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
 mm/internal.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/mm/internal.h b/mm/internal.h
index 35016b1afc8bf..b4d0df4c35adf 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -418,7 +418,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
 		struct vm_area_struct *vma, pte_t *ptep, pte_t *ptentp,
 		unsigned int max_nr, fpb_t flags)
 {
-	bool any_writable = false, any_young = false, any_dirty = false;
+	bool any_writable = false, any_young = false, any_dirty = false, writable;
 	pte_t expected_pte, pte = *ptentp;
 	unsigned int nr, cur_nr;
 
@@ -438,6 +438,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
 
 	nr = pte_batch_hint(ptep, pte);
 	expected_pte = __pte_batch_clear_ignored(pte_advance_pfn(pte, nr), flags);
+	writable = !!pte_write(pte);
 	ptep = ptep + nr;
 
 	while (nr < max_nr) {
@@ -446,6 +447,16 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
 		if (!pte_same(__pte_batch_clear_ignored(pte, flags), expected_pte))
 			break;
 
+		/*
+		 * x86 shadow-stack PTEs encode writability as RW=0, DIRTY=1.
+		 * Without FPB_RESPECT_DIRTY, __pte_batch_clear_ignored()
+		 * considers shadow-stack PTE as the same with subsequent
+		 * ordinary read-only PTEs. Therefore, check whether pte writable.
+		 */
+		if (unlikely((flags & (FPB_RESPECT_WRITE | FPB_RESPECT_DIRTY)) == FPB_RESPECT_WRITE &&
+			     writable != !!pte_write(pte)))
+			break;
+
 		if (flags & FPB_MERGE_WRITE)
 			any_writable |= pte_write(pte);
 		if (flags & FPB_MERGE_YOUNG_DIRTY) {

-- 
2.43.0

[PATCH v2 3/4] mm: internal: introduce FPB_LOCKLESS for folio_pte_batch_flags()

[Yeoreum Yun]

From: Yuan-Hao Hsu <aa9736195201@gmail.com>

Add support for calling folio_pte_batch_flags() in GUP-fast context when we are
not holding the PT lock. Instead of reading PTEs through ptep_get(), we have to
use ptep_get_lockless().

Specify the new behavior through a new flag FPB_LOCKLESS. We'll make use of this
functionality for batching PTEs in gup_fast_pte_range() next.

Signed-off-by: Yuan-Hao Hsu <aa9736195201@gmail.com>
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
 mm/internal.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/mm/internal.h b/mm/internal.h
index b4d0df4c35adf..c1824b0193f4d 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -373,6 +373,12 @@ typedef int __bitwise fpb_t;
  */
 #define FPB_MERGE_YOUNG_DIRTY		((__force fpb_t)BIT(4))
 
+/*
+ * Read the page table entries with ptep_get_lockless(): the caller does not
+ * hold the page table lock (e.g, GUP-fast).
+ */
+#define FPB_LOCKLESS			((__force fpb_t)BIT(5))
+
 static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
 {
 	if (!(flags & FPB_RESPECT_DIRTY))
@@ -442,7 +448,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
 	ptep = ptep + nr;
 
 	while (nr < max_nr) {
-		pte = ptep_get(ptep);
+		pte = (flags & FPB_LOCKLESS) ? ptep_get_lockless(ptep) : ptep_get(ptep);
 
 		if (!pte_same(__pte_batch_clear_ignored(pte, flags), expected_pte))
 			break;

-- 
2.43.0

[PATCH v2 4/4] mm: gup: batch PTE-mapped large folios in gup_fast_pte_range()

[Yeoreum Yun]

GUP-fast grabs a PTE-mapped large folio one page at a time.  Every PTE
costs a try_grab_folio_fast() (a refcount cmpxchg, plus the pincount
atomic, a full barrier and a node stat update for FOLL_PIN), a
gup_fast_folio_allowed() and a folio_set_referenced(), so a 64 kB mTHP
pays sixteen of each and a PTE-mapped 2 MB THP pays 512.

The users are the hot ones: iov_iter_extract_user_pages() pins every
O_DIRECT buffer through pin_user_pages_fast(), io_uring registers its
buffers through it, and so does RDMA memory registration.
With mTHP those buffers are PTE-mapped large folios.

After the head page has been grabbed and verified as before, extend the
run to the following PTEs that map the next pages of the same folio
with the same protection bits, take the remaining references for the
run with try_grab_folio(), and fill pages[] from the run.
The PTEs of the run are read while the reference on the folio is held,
so the folio can neither be collapsed, freed nor split underneath the scan.
A read-only run of an anonymous folio stops at the first page
that gup_must_unshare() rejects, so the per-page exclusivity guarantee for
FOLL_PIN is kept.

folio_pte_batch_flags() does the scan.  It reads the entries with
ptep_get(), which is not safe without the page table lock on
CONFIG_GUP_GET_PXX_LOW_HIGH (x86 PAE, mips32, sh) or with arm64 contpte,
and GUP-fast reads every PTE with ptep_get_lockless() for that reason.
Add FPB_LOCKLESS to make the helper do the same; it is inlined per call
site, so the other callers do not change.

The order-0 path keeps its instruction sequence.  The folio flags are
tested before folio_set_referenced() on purpose: a load of the flags
right after that locked instruction cost about 25% on order-0 pages in
the gup_test benchmark below, and reading them first brings that path
back to the baseline.

gup_test ioctl over a 256 MB anonymous region, FOLL_WRITE, 65536 pages
per call, median of 15 runs, two baseline boots / one patched boot, on
an x86-64 VM:

                       get_user_pages_fast()   pin_user_pages_fast()
                       before       after      before       after
  4 kB pages           849/789 us   783 us     1174/1138 us 1094 us
  64 kB mTHP           812/811 us   126 us     1215/1113 us  188 us
  1 MB mTHP            799/774 us    59 us     1185/1077 us   64 us
  2 MB THP, PTE-mapped 803/786 us    57 us     1159/1120 us   58 us
  2 MB THP, PMD-mapped  42/40 us     40 us       46/41 us     42 us

fio O_DIRECT randread of a null_blk device (CPU bound) with the I/O
buffer in 64 kB mTHP, median of 15 five-second runs:

  bs=1M psync                 126.2/125.6 GB/s  ->  220.6 GB/s
  bs=1M io_uring, iodepth 16   71.2/71.6 GB/s   ->  112.1 GB/s
  bs=64k io_uring, iodepth 16  50.2/50.7 GB/s   ->   60.1 GB/s

IORING_REGISTER_BUFFERS of 1 GB, median of 15:

  64 kB mTHP           6179/5998 us  ->  1599 us
  1 MB mTHP            5790/5801 us  ->   893 us
  4 kB pages           7173/6741 us  ->  6763 us

Signed-off-by: Yuan-Hao Hsu <aa9736195201@gmail.com>
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
 mm/gup.c | 46 +++++++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index 49681f63950ec..94a9dc45e5953 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2901,7 +2901,8 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
 		unsigned long addr, unsigned long end,
 		unsigned int flags, struct page **pages)
 {
-	unsigned long nr_pages = 0;
+	unsigned long nr_pages = 0, nr_batch;
+	struct folio *folio = NULL;
 	pte_t *ptep, *ptem;
 
 	ptem = ptep = pte_offset_map(&pmd, addr);
@@ -2910,7 +2911,9 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
 	do {
 		pte_t pte = ptep_get_lockless(ptep);
 		struct page *page;
-		struct folio *folio;
+		unsigned long i;
+
+		nr_batch = 1;
 
 		/*
 		 * Always fallback to ordinary GUP on PROT_NONE-mapped pages:
@@ -2937,19 +2940,21 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
 			goto pte_unmap;
 
 		if (unlikely(pmd_val(pmd) != pmd_val(pmdp_get_lockless(pmdp))) ||
-		    unlikely(pte_val(pte) != pte_val(ptep_get_lockless(ptep)))) {
-			gup_put_folio(folio, 1, flags);
+		    unlikely(pte_val(pte) != pte_val(ptep_get_lockless(ptep))))
 			goto pte_unmap;
-		}
 
-		if (!gup_fast_folio_allowed(folio, flags)) {
-			gup_put_folio(folio, 1, flags);
+		if (!gup_fast_folio_allowed(folio, flags))
 			goto pte_unmap;
-		}
 
-		if (!pte_write(pte) && gup_must_unshare(NULL, flags, folio, page)) {
-			gup_put_folio(folio, 1, flags);
-			goto pte_unmap;
+		if (folio_test_large(folio))
+			nr_batch = folio_pte_batch_flags(folio, NULL, ptep, &pte,
+							 (end - addr) >> PAGE_SHIFT,
+							 FPB_LOCKLESS | FPB_RESPECT_WRITE);
+
+		if (!pte_write(pte)) {
+			nr_batch = gup_unshared_pages(NULL, flags, folio, page, nr_batch);
+			if (nr_batch == 0)
+				goto pte_unmap;
 		}
 
 		/*
@@ -2958,16 +2963,23 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
 		 * see Documentation/core-api/pin_user_pages.rst for
 		 * details.
 		 */
-		if ((flags & FOLL_PIN) && arch_make_folio_accessible(folio)) {
-			gup_put_folio(folio, 1, flags);
+		if ((flags & FOLL_PIN) && arch_make_folio_accessible(folio))
 			goto pte_unmap;
-		}
+
+		if (nr_batch > 1 && try_grab_folio(folio, nr_batch - 1, flags))
+			nr_batch = 1;
+
 		folio_set_referenced(folio);
-		pages[nr_pages] = page;
-		nr_pages++;
-	} while (ptep++, addr += PAGE_SIZE, addr != end);
+		for (i = 0; i < nr_batch; i++) {
+			pages[nr_pages + i] = page + i;
+		}
+		nr_pages += nr_batch;
+		folio = NULL;
+	} while (ptep += nr_batch, addr += nr_batch * PAGE_SIZE, addr != end);
 
 pte_unmap:
+	if (folio)
+		gup_put_folio(folio, 1, flags);
 	pte_unmap(ptem);
 	return nr_pages;
 }

-- 
2.43.0

Re: [PATCH v2 2/4] mm: internal: fix inconsistent in folio_pte_batch_flags() for shadow stack

[David Hildenbrand (Arm)]

On 11/2/26 12:30, Yeoreum Yun wrote:
> In x86, shadow stack PTE is marked with PAGE_RW=0 and PAGE_DIRTY=1.
> If the shadow stack pte entry is consequence with the read-only pte
> like PAGE_RW=0 and PAGE_DIRTY=0, folio_pte_batch_flags()
> with FPB_RESPECT_WRITE without FPB_RESPECT_DIRTY considers both are
> the same.
> 
> To address this, save the first pte's writable and compare subsequence
> pte's writablity with pte_write() when FPB_RESPECT_WRITE is set
> without FPB_RESPECT_DIRTY.


I think this should be patch #1 and likely carry a Fixes: tag.

I think we can already create an inconsistency by using a mixture of

* MADV_HUGEPAGE (overwrite for nohugepage for shadow stack)
* MADV_DONTFORK + fork() + MADV_DOFORK
* FPB_RESPECT_WRITE users

Not that it would matter in practice.

> 
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
>  mm/internal.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 35016b1afc8bf..b4d0df4c35adf 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -418,7 +418,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
>  		struct vm_area_struct *vma, pte_t *ptep, pte_t *ptentp,
>  		unsigned int max_nr, fpb_t flags)
>  {
> -	bool any_writable = false, any_young = false, any_dirty = false;
> +	bool any_writable = false, any_young = false, any_dirty = false, writable;
>  	pte_t expected_pte, pte = *ptentp;
>  	unsigned int nr, cur_nr;
>  
> @@ -438,6 +438,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
>  
>  	nr = pte_batch_hint(ptep, pte);
>  	expected_pte = __pte_batch_clear_ignored(pte_advance_pfn(pte, nr), flags);
> +	writable = !!pte_write(pte);
>  	ptep = ptep + nr;
>  
>  	while (nr < max_nr) {
> @@ -446,6 +447,16 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
>  		if (!pte_same(__pte_batch_clear_ignored(pte, flags), expected_pte))
>  			break;
>  
> +		/*
> +		 * x86 shadow-stack PTEs encode writability as RW=0, DIRTY=1.
> +		 * Without FPB_RESPECT_DIRTY, __pte_batch_clear_ignored()
> +		 * considers shadow-stack PTE as the same with subsequent
> +		 * ordinary read-only PTEs. Therefore, check whether pte writable.
> +		 */

That information doesn't really belong in here in that detail level.

"/* pte_mkclean() can remove the writable bit in some cases. */"

> +		if (unlikely((flags & (FPB_RESPECT_WRITE | FPB_RESPECT_DIRTY)) == FPB_RESPECT_WRITE &&
> +			     writable != !!pte_write(pte)))

It might be nicer and more efficient to track if any was readonly/not-writable and

	any_unwritable = !pte_write(pte);

And checking once after the loop.


I checked and we cannot really fix x86 without an additional sw bit to remember
"shadow stack PTE".

As discussed, the simpler alternative would be to just disallow large folios in
shadow stacks entirely for now, given that vm_mmap_shadow_stack() already
disables large folios explicitly.

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 4b71d010b2aff..9a2dc90ef9690 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -367,6 +367,9 @@ int hugepage_madvise(struct vm_area_struct *vma,
 {
        switch (advice) {
        case MADV_HUGEPAGE:
+               /* PTE batching doesn't work reliably. */
+               if (vma_test_single_mask(vma, VMA_SHADOW_STACK))
+                       break;
                *vm_flags &= ~VM_NOHUGEPAGE;
                *vm_flags |= VM_HUGEPAGE;
                break;

-- 
Cheers,

David

Re: [PATCH v2 1/4] mm: gup: introduce gup_unshared_pages()

[David Hildenbrand (Arm)]

On 11/2/26 12:30, Yeoreum Yun wrote:
> This is preparatory patch to make gup_fast_pte_range() batchable.
> 
> Introduce gup_unshared_pages() to count consecutive pages within a folio
> that do not require unsharing, and use it to implement gup_must_unshare().
> 
> Also add a folio argument to gup_must_unshare() so callers can reuse an
> existing folio and avoid the compound_head() call implicit in PageAnon().
> 
> There is no functional change.
> 
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
>  mm/gup.c      | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
>  mm/internal.h | 49 ++++++------------------------------------
>  2 files changed, 70 insertions(+), 48 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index 7d43ce717b246..49681f63950ec 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -633,6 +633,59 @@ static struct page *no_page_table(struct vm_area_struct *vma,
>  	return NULL;
>  }
>  
> +unsigned long gup_unshared_pages(struct vm_area_struct *vma, unsigned int flags,
> +				 struct folio *folio, struct page *page,
> +				 unsigned long nr_pages)

Two tab indent.


This function should be static.

As a prerequisite patch you should move gup_unshared_pages() into this file.

$ git grep gup_must_unshare
include/linux/rmap.h:    * gup_must_unshare().
include/linux/rmap.h:    * gup_must_unshare().
mm/gup.c:       if (!pud_write(pud) && gup_must_unshare(vma, flags, page))
mm/gup.c:       if (!pmd_write(pmdval) && gup_must_unshare(vma, flags, page))
mm/gup.c:       if (!pte_write(pte) && gup_must_unshare(vma, flags, page)) {
mm/gup.c:               if (!pte_write(pte) && gup_must_unshare(NULL, flags, page)) {
mm/gup.c:       if (!pmd_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
mm/gup.c:       if (!pud_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
mm/internal.h:static inline bool gup_must_unshare(struct vm_area_struct *vma,


I'm not 100% happy about the function name due to the !folio_test_anon()+!FOLL_LONGTERM
path, but cannot think of something better.

> +{
> +	unsigned int i;
> +
> +	/*
> +	 * FOLL_WRITE is implicitly handled correctly as the page table entry
> +	 * has to be writable -- and if it references (part of) an anonymous
> +	 * folio, that part is required to be marked exclusive.
> +	 */
> +	if ((flags & (FOLL_WRITE | FOLL_PIN)) != FOLL_PIN)
> +		return nr_pages;
> +	/*
> +	 * Note: PageAnon(page) is stable until the page is actually getting

You are not calling PageAnon() anymore.

/* folio_test_anon(folio) is stable until folio gets freed. */

> +	 * freed.
> +	 */
> +	if (!folio_test_anon(folio)) {
> +		/*
> +		 * We only care about R/O long-term pining: R/O short-term
> +		 * pinning does not have the semantics to observe successive
> +		 * changes through the process page tables.
> +		 */
> +		if (!(flags & FOLL_LONGTERM))
> +			return nr_pages;
> +
> +		/* We really need the vma ... */
> +		if (!vma || vma_is_cow_mapping(vma))
> +			return 0;
> +
> +		/*
> +		 * ... because we only care about writable private ("COW")
> +		 * mappings where we have to break COW early.
> +		 */
> +		return nr_pages;
> +	}
> +
> +	/* Paired with a memory barrier in folio_try_share_anon_rmap_*(). */
> +	if (IS_ENABLED(CONFIG_HAVE_GUP_FAST))
> +		smp_rmb();
> +
> +	/*
> +	 * Note that KSM pages cannot be exclusive, and consequently,
> +	 * cannot get pinned.
> +	 */
> +	for (i = 0; i < nr_pages; i++) {
> +		if (!PageAnonExclusive(page + i))
> +			return i;
> +	}
> +
> +	return nr_pages;
> +}
> +
>  #ifdef CONFIG_PGTABLE_HAS_HUGE_LEAVES
>  /* FOLL_FORCE can write to even unwritable PUDs in COW mappings. */
>  static inline bool can_follow_write_pud(pud_t pud, struct page *page,
> @@ -668,7 +721,8 @@ static struct page *follow_huge_pud(struct vm_area_struct *vma,
>  	pfn += (addr & ~PUD_MASK) >> PAGE_SHIFT;
>  	page = pfn_to_page(pfn);
>  
> -	if (!pud_write(pud) && gup_must_unshare(vma, flags, page))
> +	if (!pud_write(pud) &&
> +	    gup_must_unshare(vma, flags, page_folio(page), page))
>  		return ERR_PTR(-EMLINK);

There is already another instance of page_folio(page) int his file. Use a local
variable and have one folio lookup. (that's the whole reason for passing a folio)

>  
>  	ret = try_grab_folio(page_folio(page), 1, flags);
> @@ -722,7 +776,8 @@ static struct page *follow_huge_pmd(struct vm_area_struct *vma,
>  	if (pmd_protnone(*pmd) && !gup_can_follow_protnone(vma, flags))
>  		return NULL;
>  
> -	if (!pmd_write(pmdval) && gup_must_unshare(vma, flags, page))
> +	if (!pmd_write(pmdval) &&
> +	    gup_must_unshare(vma, flags, page_folio(page), page))
>  		return ERR_PTR(-EMLINK);

Dito.

>  
>  	VM_WARN_ON_ONCE_PAGE((flags & FOLL_PIN) && PageAnon(page) &&
> @@ -846,7 +901,7 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
>  	}
>  	folio = page_folio(page);
>  
> -	if (!pte_write(pte) && gup_must_unshare(vma, flags, page)) {
> +	if (!pte_write(pte) && gup_must_unshare(vma, flags, folio, page)) {
>  		page = ERR_PTR(-EMLINK);
>  		goto out;
>  	}
> @@ -2892,7 +2947,7 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
>  			goto pte_unmap;
>  		}
>  
> -		if (!pte_write(pte) && gup_must_unshare(NULL, flags, page)) {
> +		if (!pte_write(pte) && gup_must_unshare(NULL, flags, folio, page)) {
>  			gup_put_folio(folio, 1, flags);
>  			goto pte_unmap;
>  		}
> @@ -2969,7 +3024,8 @@ static unsigned long gup_fast_pmd_leaf(pmd_t orig, pmd_t *pmdp,
>  		gup_put_folio(folio, nr_pages, flags);
>  		return 0;
>  	}
> -	if (!pmd_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
> +	if (!pmd_write(orig) &&
> +	    gup_must_unshare(NULL, flags, folio, &folio->page)) {


This only works because we don't have PMD mappings of things that are larger than PMDs.

While at it, pass the proper page.

>  		gup_put_folio(folio, nr_pages, flags);
>  		return 0;
>  	}
> @@ -3011,7 +3067,8 @@ static unsigned long gup_fast_pud_leaf(pud_t orig, pud_t *pudp,
>  		return 0;
>  	}
>  
> -	if (!pud_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
> +	if (!pud_write(orig) &&
> +	    gup_must_unshare(NULL, flags, folio, &folio->page)) {

Dito.

>  		gup_put_folio(folio, nr_pages, flags);
>  		return 0;
>  	}
> diff --git a/mm/internal.h b/mm/internal.h
> index c53c369bf8b5d..35016b1afc8bf 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1282,6 +1282,10 @@ int migrate_device_coherent_folio(struct folio *folio);
>  int __must_check try_grab_folio(struct folio *folio, int refs,
>  				unsigned int flags);
>  
> +unsigned long gup_unshared_pages(struct vm_area_struct *vma, unsigned int flags,
> +				 struct folio *folio, struct page *page,
> +				 unsigned long nr_pages);
> +
>  /*
>   * mm/huge_memory.c
>   */
> @@ -1354,51 +1358,12 @@ enum {
>   * to fallback to the slow path just to lookup the vma.
>   */
>  static inline bool gup_must_unshare(struct vm_area_struct *vma,
> -				    unsigned int flags, struct page *page)
> +				    unsigned int flags,
> +				    struct folio *folio, struct page *page)

Whatever you touch, please also convert to two-tab indent.

-- 
Cheers,

David

Re: [PATCH v2 3/4] mm: internal: introduce FPB_LOCKLESS for folio_pte_batch_flags()

[David Hildenbrand (Arm)]

On 11/2/26 12:30, Yeoreum Yun wrote:
> From: Yuan-Hao Hsu <aa9736195201@gmail.com>
> 
> Add support for calling folio_pte_batch_flags() in GUP-fast context when we are
> not holding the PT lock. Instead of reading PTEs through ptep_get(), we have to
> use ptep_get_lockless().
> 
> Specify the new behavior through a new flag FPB_LOCKLESS. We'll make use of this
> functionality for batching PTEs in gup_fast_pte_range() next.
> 
> Signed-off-by: Yuan-Hao Hsu <aa9736195201@gmail.com>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
>  mm/internal.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index b4d0df4c35adf..c1824b0193f4d 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -373,6 +373,12 @@ typedef int __bitwise fpb_t;
>   */
>  #define FPB_MERGE_YOUNG_DIRTY		((__force fpb_t)BIT(4))
>  
> +/*
> + * Read the page table entries with ptep_get_lockless(): the caller does not
> + * hold the page table lock (e.g, GUP-fast).
> + */
> +#define FPB_LOCKLESS			((__force fpb_t)BIT(5))
> +
>  static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
>  {
>  	if (!(flags & FPB_RESPECT_DIRTY))
> @@ -442,7 +448,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
>  	ptep = ptep + nr;
>  
>  	while (nr < max_nr) {
> -		pte = ptep_get(ptep);
> +		pte = (flags & FPB_LOCKLESS) ? ptep_get_lockless(ptep) : ptep_get(ptep);
>  
>  		if (!pte_same(__pte_batch_clear_ignored(pte, flags), expected_pte))
>  			break;
> 

Acked-by: David Hildenbrand (Arm) <david@kernel.org>

-- 
Cheers,

David

Re: [PATCH v2 2/4] mm: internal: fix inconsistent in folio_pte_batch_flags() for shadow stack

[Yeoreum Yun]

On Mon, Nov 02, 2026 at 01:10:21PM +0100, David Hildenbrand (Arm) wrote:
> On 11/2/26 12:30, Yeoreum Yun wrote:
> > In x86, shadow stack PTE is marked with PAGE_RW=0 and PAGE_DIRTY=1.
> > If the shadow stack pte entry is consequence with the read-only pte
> > like PAGE_RW=0 and PAGE_DIRTY=0, folio_pte_batch_flags()
> > with FPB_RESPECT_WRITE without FPB_RESPECT_DIRTY considers both are
> > the same.
> > 
> > To address this, save the first pte's writable and compare subsequence
> > pte's writablity with pte_write() when FPB_RESPECT_WRITE is set
> > without FPB_RESPECT_DIRTY.
> 
> 
> I think this should be patch #1 and likely carry a Fixes: tag.

I see. I'll move and add Fixes tag.

> 
> I think we can already create an inconsistency by using a mixture of
> 
> * MADV_HUGEPAGE (overwrite for nohugepage for shadow stack)
> * MADV_DONTFORK + fork() + MADV_DOFORK
> * FPB_RESPECT_WRITE users
> 
> Not that it would matter in practice.

True. but it's possible anyway... without alternative suggestion.

> 
> > 
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
> >  mm/internal.h | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/internal.h b/mm/internal.h
> > index 35016b1afc8bf..b4d0df4c35adf 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -418,7 +418,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
> >  		struct vm_area_struct *vma, pte_t *ptep, pte_t *ptentp,
> >  		unsigned int max_nr, fpb_t flags)
> >  {
> > -	bool any_writable = false, any_young = false, any_dirty = false;
> > +	bool any_writable = false, any_young = false, any_dirty = false, writable;
> >  	pte_t expected_pte, pte = *ptentp;
> >  	unsigned int nr, cur_nr;
> >  
> > @@ -438,6 +438,7 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
> >  
> >  	nr = pte_batch_hint(ptep, pte);
> >  	expected_pte = __pte_batch_clear_ignored(pte_advance_pfn(pte, nr), flags);
> > +	writable = !!pte_write(pte);
> >  	ptep = ptep + nr;
> >  
> >  	while (nr < max_nr) {
> > @@ -446,6 +447,16 @@ static inline unsigned int folio_pte_batch_flags(struct folio *folio,
> >  		if (!pte_same(__pte_batch_clear_ignored(pte, flags), expected_pte))
> >  			break;
> >  
> > +		/*
> > +		 * x86 shadow-stack PTEs encode writability as RW=0, DIRTY=1.
> > +		 * Without FPB_RESPECT_DIRTY, __pte_batch_clear_ignored()
> > +		 * considers shadow-stack PTE as the same with subsequent
> > +		 * ordinary read-only PTEs. Therefore, check whether pte writable.
> > +		 */
> 
> That information doesn't really belong in here in that detail level.
> "/* pte_mkclean() can remove the writable bit in some cases. */"
> 

I see. I'll change with simple as you suggest.

> > +		if (unlikely((flags & (FPB_RESPECT_WRITE | FPB_RESPECT_DIRTY)) == FPB_RESPECT_WRITE &&
> > +			     writable != !!pte_write(pte)))
> 
> It might be nicer and more efficient to track if any was readonly/not-writable and
> 
> 	any_unwritable = !pte_write(pte);
> 
> And checking once after the loop.

Sorry. But I couldn't follow. If we accumalate with any_unwritable
and check it out of loop once only, we couldn't properly nr value or
Am I missing something?

> 
> 
> I checked and we cannot really fix x86 without an additional sw bit to remember
> "shadow stack PTE".

Agree.

> 
> As discussed, the simpler alternative would be to just disallow large folios in
> shadow stacks entirely for now, given that vm_mmap_shadow_stack() already
> disables large folios explicitly.
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 4b71d010b2aff..9a2dc90ef9690 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -367,6 +367,9 @@ int hugepage_madvise(struct vm_area_struct *vma,
>  {
>         switch (advice) {
>         case MADV_HUGEPAGE:
> +               /* PTE batching doesn't work reliably. */
> +               if (vma_test_single_mask(vma, VMA_SHADOW_STACK))
> +                       break;
>                 *vm_flags &= ~VM_NOHUGEPAGE;
>                 *vm_flags |= VM_HUGEPAGE;
>                 break;
> 

Yeap. once we do this, we don't need to this fix patch anyway.
But I don't know whether there is practical case changing the
VMA_SHADOW_STACK with huge though it is initailised with NOHUGE.

> -- 
> Cheers,
> 
> David

-- 
Sincerely,
Yeoreum Yun

Re: [PATCH v2 1/4] mm: gup: introduce gup_unshared_pages()

[Yeoreum Yun]

> On 11/2/26 12:30, Yeoreum Yun wrote:
> > This is preparatory patch to make gup_fast_pte_range() batchable.
> > 
> > Introduce gup_unshared_pages() to count consecutive pages within a folio
> > that do not require unsharing, and use it to implement gup_must_unshare().
> > 
> > Also add a folio argument to gup_must_unshare() so callers can reuse an
> > existing folio and avoid the compound_head() call implicit in PageAnon().
> > 
> > There is no functional change.
> > 
> > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > ---
> >  mm/gup.c      | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
> >  mm/internal.h | 49 ++++++------------------------------------
> >  2 files changed, 70 insertions(+), 48 deletions(-)
> > 
> > diff --git a/mm/gup.c b/mm/gup.c
> > index 7d43ce717b246..49681f63950ec 100644
> > --- a/mm/gup.c
> > +++ b/mm/gup.c
> > @@ -633,6 +633,59 @@ static struct page *no_page_table(struct vm_area_struct *vma,
> >  	return NULL;
> >  }
> >  
> > +unsigned long gup_unshared_pages(struct vm_area_struct *vma, unsigned int flags,
> > +				 struct folio *folio, struct page *page,
> > +				 unsigned long nr_pages)
> 
> Two tab indent.

Okay.

> 
> 
> This function should be static.

Acked.

> 
> As a prerequisite patch you should move gup_unshared_pages() into this file.
> 
> $ git grep gup_must_unshare
> include/linux/rmap.h:    * gup_must_unshare().
> include/linux/rmap.h:    * gup_must_unshare().
> mm/gup.c:       if (!pud_write(pud) && gup_must_unshare(vma, flags, page))
> mm/gup.c:       if (!pmd_write(pmdval) && gup_must_unshare(vma, flags, page))
> mm/gup.c:       if (!pte_write(pte) && gup_must_unshare(vma, flags, page)) {
> mm/gup.c:               if (!pte_write(pte) && gup_must_unshare(NULL, flags, page)) {
> mm/gup.c:       if (!pmd_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
> mm/gup.c:       if (!pud_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
> mm/internal.h:static inline bool gup_must_unshare(struct vm_area_struct *vma,

Might you want move gup_must_unshared() into gup.c from internal.h
I'll do.

> 
> 
> I'm not 100% happy about the function name due to the !folio_test_anon()+!FOLL_LONGTERM
> path, but cannot think of something better.

True.

> 
> > +{
> > +	unsigned int i;
> > +
> > +	/*
> > +	 * FOLL_WRITE is implicitly handled correctly as the page table entry
> > +	 * has to be writable -- and if it references (part of) an anonymous
> > +	 * folio, that part is required to be marked exclusive.
> > +	 */
> > +	if ((flags & (FOLL_WRITE | FOLL_PIN)) != FOLL_PIN)
> > +		return nr_pages;
> > +	/*
> > +	 * Note: PageAnon(page) is stable until the page is actually getting
> 
> You are not calling PageAnon() anymore.

Yes. I'll remove it.

> /* folio_test_anon(folio) is stable until folio gets freed. */
> 
> > +	 * freed.
> > +	 */
> > +	if (!folio_test_anon(folio)) {
> > +		/*
> > +		 * We only care about R/O long-term pining: R/O short-term
> > +		 * pinning does not have the semantics to observe successive
> > +		 * changes through the process page tables.
> > +		 */
> > +		if (!(flags & FOLL_LONGTERM))
> > +			return nr_pages;
> > +
> > +		/* We really need the vma ... */
> > +		if (!vma || vma_is_cow_mapping(vma))
> > +			return 0;
> > +
> > +		/*
> > +		 * ... because we only care about writable private ("COW")
> > +		 * mappings where we have to break COW early.
> > +		 */
> > +		return nr_pages;
> > +	}
> > +
> > +	/* Paired with a memory barrier in folio_try_share_anon_rmap_*(). */
> > +	if (IS_ENABLED(CONFIG_HAVE_GUP_FAST))
> > +		smp_rmb();
> > +
> > +	/*
> > +	 * Note that KSM pages cannot be exclusive, and consequently,
> > +	 * cannot get pinned.
> > +	 */
> > +	for (i = 0; i < nr_pages; i++) {
> > +		if (!PageAnonExclusive(page + i))
> > +			return i;
> > +	}
> > +
> > +	return nr_pages;
> > +}
> > +
> >  #ifdef CONFIG_PGTABLE_HAS_HUGE_LEAVES
> >  /* FOLL_FORCE can write to even unwritable PUDs in COW mappings. */
> >  static inline bool can_follow_write_pud(pud_t pud, struct page *page,
> > @@ -668,7 +721,8 @@ static struct page *follow_huge_pud(struct vm_area_struct *vma,
> >  	pfn += (addr & ~PUD_MASK) >> PAGE_SHIFT;
> >  	page = pfn_to_page(pfn);
> >  
> > -	if (!pud_write(pud) && gup_must_unshare(vma, flags, page))
> > +	if (!pud_write(pud) &&
> > +	    gup_must_unshare(vma, flags, page_folio(page), page))
> >  		return ERR_PTR(-EMLINK);
> 
> There is already another instance of page_folio(page) int his file. Use a local
> variable and have one folio lookup. (that's the whole reason for passing a folio)

Okay. I'll merge.

> 
> >  
> >  	ret = try_grab_folio(page_folio(page), 1, flags);
> > @@ -722,7 +776,8 @@ static struct page *follow_huge_pmd(struct vm_area_struct *vma,
> >  	if (pmd_protnone(*pmd) && !gup_can_follow_protnone(vma, flags))
> >  		return NULL;
> >  
> > -	if (!pmd_write(pmdval) && gup_must_unshare(vma, flags, page))
> > +	if (!pmd_write(pmdval) &&
> > +	    gup_must_unshare(vma, flags, page_folio(page), page))
> >  		return ERR_PTR(-EMLINK);
> 
> Dito.

Acked.

> 
> >  
> >  	VM_WARN_ON_ONCE_PAGE((flags & FOLL_PIN) && PageAnon(page) &&
> > @@ -846,7 +901,7 @@ static struct page *follow_page_pte(struct vm_area_struct *vma,
> >  	}
> >  	folio = page_folio(page);
> >  
> > -	if (!pte_write(pte) && gup_must_unshare(vma, flags, page)) {
> > +	if (!pte_write(pte) && gup_must_unshare(vma, flags, folio, page)) {
> >  		page = ERR_PTR(-EMLINK);
> >  		goto out;
> >  	}
> > @@ -2892,7 +2947,7 @@ static unsigned long gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp,
> >  			goto pte_unmap;
> >  		}
> >  
> > -		if (!pte_write(pte) && gup_must_unshare(NULL, flags, page)) {
> > +		if (!pte_write(pte) && gup_must_unshare(NULL, flags, folio, page)) {
> >  			gup_put_folio(folio, 1, flags);
> >  			goto pte_unmap;
> >  		}
> > @@ -2969,7 +3024,8 @@ static unsigned long gup_fast_pmd_leaf(pmd_t orig, pmd_t *pmdp,
> >  		gup_put_folio(folio, nr_pages, flags);
> >  		return 0;
> >  	}
> > -	if (!pmd_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
> > +	if (!pmd_write(orig) &&
> > +	    gup_must_unshare(NULL, flags, folio, &folio->page)) {
> 
> 
> This only works because we don't have PMD mappings of things that are larger than PMDs.
> 
> While at it, pass the proper page.

Okay. I'll change with pmd_page(orig).

> 
> >  		gup_put_folio(folio, nr_pages, flags);
> >  		return 0;
> >  	}
> > @@ -3011,7 +3067,8 @@ static unsigned long gup_fast_pud_leaf(pud_t orig, pud_t *pudp,
> >  		return 0;
> >  	}
> >  
> > -	if (!pud_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) {
> > +	if (!pud_write(orig) &&
> > +	    gup_must_unshare(NULL, flags, folio, &folio->page)) {
> 
> Dito.

Acked.

> 
> >  		gup_put_folio(folio, nr_pages, flags);
> >  		return 0;
> >  	}
> > diff --git a/mm/internal.h b/mm/internal.h
> > index c53c369bf8b5d..35016b1afc8bf 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -1282,6 +1282,10 @@ int migrate_device_coherent_folio(struct folio *folio);
> >  int __must_check try_grab_folio(struct folio *folio, int refs,
> >  				unsigned int flags);
> >  
> > +unsigned long gup_unshared_pages(struct vm_area_struct *vma, unsigned int flags,
> > +				 struct folio *folio, struct page *page,
> > +				 unsigned long nr_pages);
> > +
> >  /*
> >   * mm/huge_memory.c
> >   */
> > @@ -1354,51 +1358,12 @@ enum {
> >   * to fallback to the slow path just to lookup the vma.
> >   */
> >  static inline bool gup_must_unshare(struct vm_area_struct *vma,
> > -				    unsigned int flags, struct page *page)
> > +				    unsigned int flags,
> > +				    struct folio *folio, struct page *page)
> 
> Whatever you touch, please also convert to two-tab indent.

Okay.

> 
> -- 
> Cheers,
> 
> David

-- 
Sincerely,
Yeoreum Yun

Re: [PATCH v2 2/4] mm: internal: fix inconsistent in folio_pte_batch_flags() for shadow stack

[David Hildenbrand (Arm)]

>>> +		if (unlikely((flags & (FPB_RESPECT_WRITE | FPB_RESPECT_DIRTY)) == FPB_RESPECT_WRITE &&
>>> +			     writable != !!pte_write(pte)))
>>
>> It might be nicer and more efficient to track if any was readonly/not-writable and
>>
>> 	any_unwritable = !pte_write(pte);
>>
>> And checking once after the loop.
> 
> Sorry. But I couldn't follow. If we accumalate with any_unwritable
> and check it out of loop once only, we couldn't properly nr value or
> Am I missing something?

Ah. The idea was to simply reject the batch entirely if that happens. It would
only matter if some folio PTEs are writable while others are not ... well, ok,
there are some cases where that can happen, but it's not really the common case.

I think we should simply disallow ever getting large folios into shadow stack
VMAs for now. So I'd prefer the fix below.

Thoughts?

>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 4b71d010b2aff..9a2dc90ef9690 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -367,6 +367,9 @@ int hugepage_madvise(struct vm_area_struct *vma,
>>  {
>>         switch (advice) {
>>         case MADV_HUGEPAGE:
>> +               /* PTE batching doesn't work reliably. */
>> +               if (vma_test_single_mask(vma, VMA_SHADOW_STACK))
>> +                       break;
>>                 *vm_flags &= ~VM_NOHUGEPAGE;
>>                 *vm_flags |= VM_HUGEPAGE;
>>                 break;
>>
> 
> Yeap. once we do this, we don't need to this fix patch anyway.
> But I don't know whether there is practical case changing the
> VMA_SHADOW_STACK with huge though it is initailised with NOHUGE.



-- 
Cheers,

David

Re: [PATCH v2 2/4] mm: internal: fix inconsistent in folio_pte_batch_flags() for shadow stack

[Yeoreum Yun]

> >>> +		if (unlikely((flags & (FPB_RESPECT_WRITE | FPB_RESPECT_DIRTY)) == FPB_RESPECT_WRITE &&
> >>> +			     writable != !!pte_write(pte)))
> >>
> >> It might be nicer and more efficient to track if any was readonly/not-writable and
> >>
> >> 	any_unwritable = !pte_write(pte);
> >>
> >> And checking once after the loop.
> > 
> > Sorry. But I couldn't follow. If we accumalate with any_unwritable
> > and check it out of loop once only, we couldn't properly nr value or
> > Am I missing something?
> 
> Ah. The idea was to simply reject the batch entirely if that happens. It would
> only matter if some folio PTEs are writable while others are not ... well, ok,
> there are some cases where that can happen, but it's not really the common case.
> 
> I think we should simply disallow ever getting large folios into shadow stack
> VMAs for now. So I'd prefer the fix below.
> 
> Thoughts?

That's my preference too and we've discussed :)
I'll repost with below unless anyone has any strong comment.


> 
> >>
> >> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >> index 4b71d010b2aff..9a2dc90ef9690 100644
> >> --- a/mm/khugepaged.c
> >> +++ b/mm/khugepaged.c
> >> @@ -367,6 +367,9 @@ int hugepage_madvise(struct vm_area_struct *vma,
> >>  {
> >>         switch (advice) {
> >>         case MADV_HUGEPAGE:
> >> +               /* PTE batching doesn't work reliably. */
> >> +               if (vma_test_single_mask(vma, VMA_SHADOW_STACK))
> >> +                       break;
> >>                 *vm_flags &= ~VM_NOHUGEPAGE;
> >>                 *vm_flags |= VM_HUGEPAGE;
> >>                 break;
> >>
> > 
> > Yeap. once we do this, we don't need to this fix patch anyway.
> > But I don't know whether there is practical case changing the
> > VMA_SHADOW_STACK with huge though it is initailised with NOHUGE.
> 
> 
> 
> -- 
> Cheers,
> 
> David

-- 
Sincerely,
Yeoreum Yun

Re: [PATCH v2 2/4] mm: internal: fix inconsistent in folio_pte_batch_flags() for shadow stack

[David Hildenbrand (Arm)]

On 11/2/26 15:07, Yeoreum Yun wrote:
>>>
>>> Sorry. But I couldn't follow. If we accumalate with any_unwritable
>>> and check it out of loop once only, we couldn't properly nr value or
>>> Am I missing something?
>>
>> Ah. The idea was to simply reject the batch entirely if that happens. It would
>> only matter if some folio PTEs are writable while others are not ... well, ok,
>> there are some cases where that can happen, but it's not really the common case.
>>
>> I think we should simply disallow ever getting large folios into shadow stack
>> VMAs for now. So I'd prefer the fix below.
>>
>> Thoughts?
> 
> That's my preference too and we've discussed :)
> I'll repost with below unless anyone has any strong comment.

Yes, best to wait a couple of days for people to catch up.

-- 
Cheers,

David