Now that the rmap overhaul[1] is upstream that provides a clean interface
for rmap batching, let's implement PTE batching during fork when processing
PTE-mapped THPs.
This series is partially based on Ryan's previous work[2] to implement
cont-pte support on arm64, but its a complete rewrite based on [1] to
optimize all architectures independent of any such PTE bits, and to
use the new rmap batching functions that simplify the code and prepare
for further rmap accounting changes.
We collect consecutive PTEs that map consecutive pages of the same large
folio, making sure that the other PTE bits are compatible, and (a) adjust
the refcount only once per batch, (b) call rmap handling functions only
once per batch and (c) perform batch PTE setting/updates.
While this series should be beneficial for adding cont-pte support on
ARM64[2], it's one of the requirements for maintaining a total mapcount[3]
for large folios with minimal added overhead and further changes[4] that
build up on top of the total mapcount.
Independent of all that, this series results in a speedup during fork with
PTE-mapped THP, which is the default with THPs that are smaller than a PMD
(for example, 16KiB to 1024KiB mTHPs for anonymous memory[5]).
On an Intel Xeon Silver 4210R CPU, fork'ing with 1GiB of PTE-mapped folios
of the same size (stddev < 1%) results in the following runtimes
for fork() (shorter is better):
Folio Size | v6.8-rc1 | New | Change
------------------------------------------
4KiB | 0.014328 | 0.014035 | - 2%
16KiB | 0.014263 | 0.01196 | -16%
32KiB | 0.014334 | 0.01094 | -24%
64KiB | 0.014046 | 0.010444 | -26%
128KiB | 0.014011 | 0.010063 | -28%
256KiB | 0.013993 | 0.009938 | -29%
512KiB | 0.013983 | 0.00985 | -30%
1024KiB | 0.013986 | 0.00982 | -30%
2048KiB | 0.014305 | 0.010076 | -30%
Note that these numbers are even better than the ones from v1 (verified
over multiple reboots), even though there were only minimal code changes.
Well, I removed a pte_mclean() call for anon folios, maybe that also
plays a role. Alternatively, either v1 or v2 is seriously buggy ;)
But my experience is that fork() is extremely sensitive to code size,
inlining, ... so I suspect we'll see on other architectures rather a change
of -20% instead of -30%, and it will be easy to "lose" some of that speedup
in the future by subtle code changes.
Next up is PTE batching when unmapping, that I'll send out next week.
Only tested on x86-64. Compile-tested on most other architectures.
v1 -> v2:
* "arm64/mm: Make set_ptes() robust when OAs cross 48-bit boundary"
-> Added patch from Ryan
* "arm/pgtable: define PFN_PTE_SHIFT"
-> Removed the arm64 bits
* "mm/pgtable: make pte_next_pfn() independent of set_ptes()"
* "arm/mm: use pte_next_pfn() in set_ptes()"
* "powerpc/mm: use pte_next_pfn() in set_ptes()"
-> Added to use pte_next_pfn() in some arch set_ptes() implementations
I tried to make use of pte_next_pfn() also in the others, but it's
not trivial because the other archs implement set_ptes() in their
asm/pgtable.h. Future work.
* "mm/memory: factor out copying the actual PTE in copy_present_pte()"
-> Move common folio_get() out of if/else
* "mm/memory: optimize fork() with PTE-mapped THP"
-> Add doc for wrprotect_ptes
-> Extend description to mention handling of pinned folios
-> Move common folio_ref_add() out of if/else
* "mm/memory: ignore dirty/accessed/soft-dirty bits in folio_pte_batch()"
-> Be more conservative with dirt/soft-dirty, let the caller specify
using flags
[1] https://lkml.kernel.org/r/[email protected]
[2] https://lkml.kernel.org/r/[email protected]
[3] https://lkml.kernel.org/r/[email protected]
[4] https://lkml.kernel.org/r/[email protected]
[5] https://lkml.kernel.org/r/[email protected]
Cc: Andrew Morton <[email protected]>
Cc: Matthew Wilcox (Oracle) <[email protected]>
Cc: Ryan Roberts <[email protected]>
Cc: Russell King <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: Dinh Nguyen <[email protected]>
Cc: Michael Ellerman <[email protected]>
Cc: Nicholas Piggin <[email protected]>
Cc: Christophe Leroy <[email protected]>
Cc: "Aneesh Kumar K.V" <[email protected]>
Cc: "Naveen N. Rao" <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Albert Ou <[email protected]>
Cc: Alexander Gordeev <[email protected]>
Cc: Gerald Schaefer <[email protected]>
Cc: Heiko Carstens <[email protected]>
Cc: Vasily Gorbik <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Cc: Sven Schnelle <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
David Hildenbrand (14):
arm/pgtable: define PFN_PTE_SHIFT
nios2/pgtable: define PFN_PTE_SHIFT
powerpc/pgtable: define PFN_PTE_SHIFT
riscv/pgtable: define PFN_PTE_SHIFT
s390/pgtable: define PFN_PTE_SHIFT
sparc/pgtable: define PFN_PTE_SHIFT
mm/pgtable: make pte_next_pfn() independent of set_ptes()
arm/mm: use pte_next_pfn() in set_ptes()
powerpc/mm: use pte_next_pfn() in set_ptes()
mm/memory: factor out copying the actual PTE in copy_present_pte()
mm/memory: pass PTE to copy_present_pte()
mm/memory: optimize fork() with PTE-mapped THP
mm/memory: ignore dirty/accessed/soft-dirty bits in folio_pte_batch()
mm/memory: ignore writable bit in folio_pte_batch()
Ryan Roberts (1):
arm64/mm: Make set_ptes() robust when OAs cross 48-bit boundary
arch/arm/include/asm/pgtable.h | 2 +
arch/arm/mm/mmu.c | 2 +-
arch/arm64/include/asm/pgtable.h | 28 ++--
arch/nios2/include/asm/pgtable.h | 2 +
arch/powerpc/include/asm/pgtable.h | 2 +
arch/powerpc/mm/pgtable.c | 5 +-
arch/riscv/include/asm/pgtable.h | 2 +
arch/s390/include/asm/pgtable.h | 2 +
arch/sparc/include/asm/pgtable_64.h | 2 +
include/linux/pgtable.h | 33 ++++-
mm/memory.c | 212 ++++++++++++++++++++++------
11 files changed, 229 insertions(+), 63 deletions(-)
--
2.43.0
We want to make use of pte_next_pfn() outside of set_ptes(). Let's
simply define PFN_PTE_SHIFT, required by pte_next_pfn().
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/nios2/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/nios2/include/asm/pgtable.h b/arch/nios2/include/asm/pgtable.h
index 5144506dfa693..d052dfcbe8d3a 100644
--- a/arch/nios2/include/asm/pgtable.h
+++ b/arch/nios2/include/asm/pgtable.h
@@ -178,6 +178,8 @@ static inline void set_pte(pte_t *ptep, pte_t pteval)
*ptep = pteval;
}
+#define PFN_PTE_SHIFT 0
+
static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, unsigned int nr)
{
--
2.43.0
We want to make use of pte_next_pfn() outside of set_ptes(). Let's
simply define PFN_PTE_SHIFT, required by pte_next_pfn().
Reviewed-by: Alexandre Ghiti <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/riscv/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 0c94260b5d0c1..add5cd30ab34d 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -523,6 +523,8 @@ static inline void __set_pte_at(pte_t *ptep, pte_t pteval)
set_pte(ptep, pteval);
}
+#define PFN_PTE_SHIFT _PAGE_PFN_SHIFT
+
static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pteval, unsigned int nr)
{
--
2.43.0
We want to make use of pte_next_pfn() outside of set_ptes(). Let's
simply define PFN_PTE_SHIFT, required by pte_next_pfn().
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/s390/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/s390/include/asm/pgtable.h b/arch/s390/include/asm/pgtable.h
index 1299b56e43f6f..4b91e65c85d97 100644
--- a/arch/s390/include/asm/pgtable.h
+++ b/arch/s390/include/asm/pgtable.h
@@ -1316,6 +1316,8 @@ pgprot_t pgprot_writecombine(pgprot_t prot);
#define pgprot_writethrough pgprot_writethrough
pgprot_t pgprot_writethrough(pgprot_t prot);
+#define PFN_PTE_SHIFT PAGE_SHIFT
+
/*
* Set multiple PTEs to consecutive pages with a single call. All PTEs
* are within the same folio, PMD and VMA.
--
2.43.0
We want to make use of pte_next_pfn() outside of set_ptes(). Let's
simply define PFN_PTE_SHIFT, required by pte_next_pfn().
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/sparc/include/asm/pgtable_64.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/sparc/include/asm/pgtable_64.h b/arch/sparc/include/asm/pgtable_64.h
index a8c871b7d7860..652af9d63fa29 100644
--- a/arch/sparc/include/asm/pgtable_64.h
+++ b/arch/sparc/include/asm/pgtable_64.h
@@ -929,6 +929,8 @@ static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
maybe_tlb_batch_add(mm, addr, ptep, orig, fullmm, PAGE_SHIFT);
}
+#define PFN_PTE_SHIFT PAGE_SHIFT
+
static inline void set_ptes(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, unsigned int nr)
{
--
2.43.0
Let's provide pte_next_pfn(), independently of set_ptes(). This allows for
using the generic pte_next_pfn() version in some arch-specific set_ptes()
implementations, and prepares for reusing pte_next_pfn() in other context.
Signed-off-by: David Hildenbrand <[email protected]>
---
include/linux/pgtable.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index f6d0e3513948a..351cd9dc7194f 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -212,7 +212,6 @@ static inline int pmd_dirty(pmd_t pmd)
#define arch_flush_lazy_mmu_mode() do {} while (0)
#endif
-#ifndef set_ptes
#ifndef pte_next_pfn
static inline pte_t pte_next_pfn(pte_t pte)
@@ -221,6 +220,7 @@ static inline pte_t pte_next_pfn(pte_t pte)
}
#endif
+#ifndef set_ptes
/**
* set_ptes - Map consecutive pages to a contiguous range of addresses.
* @mm: Address space to map the pages into.
--
2.43.0
Let's use our handy helper now that it's available on all archs.
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/arm/mm/mmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 674ed71573a84..c24e29c0b9a48 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1814,6 +1814,6 @@ void set_ptes(struct mm_struct *mm, unsigned long addr,
if (--nr == 0)
break;
ptep++;
- pte_val(pteval) += PAGE_SIZE;
+ pteval = pte_next_pfn(pteval);
}
}
--
2.43.0
Let's prepare for further changes.
Reviewed-by: Ryan Roberts <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
mm/memory.c | 63 ++++++++++++++++++++++++++++-------------------------
1 file changed, 33 insertions(+), 30 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 7e1f4849463aa..10fc14ff8e49b 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -930,6 +930,29 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
return 0;
}
+static inline void __copy_present_pte(struct vm_area_struct *dst_vma,
+ struct vm_area_struct *src_vma, pte_t *dst_pte, pte_t *src_pte,
+ pte_t pte, unsigned long addr)
+{
+ struct mm_struct *src_mm = src_vma->vm_mm;
+
+ /* If it's a COW mapping, write protect it both processes. */
+ if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte)) {
+ ptep_set_wrprotect(src_mm, addr, src_pte);
+ pte = pte_wrprotect(pte);
+ }
+
+ /* If it's a shared mapping, mark it clean in the child. */
+ if (src_vma->vm_flags & VM_SHARED)
+ pte = pte_mkclean(pte);
+ pte = pte_mkold(pte);
+
+ if (!userfaultfd_wp(dst_vma))
+ pte = pte_clear_uffd_wp(pte);
+
+ set_pte_at(dst_vma->vm_mm, addr, dst_pte, pte);
+}
+
/*
* Copy one pte. Returns 0 if succeeded, or -EAGAIN if one preallocated page
* is required to copy this pte.
@@ -939,23 +962,23 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
pte_t *dst_pte, pte_t *src_pte, unsigned long addr, int *rss,
struct folio **prealloc)
{
- struct mm_struct *src_mm = src_vma->vm_mm;
- unsigned long vm_flags = src_vma->vm_flags;
pte_t pte = ptep_get(src_pte);
struct page *page;
struct folio *folio;
page = vm_normal_page(src_vma, addr, pte);
- if (page)
- folio = page_folio(page);
- if (page && folio_test_anon(folio)) {
+ if (unlikely(!page))
+ goto copy_pte;
+
+ folio = page_folio(page);
+ folio_get(folio);
+ if (folio_test_anon(folio)) {
/*
* If this page may have been pinned by the parent process,
* copy the page immediately for the child so that we'll always
* guarantee the pinned page won't be randomly replaced in the
* future.
*/
- folio_get(folio);
if (unlikely(folio_try_dup_anon_rmap_pte(folio, page, src_vma))) {
/* Page may be pinned, we have to copy. */
folio_put(folio);
@@ -963,34 +986,14 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
addr, rss, prealloc, page);
}
rss[MM_ANONPAGES]++;
- } else if (page) {
- folio_get(folio);
+ VM_WARN_ON_FOLIO(PageAnonExclusive(page), folio);
+ } else {
folio_dup_file_rmap_pte(folio, page);
rss[mm_counter_file(page)]++;
}
- /*
- * If it's a COW mapping, write protect it both
- * in the parent and the child
- */
- if (is_cow_mapping(vm_flags) && pte_write(pte)) {
- ptep_set_wrprotect(src_mm, addr, src_pte);
- pte = pte_wrprotect(pte);
- }
- VM_BUG_ON(page && folio_test_anon(folio) && PageAnonExclusive(page));
-
- /*
- * If it's a shared mapping, mark it clean in
- * the child
- */
- if (vm_flags & VM_SHARED)
- pte = pte_mkclean(pte);
- pte = pte_mkold(pte);
-
- if (!userfaultfd_wp(dst_vma))
- pte = pte_clear_uffd_wp(pte);
-
- set_pte_at(dst_vma->vm_mm, addr, dst_pte, pte);
+copy_pte:
+ __copy_present_pte(dst_vma, src_vma, dst_pte, src_pte, pte, addr);
return 0;
}
--
2.43.0
Let's implement PTE batching when consecutive (present) PTEs map
consecutive pages of the same large folio, and all other PTE bits besides
the PFNs are equal.
We will optimize folio_pte_batch() separately, to ignore selected
PTE bits. This patch is based on work by Ryan Roberts.
Use __always_inline for __copy_present_ptes() and keep the handling for
single PTEs completely separate from the multi-PTE case: we really want
the compiler to optimize for the single-PTE case with small folios, to
not degrade performance.
Note that PTE batching will never exceed a single page table and will
always stay within VMA boundaries.
Further, processing PTE-mapped THP that maybe pinned and have
PageAnonExclusive set on at least one subpage should work as expected,
but there is room for improvement: We will repeatedly (1) detect a PTE
batch (2) detect that we have to copy a page (3) fall back and allocate a
single page to copy a single page. For now we won't care as pinned pages
are a corner case, and we should rather look into maintaining only a
single PageAnonExclusive bit for large folios.
Signed-off-by: David Hildenbrand <[email protected]>
---
include/linux/pgtable.h | 31 +++++++++++
mm/memory.c | 112 +++++++++++++++++++++++++++++++++-------
2 files changed, 124 insertions(+), 19 deletions(-)
diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
index 351cd9dc7194f..891ed246978a4 100644
--- a/include/linux/pgtable.h
+++ b/include/linux/pgtable.h
@@ -650,6 +650,37 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addres
}
#endif
+#ifndef wrprotect_ptes
+/**
+ * wrprotect_ptes - Write-protect consecutive pages that are mapped to a
+ * contiguous range of addresses.
+ * @mm: Address space to map the pages into.
+ * @addr: Address the first page is mapped at.
+ * @ptep: Page table pointer for the first entry.
+ * @nr: Number of pages to write-protect.
+ *
+ * May be overridden by the architecture; otherwise, implemented as a simple
+ * loop over ptep_set_wrprotect().
+ *
+ * Note that PTE bits in the PTE range besides the PFN can differ. For example,
+ * some PTEs might already be write-protected.
+ *
+ * Context: The caller holds the page table lock. The pages all belong
+ * to the same folio. The PTEs are all in the same PMD.
+ */
+static inline void wrprotect_ptes(struct mm_struct *mm, unsigned long addr,
+ pte_t *ptep, unsigned int nr)
+{
+ for (;;) {
+ ptep_set_wrprotect(mm, addr, ptep);
+ if (--nr == 0)
+ break;
+ ptep++;
+ addr += PAGE_SIZE;
+ }
+}
+#endif
+
/*
* On some architectures hardware does not set page access bit when accessing
* memory page, it is responsibility of software setting this bit. It brings
diff --git a/mm/memory.c b/mm/memory.c
index 729ca4d6a820c..4d1be89a01ee0 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -930,15 +930,15 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
return 0;
}
-static inline void __copy_present_pte(struct vm_area_struct *dst_vma,
+static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma,
struct vm_area_struct *src_vma, pte_t *dst_pte, pte_t *src_pte,
- pte_t pte, unsigned long addr)
+ pte_t pte, unsigned long addr, int nr)
{
struct mm_struct *src_mm = src_vma->vm_mm;
/* If it's a COW mapping, write protect it both processes. */
if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte)) {
- ptep_set_wrprotect(src_mm, addr, src_pte);
+ wrprotect_ptes(src_mm, addr, src_pte, nr);
pte = pte_wrprotect(pte);
}
@@ -950,26 +950,93 @@ static inline void __copy_present_pte(struct vm_area_struct *dst_vma,
if (!userfaultfd_wp(dst_vma))
pte = pte_clear_uffd_wp(pte);
- set_pte_at(dst_vma->vm_mm, addr, dst_pte, pte);
+ set_ptes(dst_vma->vm_mm, addr, dst_pte, pte, nr);
+}
+
+/*
+ * Detect a PTE batch: consecutive (present) PTEs that map consecutive
+ * pages of the same folio.
+ *
+ * All PTEs inside a PTE batch have the same PTE bits set, excluding the PFN.
+ */
+static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
+ pte_t *start_ptep, pte_t pte, int max_nr)
+{
+ unsigned long folio_end_pfn = folio_pfn(folio) + folio_nr_pages(folio);
+ const pte_t *end_ptep = start_ptep + max_nr;
+ pte_t expected_pte = pte_next_pfn(pte);
+ pte_t *ptep = start_ptep + 1;
+
+ VM_WARN_ON_FOLIO(!pte_present(pte), folio);
+
+ while (ptep != end_ptep) {
+ pte = ptep_get(ptep);
+
+ if (!pte_same(pte, expected_pte))
+ break;
+
+ /*
+ * Stop immediately once we reached the end of the folio. In
+ * corner cases the next PFN might fall into a different
+ * folio.
+ */
+ if (pte_pfn(pte) == folio_end_pfn)
+ break;
+
+ expected_pte = pte_next_pfn(expected_pte);
+ ptep++;
+ }
+
+ return ptep - start_ptep;
}
/*
- * Copy one pte. Returns 0 if succeeded, or -EAGAIN if one preallocated page
- * is required to copy this pte.
+ * Copy one present PTE, trying to batch-process subsequent PTEs that map
+ * consecutive pages of the same folio by copying them as well.
+ *
+ * Returns -EAGAIN if one preallocated page is required to copy the next PTE.
+ * Otherwise, returns the number of copied PTEs (at least 1).
*/
static inline int
-copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
+copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
pte_t *dst_pte, pte_t *src_pte, pte_t pte, unsigned long addr,
- int *rss, struct folio **prealloc)
+ int max_nr, int *rss, struct folio **prealloc)
{
struct page *page;
struct folio *folio;
+ int err, nr;
page = vm_normal_page(src_vma, addr, pte);
if (unlikely(!page))
goto copy_pte;
folio = page_folio(page);
+
+ /*
+ * If we likely have to copy, just don't bother with batching. Make
+ * sure that the common "small folio" case is as fast as possible
+ * by keeping the batching logic separate.
+ */
+ if (unlikely(!*prealloc && folio_test_large(folio) && max_nr != 1)) {
+ nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr);
+ folio_ref_add(folio, nr);
+ if (folio_test_anon(folio)) {
+ if (unlikely(folio_try_dup_anon_rmap_ptes(folio, page,
+ nr, src_vma))) {
+ folio_ref_sub(folio, nr);
+ return -EAGAIN;
+ }
+ rss[MM_ANONPAGES] += nr;
+ VM_WARN_ON_FOLIO(PageAnonExclusive(page), folio);
+ } else {
+ folio_dup_file_rmap_ptes(folio, page, nr);
+ rss[mm_counter_file(page)] += nr;
+ }
+ __copy_present_ptes(dst_vma, src_vma, dst_pte, src_pte, pte,
+ addr, nr);
+ return nr;
+ }
+
folio_get(folio);
if (folio_test_anon(folio)) {
/*
@@ -981,8 +1048,9 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
if (unlikely(folio_try_dup_anon_rmap_pte(folio, page, src_vma))) {
/* Page may be pinned, we have to copy. */
folio_put(folio);
- return copy_present_page(dst_vma, src_vma, dst_pte, src_pte,
- addr, rss, prealloc, page);
+ err = copy_present_page(dst_vma, src_vma, dst_pte, src_pte,
+ addr, rss, prealloc, page);
+ return err ? err : 1;
}
rss[MM_ANONPAGES]++;
VM_WARN_ON_FOLIO(PageAnonExclusive(page), folio);
@@ -992,8 +1060,8 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
}
copy_pte:
- __copy_present_pte(dst_vma, src_vma, dst_pte, src_pte, pte, addr);
- return 0;
+ __copy_present_ptes(dst_vma, src_vma, dst_pte, src_pte, pte, addr, 1);
+ return 1;
}
static inline struct folio *folio_prealloc(struct mm_struct *src_mm,
@@ -1030,10 +1098,11 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
pte_t *src_pte, *dst_pte;
pte_t ptent;
spinlock_t *src_ptl, *dst_ptl;
- int progress, ret = 0;
+ int progress, max_nr, ret = 0;
int rss[NR_MM_COUNTERS];
swp_entry_t entry = (swp_entry_t){0};
struct folio *prealloc = NULL;
+ int nr;
again:
progress = 0;
@@ -1064,6 +1133,8 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
arch_enter_lazy_mmu_mode();
do {
+ nr = 1;
+
/*
* We are holding two locks at this point - either of them
* could generate latencies in another task on another CPU.
@@ -1100,9 +1171,10 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
*/
WARN_ON_ONCE(ret != -ENOENT);
}
- /* copy_present_pte() will clear `*prealloc' if consumed */
- ret = copy_present_pte(dst_vma, src_vma, dst_pte, src_pte,
- ptent, addr, rss, &prealloc);
+ /* copy_present_ptes() will clear `*prealloc' if consumed */
+ max_nr = (end - addr) / PAGE_SIZE;
+ ret = copy_present_ptes(dst_vma, src_vma, dst_pte, src_pte,
+ ptent, addr, max_nr, rss, &prealloc);
/*
* If we need a pre-allocated page for this pte, drop the
* locks, allocate, and try again.
@@ -1119,8 +1191,10 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
folio_put(prealloc);
prealloc = NULL;
}
- progress += 8;
- } while (dst_pte++, src_pte++, addr += PAGE_SIZE, addr != end);
+ nr = ret;
+ progress += 8 * nr;
+ } while (dst_pte += nr, src_pte += nr, addr += PAGE_SIZE * nr,
+ addr != end);
arch_leave_lazy_mmu_mode();
pte_unmap_unlock(orig_src_pte, src_ptl);
@@ -1141,7 +1215,7 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
prealloc = folio_prealloc(src_mm, src_vma, addr, false);
if (!prealloc)
return -ENOMEM;
- } else if (ret) {
+ } else if (ret < 0) {
VM_WARN_ON_ONCE(1);
}
--
2.43.0
Let's always ignore the accessed/young bit: we'll always mark the PTE
as old in our child process during fork, and upcoming users will
similarly not care.
Ignore the dirty bit only if we don't want to duplicate the dirty bit
into the child process during fork. Maybe, we could just set all PTEs
in the child dirty if any PTE is dirty. For now, let's keep the behavior
unchanged, this can be optimized later if required.
Ignore the soft-dirty bit only if the bit doesn't have any meaning in
the src vma, and similarly won't have any in the copied dst vma.
For now, we won't bother with the uffd-wp bit.
Signed-off-by: David Hildenbrand <[email protected]>
---
mm/memory.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 4d1be89a01ee0..b3f035fe54c8d 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -953,24 +953,44 @@ static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma,
set_ptes(dst_vma->vm_mm, addr, dst_pte, pte, nr);
}
+/* Flags for folio_pte_batch(). */
+typedef int __bitwise fpb_t;
+
+/* Compare PTEs after pte_mkclean(), ignoring the dirty bit. */
+#define FPB_IGNORE_DIRTY ((__force fpb_t)BIT(0))
+
+/* Compare PTEs after pte_clear_soft_dirty(), ignoring the soft-dirty bit. */
+#define FPB_IGNORE_SOFT_DIRTY ((__force fpb_t)BIT(1))
+
+static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
+{
+ if (flags & FPB_IGNORE_DIRTY)
+ pte = pte_mkclean(pte);
+ if (likely(flags & FPB_IGNORE_SOFT_DIRTY))
+ pte = pte_clear_soft_dirty(pte);
+ return pte_mkold(pte);
+}
+
/*
* Detect a PTE batch: consecutive (present) PTEs that map consecutive
* pages of the same folio.
*
- * All PTEs inside a PTE batch have the same PTE bits set, excluding the PFN.
+ * All PTEs inside a PTE batch have the same PTE bits set, excluding the PFN,
+ * the accessed bit, dirty bit (with FPB_IGNORE_DIRTY) and soft-dirty bit
+ * (with FPB_IGNORE_SOFT_DIRTY).
*/
static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
- pte_t *start_ptep, pte_t pte, int max_nr)
+ pte_t *start_ptep, pte_t pte, int max_nr, fpb_t flags)
{
unsigned long folio_end_pfn = folio_pfn(folio) + folio_nr_pages(folio);
const pte_t *end_ptep = start_ptep + max_nr;
- pte_t expected_pte = pte_next_pfn(pte);
+ pte_t expected_pte = __pte_batch_clear_ignored(pte_next_pfn(pte), flags);
pte_t *ptep = start_ptep + 1;
VM_WARN_ON_FOLIO(!pte_present(pte), folio);
while (ptep != end_ptep) {
- pte = ptep_get(ptep);
+ pte = __pte_batch_clear_ignored(ptep_get(ptep), flags);
if (!pte_same(pte, expected_pte))
break;
@@ -1004,6 +1024,7 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
{
struct page *page;
struct folio *folio;
+ fpb_t flags = 0;
int err, nr;
page = vm_normal_page(src_vma, addr, pte);
@@ -1018,7 +1039,12 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
* by keeping the batching logic separate.
*/
if (unlikely(!*prealloc && folio_test_large(folio) && max_nr != 1)) {
- nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr);
+ if (src_vma->vm_flags & VM_SHARED)
+ flags |= FPB_IGNORE_DIRTY;
+ if (!vma_soft_dirty_enabled(src_vma))
+ flags |= FPB_IGNORE_SOFT_DIRTY;
+
+ nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr, flags);
folio_ref_add(folio, nr);
if (folio_test_anon(folio)) {
if (unlikely(folio_try_dup_anon_rmap_ptes(folio, page,
--
2.43.0
.. and conditionally return to the caller if any PTE except the first one
is writable. fork() has to make sure to properly write-protect in case any
PTE is writable. Other users (e.g., page unmaping) are expected to not
care.
Reviewed-by: Ryan Roberts <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
mm/memory.c | 30 ++++++++++++++++++++++++------
1 file changed, 24 insertions(+), 6 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index b3f035fe54c8d..64442da3df6d4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -968,7 +968,7 @@ static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
pte = pte_mkclean(pte);
if (likely(flags & FPB_IGNORE_SOFT_DIRTY))
pte = pte_clear_soft_dirty(pte);
- return pte_mkold(pte);
+ return pte_wrprotect(pte_mkold(pte));
}
/*
@@ -976,21 +976,32 @@ static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
* pages of the same folio.
*
* All PTEs inside a PTE batch have the same PTE bits set, excluding the PFN,
- * the accessed bit, dirty bit (with FPB_IGNORE_DIRTY) and soft-dirty bit
- * (with FPB_IGNORE_SOFT_DIRTY).
+ * the accessed bit, writable bit, dirty bit (with FPB_IGNORE_DIRTY) and
+ * soft-dirty bit (with FPB_IGNORE_SOFT_DIRTY).
+ *
+ * If "any_writable" is set, it will indicate if any other PTE besides the
+ * first (given) PTE is writable.
*/
static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
- pte_t *start_ptep, pte_t pte, int max_nr, fpb_t flags)
+ pte_t *start_ptep, pte_t pte, int max_nr, fpb_t flags,
+ bool *any_writable)
{
unsigned long folio_end_pfn = folio_pfn(folio) + folio_nr_pages(folio);
const pte_t *end_ptep = start_ptep + max_nr;
pte_t expected_pte = __pte_batch_clear_ignored(pte_next_pfn(pte), flags);
pte_t *ptep = start_ptep + 1;
+ bool writable;
+
+ if (any_writable)
+ *any_writable = false;
VM_WARN_ON_FOLIO(!pte_present(pte), folio);
while (ptep != end_ptep) {
- pte = __pte_batch_clear_ignored(ptep_get(ptep), flags);
+ pte = ptep_get(ptep);
+ if (any_writable)
+ writable = !!pte_write(pte);
+ pte = __pte_batch_clear_ignored(pte, flags);
if (!pte_same(pte, expected_pte))
break;
@@ -1003,6 +1014,9 @@ static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
if (pte_pfn(pte) == folio_end_pfn)
break;
+ if (any_writable)
+ *any_writable |= writable;
+
expected_pte = pte_next_pfn(expected_pte);
ptep++;
}
@@ -1024,6 +1038,7 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
{
struct page *page;
struct folio *folio;
+ bool any_writable;
fpb_t flags = 0;
int err, nr;
@@ -1044,7 +1059,8 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
if (!vma_soft_dirty_enabled(src_vma))
flags |= FPB_IGNORE_SOFT_DIRTY;
- nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr, flags);
+ nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr, flags,
+ &any_writable);
folio_ref_add(folio, nr);
if (folio_test_anon(folio)) {
if (unlikely(folio_try_dup_anon_rmap_ptes(folio, page,
@@ -1058,6 +1074,8 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
folio_dup_file_rmap_ptes(folio, page, nr);
rss[mm_counter_file(page)] += nr;
}
+ if (any_writable)
+ pte = pte_mkwrite(pte, src_vma);
__copy_present_ptes(dst_vma, src_vma, dst_pte, src_pte, pte,
addr, nr);
return nr;
--
2.43.0
Let's use our handy new helper. Note that the implementation is slightly
different, but shouldn't really make a difference in practice.
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/powerpc/mm/pgtable.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index a04ae4449a025..549a440ed7f65 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -220,10 +220,7 @@ void set_ptes(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
break;
ptep++;
addr += PAGE_SIZE;
- /*
- * increment the pfn.
- */
- pte = pfn_pte(pte_pfn(pte) + 1, pte_pgprot((pte)));
+ pte = pte_next_pfn(pte);
}
}
--
2.43.0
From: Ryan Roberts <[email protected]>
Since the high bits [51:48] of an OA are not stored contiguously in the
PTE, there is a theoretical bug in set_ptes(), which just adds PAGE_SIZE
to the pte to get the pte with the next pfn. This works until the pfn
crosses the 48-bit boundary, at which point we overflow into the upper
attributes.
Of course one could argue (and Matthew Wilcox has :) that we will never
see a folio cross this boundary because we only allow naturally aligned
power-of-2 allocation, so this would require a half-petabyte folio. So
its only a theoretical bug. But its better that the code is robust
regardless.
I've implemented pte_next_pfn() as part of the fix, which is an opt-in
core-mm interface. So that is now available to the core-mm, which will
be needed shortly to support forthcoming fork()-batching optimizations.
Link: https://lkml.kernel.org/r/[email protected]
Fixes: 4a169d61c2ed ("arm64: implement the new page table range API")
Closes: https://lore.kernel.org/linux-mm/[email protected]/
Signed-off-by: Ryan Roberts <[email protected]>
Reviewed-by: Catalin Marinas <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/arm64/include/asm/pgtable.h | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 79ce70fbb751c..52d0b0a763f16 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -341,6 +341,22 @@ static inline void __sync_cache_and_tags(pte_t pte, unsigned int nr_pages)
mte_sync_tags(pte, nr_pages);
}
+/*
+ * Select all bits except the pfn
+ */
+static inline pgprot_t pte_pgprot(pte_t pte)
+{
+ unsigned long pfn = pte_pfn(pte);
+
+ return __pgprot(pte_val(pfn_pte(pfn, __pgprot(0))) ^ pte_val(pte));
+}
+
+#define pte_next_pfn pte_next_pfn
+static inline pte_t pte_next_pfn(pte_t pte)
+{
+ return pfn_pte(pte_pfn(pte) + 1, pte_pgprot(pte));
+}
+
static inline void set_ptes(struct mm_struct *mm,
unsigned long __always_unused addr,
pte_t *ptep, pte_t pte, unsigned int nr)
@@ -354,7 +370,7 @@ static inline void set_ptes(struct mm_struct *mm,
if (--nr == 0)
break;
ptep++;
- pte_val(pte) += PAGE_SIZE;
+ pte = pte_next_pfn(pte);
}
}
#define set_ptes set_ptes
@@ -433,16 +449,6 @@ static inline pte_t pte_swp_clear_exclusive(pte_t pte)
return clear_pte_bit(pte, __pgprot(PTE_SWP_EXCLUSIVE));
}
-/*
- * Select all bits except the pfn
- */
-static inline pgprot_t pte_pgprot(pte_t pte)
-{
- unsigned long pfn = pte_pfn(pte);
-
- return __pgprot(pte_val(pfn_pte(pfn, __pgprot(0))) ^ pte_val(pte));
-}
-
#ifdef CONFIG_NUMA_BALANCING
/*
* See the comment in include/linux/pgtable.h
--
2.43.0
We want to make use of pte_next_pfn() outside of set_ptes(). Let's
simply define PFN_PTE_SHIFT, required by pte_next_pfn().
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/arm/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index d657b84b6bf70..be91e376df79e 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -209,6 +209,8 @@ static inline void __sync_icache_dcache(pte_t pteval)
extern void __sync_icache_dcache(pte_t pteval);
#endif
+#define PFN_PTE_SHIFT PAGE_SHIFT
+
void set_ptes(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pteval, unsigned int nr);
#define set_ptes set_ptes
--
2.43.0
We want to make use of pte_next_pfn() outside of set_ptes(). Let's
simply define PFN_PTE_SHIFT, required by pte_next_pfn().
Signed-off-by: David Hildenbrand <[email protected]>
---
arch/powerpc/include/asm/pgtable.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
index 9224f23065fff..7a1ba8889aeae 100644
--- a/arch/powerpc/include/asm/pgtable.h
+++ b/arch/powerpc/include/asm/pgtable.h
@@ -41,6 +41,8 @@ struct mm_struct;
#ifndef __ASSEMBLY__
+#define PFN_PTE_SHIFT PTE_RPN_SHIFT
+
void set_ptes(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
pte_t pte, unsigned int nr);
#define set_ptes set_ptes
--
2.43.0
We already read it, let's just forward it.
This patch is based on work by Ryan Roberts.
Reviewed-by: Ryan Roberts <[email protected]>
Signed-off-by: David Hildenbrand <[email protected]>
---
mm/memory.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/mm/memory.c b/mm/memory.c
index 10fc14ff8e49b..729ca4d6a820c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -959,10 +959,9 @@ static inline void __copy_present_pte(struct vm_area_struct *dst_vma,
*/
static inline int
copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
- pte_t *dst_pte, pte_t *src_pte, unsigned long addr, int *rss,
- struct folio **prealloc)
+ pte_t *dst_pte, pte_t *src_pte, pte_t pte, unsigned long addr,
+ int *rss, struct folio **prealloc)
{
- pte_t pte = ptep_get(src_pte);
struct page *page;
struct folio *folio;
@@ -1103,7 +1102,7 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
}
/* copy_present_pte() will clear `*prealloc' if consumed */
ret = copy_present_pte(dst_vma, src_vma, dst_pte, src_pte,
- addr, rss, &prealloc);
+ ptent, addr, rss, &prealloc);
/*
* If we need a pre-allocated page for this pte, drop the
* locks, allocate, and try again.
--
2.43.0
Le 25/01/2024 à 20:32, David Hildenbrand a écrit :
> We want to make use of pte_next_pfn() outside of set_ptes(). Let's
> simply define PFN_PTE_SHIFT, required by pte_next_pfn().
>
> Signed-off-by: David Hildenbrand <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/include/asm/pgtable.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/pgtable.h b/arch/powerpc/include/asm/pgtable.h
> index 9224f23065fff..7a1ba8889aeae 100644
> --- a/arch/powerpc/include/asm/pgtable.h
> +++ b/arch/powerpc/include/asm/pgtable.h
> @@ -41,6 +41,8 @@ struct mm_struct;
>
> #ifndef __ASSEMBLY__
>
> +#define PFN_PTE_SHIFT PTE_RPN_SHIFT
> +
> void set_ptes(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> pte_t pte, unsigned int nr);
> #define set_ptes set_ptes
Le 25/01/2024 à 20:32, David Hildenbrand a écrit :
> Let's use our handy new helper. Note that the implementation is slightly
> different, but shouldn't really make a difference in practice.
>
> Signed-off-by: David Hildenbrand <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/mm/pgtable.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index a04ae4449a025..549a440ed7f65 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -220,10 +220,7 @@ void set_ptes(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
> break;
> ptep++;
> addr += PAGE_SIZE;
> - /*
> - * increment the pfn.
> - */
> - pte = pfn_pte(pte_pfn(pte) + 1, pte_pgprot((pte)));
> + pte = pte_next_pfn(pte);
> }
> }
>
Le 25/01/2024 à 20:32, David Hildenbrand a écrit :
> Let's provide pte_next_pfn(), independently of set_ptes(). This allows for
> using the generic pte_next_pfn() version in some arch-specific set_ptes()
> implementations, and prepares for reusing pte_next_pfn() in other context.
>
> Signed-off-by: David Hildenbrand <[email protected]>
Reviewed-by: Christophe Leroy <[email protected]>
> ---
> include/linux/pgtable.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index f6d0e3513948a..351cd9dc7194f 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -212,7 +212,6 @@ static inline int pmd_dirty(pmd_t pmd)
> #define arch_flush_lazy_mmu_mode() do {} while (0)
> #endif
>
> -#ifndef set_ptes
>
> #ifndef pte_next_pfn
> static inline pte_t pte_next_pfn(pte_t pte)
> @@ -221,6 +220,7 @@ static inline pte_t pte_next_pfn(pte_t pte)
> }
> #endif
>
> +#ifndef set_ptes
> /**
> * set_ptes - Map consecutive pages to a contiguous range of addresses.
> * @mm: Address space to map the pages into.
On 25.01.24 20:32, David Hildenbrand wrote:
> Let's implement PTE batching when consecutive (present) PTEs map
> consecutive pages of the same large folio, and all other PTE bits besides
> the PFNs are equal.
>
> We will optimize folio_pte_batch() separately, to ignore selected
> PTE bits. This patch is based on work by Ryan Roberts.
>
> Use __always_inline for __copy_present_ptes() and keep the handling for
> single PTEs completely separate from the multi-PTE case: we really want
> the compiler to optimize for the single-PTE case with small folios, to
> not degrade performance.
>
> Note that PTE batching will never exceed a single page table and will
> always stay within VMA boundaries.
>
> Further, processing PTE-mapped THP that maybe pinned and have
> PageAnonExclusive set on at least one subpage should work as expected,
> but there is room for improvement: We will repeatedly (1) detect a PTE
> batch (2) detect that we have to copy a page (3) fall back and allocate a
> single page to copy a single page. For now we won't care as pinned pages
> are a corner case, and we should rather look into maintaining only a
> single PageAnonExclusive bit for large folios.
>
> Signed-off-by: David Hildenbrand <[email protected]>
> ---
> include/linux/pgtable.h | 31 +++++++++++
> mm/memory.c | 112 +++++++++++++++++++++++++++++++++-------
> 2 files changed, 124 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 351cd9dc7194f..891ed246978a4 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -650,6 +650,37 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addres
> }
> #endif
>
> +#ifndef wrprotect_ptes
> +/**
> + * wrprotect_ptes - Write-protect consecutive pages that are mapped to a
> + * contiguous range of addresses.
> + * @mm: Address space to map the pages into.
> + * @addr: Address the first page is mapped at.
> + * @ptep: Page table pointer for the first entry.
> + * @nr: Number of pages to write-protect.
> + *
> + * May be overridden by the architecture; otherwise, implemented as a simple
> + * loop over ptep_set_wrprotect().
> + *
> + * Note that PTE bits in the PTE range besides the PFN can differ. For example,
> + * some PTEs might already be write-protected.
> + *
> + * Context: The caller holds the page table lock. The pages all belong
> + * to the same folio. The PTEs are all in the same PMD.
> + */
After writing documentation for another two such functions, I'll change this to:
/**
* wrprotect_ptes - Write-protect PTEs that map consecutive pages of the same
* folio.
* @mm: Address space the pages are mapped into.
* @addr: Address the first page is mapped at.
* @ptep: Page table pointer for the first entry.
* @nr: Number of entries to write-protect.
*
* May be overridden by the architecture; otherwise, implemented as a simple
* loop over ptep_set_wrprotect().
*
* Note that PTE bits in the PTE range besides the PFN can differ. For example,
* some PTEs might be write-protected.
*
* Context: The caller holds the page table lock. The PTEs map consecutive
* pages that belong to the same folio. The PTEs are all in the same PMD.
*/
--
Cheers,
David / dhildenb
On 25/01/2024 19:32, David Hildenbrand wrote:
> Let's implement PTE batching when consecutive (present) PTEs map
> consecutive pages of the same large folio, and all other PTE bits besides
> the PFNs are equal.
>
> We will optimize folio_pte_batch() separately, to ignore selected
> PTE bits. This patch is based on work by Ryan Roberts.
>
> Use __always_inline for __copy_present_ptes() and keep the handling for
> single PTEs completely separate from the multi-PTE case: we really want
> the compiler to optimize for the single-PTE case with small folios, to
> not degrade performance.
>
> Note that PTE batching will never exceed a single page table and will
> always stay within VMA boundaries.
>
> Further, processing PTE-mapped THP that maybe pinned and have
> PageAnonExclusive set on at least one subpage should work as expected,
> but there is room for improvement: We will repeatedly (1) detect a PTE
> batch (2) detect that we have to copy a page (3) fall back and allocate a
> single page to copy a single page. For now we won't care as pinned pages
> are a corner case, and we should rather look into maintaining only a
> single PageAnonExclusive bit for large folios.
>
> Signed-off-by: David Hildenbrand <[email protected]>
Reviewed-by: Ryan Roberts <[email protected]>
> ---
> include/linux/pgtable.h | 31 +++++++++++
> mm/memory.c | 112 +++++++++++++++++++++++++++++++++-------
> 2 files changed, 124 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/pgtable.h b/include/linux/pgtable.h
> index 351cd9dc7194f..891ed246978a4 100644
> --- a/include/linux/pgtable.h
> +++ b/include/linux/pgtable.h
> @@ -650,6 +650,37 @@ static inline void ptep_set_wrprotect(struct mm_struct *mm, unsigned long addres
> }
> #endif
>
> +#ifndef wrprotect_ptes
> +/**
> + * wrprotect_ptes - Write-protect consecutive pages that are mapped to a
> + * contiguous range of addresses.
> + * @mm: Address space to map the pages into.
> + * @addr: Address the first page is mapped at.
> + * @ptep: Page table pointer for the first entry.
> + * @nr: Number of pages to write-protect.
> + *
> + * May be overridden by the architecture; otherwise, implemented as a simple
> + * loop over ptep_set_wrprotect().
> + *
> + * Note that PTE bits in the PTE range besides the PFN can differ. For example,
> + * some PTEs might already be write-protected.
> + *
> + * Context: The caller holds the page table lock. The pages all belong
> + * to the same folio. The PTEs are all in the same PMD.
> + */
> +static inline void wrprotect_ptes(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, unsigned int nr)
> +{
> + for (;;) {
> + ptep_set_wrprotect(mm, addr, ptep);
> + if (--nr == 0)
> + break;
> + ptep++;
> + addr += PAGE_SIZE;
> + }
> +}
> +#endif
> +
> /*
> * On some architectures hardware does not set page access bit when accessing
> * memory page, it is responsibility of software setting this bit. It brings
> diff --git a/mm/memory.c b/mm/memory.c
> index 729ca4d6a820c..4d1be89a01ee0 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -930,15 +930,15 @@ copy_present_page(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
> return 0;
> }
>
> -static inline void __copy_present_pte(struct vm_area_struct *dst_vma,
> +static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma,
> struct vm_area_struct *src_vma, pte_t *dst_pte, pte_t *src_pte,
> - pte_t pte, unsigned long addr)
> + pte_t pte, unsigned long addr, int nr)
> {
> struct mm_struct *src_mm = src_vma->vm_mm;
>
> /* If it's a COW mapping, write protect it both processes. */
> if (is_cow_mapping(src_vma->vm_flags) && pte_write(pte)) {
> - ptep_set_wrprotect(src_mm, addr, src_pte);
> + wrprotect_ptes(src_mm, addr, src_pte, nr);
> pte = pte_wrprotect(pte);
> }
>
> @@ -950,26 +950,93 @@ static inline void __copy_present_pte(struct vm_area_struct *dst_vma,
> if (!userfaultfd_wp(dst_vma))
> pte = pte_clear_uffd_wp(pte);
>
> - set_pte_at(dst_vma->vm_mm, addr, dst_pte, pte);
> + set_ptes(dst_vma->vm_mm, addr, dst_pte, pte, nr);
> +}
> +
> +/*
> + * Detect a PTE batch: consecutive (present) PTEs that map consecutive
> + * pages of the same folio.
> + *
> + * All PTEs inside a PTE batch have the same PTE bits set, excluding the PFN.
> + */
> +static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
> + pte_t *start_ptep, pte_t pte, int max_nr)
> +{
> + unsigned long folio_end_pfn = folio_pfn(folio) + folio_nr_pages(folio);
> + const pte_t *end_ptep = start_ptep + max_nr;
> + pte_t expected_pte = pte_next_pfn(pte);
> + pte_t *ptep = start_ptep + 1;
> +
> + VM_WARN_ON_FOLIO(!pte_present(pte), folio);
> +
> + while (ptep != end_ptep) {
> + pte = ptep_get(ptep);
> +
> + if (!pte_same(pte, expected_pte))
> + break;
> +
> + /*
> + * Stop immediately once we reached the end of the folio. In
> + * corner cases the next PFN might fall into a different
> + * folio.
> + */
> + if (pte_pfn(pte) == folio_end_pfn)
> + break;
> +
> + expected_pte = pte_next_pfn(expected_pte);
> + ptep++;
> + }
> +
> + return ptep - start_ptep;
> }
>
> /*
> - * Copy one pte. Returns 0 if succeeded, or -EAGAIN if one preallocated page
> - * is required to copy this pte.
> + * Copy one present PTE, trying to batch-process subsequent PTEs that map
> + * consecutive pages of the same folio by copying them as well.
> + *
> + * Returns -EAGAIN if one preallocated page is required to copy the next PTE.
> + * Otherwise, returns the number of copied PTEs (at least 1).
> */
> static inline int
> -copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> +copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> pte_t *dst_pte, pte_t *src_pte, pte_t pte, unsigned long addr,
> - int *rss, struct folio **prealloc)
> + int max_nr, int *rss, struct folio **prealloc)
> {
> struct page *page;
> struct folio *folio;
> + int err, nr;
>
> page = vm_normal_page(src_vma, addr, pte);
> if (unlikely(!page))
> goto copy_pte;
>
> folio = page_folio(page);
> +
> + /*
> + * If we likely have to copy, just don't bother with batching. Make
> + * sure that the common "small folio" case is as fast as possible
> + * by keeping the batching logic separate.
> + */
> + if (unlikely(!*prealloc && folio_test_large(folio) && max_nr != 1)) {
> + nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr);
> + folio_ref_add(folio, nr);
> + if (folio_test_anon(folio)) {
> + if (unlikely(folio_try_dup_anon_rmap_ptes(folio, page,
> + nr, src_vma))) {
> + folio_ref_sub(folio, nr);
> + return -EAGAIN;
> + }
> + rss[MM_ANONPAGES] += nr;
> + VM_WARN_ON_FOLIO(PageAnonExclusive(page), folio);
> + } else {
> + folio_dup_file_rmap_ptes(folio, page, nr);
> + rss[mm_counter_file(page)] += nr;
> + }
> + __copy_present_ptes(dst_vma, src_vma, dst_pte, src_pte, pte,
> + addr, nr);
> + return nr;
> + }
> +
> folio_get(folio);
> if (folio_test_anon(folio)) {
> /*
> @@ -981,8 +1048,9 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> if (unlikely(folio_try_dup_anon_rmap_pte(folio, page, src_vma))) {
> /* Page may be pinned, we have to copy. */
> folio_put(folio);
> - return copy_present_page(dst_vma, src_vma, dst_pte, src_pte,
> - addr, rss, prealloc, page);
> + err = copy_present_page(dst_vma, src_vma, dst_pte, src_pte,
> + addr, rss, prealloc, page);
> + return err ? err : 1;
> }
> rss[MM_ANONPAGES]++;
> VM_WARN_ON_FOLIO(PageAnonExclusive(page), folio);
> @@ -992,8 +1060,8 @@ copy_present_pte(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> }
>
> copy_pte:
> - __copy_present_pte(dst_vma, src_vma, dst_pte, src_pte, pte, addr);
> - return 0;
> + __copy_present_ptes(dst_vma, src_vma, dst_pte, src_pte, pte, addr, 1);
> + return 1;
> }
>
> static inline struct folio *folio_prealloc(struct mm_struct *src_mm,
> @@ -1030,10 +1098,11 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> pte_t *src_pte, *dst_pte;
> pte_t ptent;
> spinlock_t *src_ptl, *dst_ptl;
> - int progress, ret = 0;
> + int progress, max_nr, ret = 0;
> int rss[NR_MM_COUNTERS];
> swp_entry_t entry = (swp_entry_t){0};
> struct folio *prealloc = NULL;
> + int nr;
>
> again:
> progress = 0;
> @@ -1064,6 +1133,8 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> arch_enter_lazy_mmu_mode();
>
> do {
> + nr = 1;
> +
> /*
> * We are holding two locks at this point - either of them
> * could generate latencies in another task on another CPU.
> @@ -1100,9 +1171,10 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> */
> WARN_ON_ONCE(ret != -ENOENT);
> }
> - /* copy_present_pte() will clear `*prealloc' if consumed */
> - ret = copy_present_pte(dst_vma, src_vma, dst_pte, src_pte,
> - ptent, addr, rss, &prealloc);
> + /* copy_present_ptes() will clear `*prealloc' if consumed */
> + max_nr = (end - addr) / PAGE_SIZE;
> + ret = copy_present_ptes(dst_vma, src_vma, dst_pte, src_pte,
> + ptent, addr, max_nr, rss, &prealloc);
> /*
> * If we need a pre-allocated page for this pte, drop the
> * locks, allocate, and try again.
> @@ -1119,8 +1191,10 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> folio_put(prealloc);
> prealloc = NULL;
> }
> - progress += 8;
> - } while (dst_pte++, src_pte++, addr += PAGE_SIZE, addr != end);
> + nr = ret;
> + progress += 8 * nr;
> + } while (dst_pte += nr, src_pte += nr, addr += PAGE_SIZE * nr,
> + addr != end);
>
> arch_leave_lazy_mmu_mode();
> pte_unmap_unlock(orig_src_pte, src_ptl);
> @@ -1141,7 +1215,7 @@ copy_pte_range(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma,
> prealloc = folio_prealloc(src_mm, src_vma, addr, false);
> if (!prealloc)
> return -ENOMEM;
> - } else if (ret) {
> + } else if (ret < 0) {
> VM_WARN_ON_ONCE(1);
> }
>
On 25/01/2024 19:32, David Hildenbrand wrote:
> Let's always ignore the accessed/young bit: we'll always mark the PTE
> as old in our child process during fork, and upcoming users will
> similarly not care.
>
> Ignore the dirty bit only if we don't want to duplicate the dirty bit
> into the child process during fork. Maybe, we could just set all PTEs
> in the child dirty if any PTE is dirty. For now, let's keep the behavior
> unchanged, this can be optimized later if required.
>
> Ignore the soft-dirty bit only if the bit doesn't have any meaning in
> the src vma, and similarly won't have any in the copied dst vma.
>
> For now, we won't bother with the uffd-wp bit.
>
> Signed-off-by: David Hildenbrand <[email protected]>
Reviewed-by: Ryan Roberts <[email protected]>
> ---
> mm/memory.c | 36 +++++++++++++++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 4d1be89a01ee0..b3f035fe54c8d 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -953,24 +953,44 @@ static __always_inline void __copy_present_ptes(struct vm_area_struct *dst_vma,
> set_ptes(dst_vma->vm_mm, addr, dst_pte, pte, nr);
> }
>
> +/* Flags for folio_pte_batch(). */
> +typedef int __bitwise fpb_t;
> +
> +/* Compare PTEs after pte_mkclean(), ignoring the dirty bit. */
> +#define FPB_IGNORE_DIRTY ((__force fpb_t)BIT(0))
> +
> +/* Compare PTEs after pte_clear_soft_dirty(), ignoring the soft-dirty bit. */
> +#define FPB_IGNORE_SOFT_DIRTY ((__force fpb_t)BIT(1))
> +
> +static inline pte_t __pte_batch_clear_ignored(pte_t pte, fpb_t flags)
> +{
> + if (flags & FPB_IGNORE_DIRTY)
> + pte = pte_mkclean(pte);
> + if (likely(flags & FPB_IGNORE_SOFT_DIRTY))
> + pte = pte_clear_soft_dirty(pte);
> + return pte_mkold(pte);
> +}
> +
> /*
> * Detect a PTE batch: consecutive (present) PTEs that map consecutive
> * pages of the same folio.
> *
> - * All PTEs inside a PTE batch have the same PTE bits set, excluding the PFN.
> + * All PTEs inside a PTE batch have the same PTE bits set, excluding the PFN,
> + * the accessed bit, dirty bit (with FPB_IGNORE_DIRTY) and soft-dirty bit
> + * (with FPB_IGNORE_SOFT_DIRTY).
> */
> static inline int folio_pte_batch(struct folio *folio, unsigned long addr,
> - pte_t *start_ptep, pte_t pte, int max_nr)
> + pte_t *start_ptep, pte_t pte, int max_nr, fpb_t flags)
> {
> unsigned long folio_end_pfn = folio_pfn(folio) + folio_nr_pages(folio);
> const pte_t *end_ptep = start_ptep + max_nr;
> - pte_t expected_pte = pte_next_pfn(pte);
> + pte_t expected_pte = __pte_batch_clear_ignored(pte_next_pfn(pte), flags);
> pte_t *ptep = start_ptep + 1;
>
> VM_WARN_ON_FOLIO(!pte_present(pte), folio);
>
> while (ptep != end_ptep) {
> - pte = ptep_get(ptep);
> + pte = __pte_batch_clear_ignored(ptep_get(ptep), flags);
>
> if (!pte_same(pte, expected_pte))
> break;
> @@ -1004,6 +1024,7 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
> {
> struct page *page;
> struct folio *folio;
> + fpb_t flags = 0;
> int err, nr;
>
> page = vm_normal_page(src_vma, addr, pte);
> @@ -1018,7 +1039,12 @@ copy_present_ptes(struct vm_area_struct *dst_vma, struct vm_area_struct *src_vma
> * by keeping the batching logic separate.
> */
> if (unlikely(!*prealloc && folio_test_large(folio) && max_nr != 1)) {
> - nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr);
> + if (src_vma->vm_flags & VM_SHARED)
> + flags |= FPB_IGNORE_DIRTY;
> + if (!vma_soft_dirty_enabled(src_vma))
> + flags |= FPB_IGNORE_SOFT_DIRTY;
> +
> + nr = folio_pte_batch(folio, addr, src_pte, pte, max_nr, flags);
> folio_ref_add(folio, nr);
> if (folio_test_anon(folio)) {
> if (unlikely(folio_try_dup_anon_rmap_ptes(folio, page,