[PATCH v6] mm: retry page faults once under the per-VMA lock

[PATCH v6] mm: retry page faults once under the per-VMA lock

[Hongru Zhang]

From: Hongru Zhang <zhanghongru@xiaomi.com>

The per-VMA lock fault path releases the per-VMA lock when the folio
cannot be locked. It then waits for the folio to become lockable
without holding the VMA lock and retries the page fault. However,
the retry always takes the mmap_lock instead of the per-VMA lock.
This can cause serious lock contention when a writer is holding the
mmap_lock at the same time.

Add a single retry under the per-VMA lock in the architecture fault
handler. This does not touch any page fault code in mm. The retry is
very likely to succeed because the first page fault has already waited
for the folio to become lockable. For example, this usually means that
any required I/O has completed. This allows faults that can make
progress on an immediate retry to stay on the per-VMA lock path,
avoiding waits on the mmap_lock when it is write-contended. This
reduces page-fault latency and mmap_lock contention. Some faults may
retry unnecessarily, for example, those in __vmf_anon_prepare() or
device-private fault handling, which require the mmap_lock. However,
these cases are expected to be infrequent and only add one cheap
per-VMA lock attempt. If the second attempt still returns
VM_FAULT_RETRY, the fault continues through the existing mmap_lock
path.

Based on the stress model from Kunwu Chan and Wang Lian in RFC v2, we
adapted a benchmark [1] to a 20-core Intel i7-12700 desktop by reducing
the thread count and adjusting the memcg limits. The benchmark uses
concurrent page faults under memcg pressure with parallel munmap to
amplify mmap_lock read-write contention.

Filemap throughput (higher is better)

+---------+------------+---------------------+
| Threads |  Vanilla   |       Patched       |
+---------+------------+---------------------+
|   40    | 1069.34 /s | 1400.13 /s (+30.9%) |
+---------+------------+---------------------+
|   60    | 1038.12 /s | 1683.37 /s (+62.2%) |
+---------+------------+---------------------+
|   80    | 1042.62 /s | 1767.83 /s (+69.6%) |
+---------+------------+---------------------+

mmap_lock contention count (lower is better)

+---------+-----------+---------+-----------+
| Threads |  Vanilla  | Patched | Reduction |
+---------+-----------+---------+-----------+
|   40    | 3,187,336 |  52,086 |  -98.4%   |
+---------+-----------+---------+-----------+
|   60    | 4,385,154 |  65,079 |  -98.5%   |
+---------+-----------+---------+-----------+
|   80    | 5,337,890 |  69,708 |  -98.7%   |
+---------+-----------+---------+-----------+

These results show that retrying once under the per-VMA lock keeps more
file-backed faults on the fast path, improving throughput and reducing
mmap_lock contention.

Using benchmark [2], we tested this on a 20-core Intel i7-12700 desktop
with a 2GB swapfile. The benchmark uses one pressure thread under memcg
limits to keep a 128MB non-zero anonymous mapping under swap pressure,
12 reader threads to fault it back in, and optional mmap writer threads
to amplify mmap_lock read-write contention. Each test ran for 60 seconds
and reported completed reader rounds per second under swap pressure.

Swap throughput (higher is better)

+--------------+-------------+---------------------------+
| mmap writers |   Vanilla   |          Patched          |
+--------------+-------------+---------------------------+
|      0       | 17303.09 /s | 17899.48 /s       (+3.4%) |
+--------------+-------------+---------------------------+
|      4       | 12596.23 /s | 16095.20 /s      (+27.8%) |
+--------------+-------------+---------------------------+
|      8       |     0.58 /s | 15420.57 /s (+2658619.0%) |
+--------------+-------------+---------------------------+

With increasing mmap_lock write pressure, Vanilla degrades sharply and
drops to near zero at eight writers. Patched kernel holds up much better.

Performance was evaluated on a Pixel 6 running Android 17, using Baidu
Tieba (com.baidu.tieba) and Tencent Video (com.tencent.qqlive), both
very popular Android apps, as the workloads. For each workload, we
performed 100 cold app launches with each kernel variant (vanilla and
patched). Each run recorded cold app launch time, measured as the time
to first frame, and the main thread's mmap_lock wait events. Shorter
launch times indicate better performance.

Baidu Tieba cold app launch time

+-----------+----------+----------+--------+
| Statistic |  Vanilla |  Patched | Change |
+-----------+----------+----------+--------+
|    Mean   | 3,672 ms | 3,580 ms |  -2.5% |
+-----------+----------+----------+--------+
|  Maximum  | 4,469 ms | 4,156 ms |  -7.0% |
+-----------+----------+----------+--------+

Baidu Tieba cold app launch time distribution

+-------------+------------+------------+
|  Time (ms)  |   Vanilla  |   Patched  |
+-------------+------------+------------+
| 2,750-2,999 |  0  (0.0%) |  1  (1.0%) |
+-------------+------------+------------+
| 3,000-3,249 |  8  (8.0%) | 14 (14.0%) |
+-------------+------------+------------+
| 3,250-3,499 | 17 (17.0%) | 30 (30.0%) |
+-------------+------------+------------+
| 3,500-3,749 | 38 (38.0%) | 27 (27.0%) |
+-------------+------------+------------+
| 3,750-3,999 | 26 (26.0%) | 18 (18.0%) |
+-------------+------------+------------+
| 4,000-4,249 |  8  (8.0%) | 10 (10.0%) |
+-------------+------------+------------+
| 4,250-4,499 |  3  (3.0%) |  0  (0.0%) |
+-------------+------------+------------+

Baidu Tieba main-thread mmap_lock wait statistics

+----------------------+----------------+----------------+--------+
|        Metric        |     Vanilla    |     Patched    | Change |
+----------------------+----------------+----------------+--------+
|    Total Wait Time   |  569.9 ms/run  |  469.9 ms/run  | -17.6% |
+----------------------+----------------+----------------+--------+
| Read-Lock Wait Count | 32.4 waits/run | 10.9 waits/run | -66.2% |
+----------------------+----------------+----------------+--------+

Tencent Video cold app launch time

+-----------+----------+----------+--------+
| Statistic |  Vanilla |  Patched | Change |
+-----------+----------+----------+--------+
|    Mean   | 1,907 ms | 1,840 ms |  -3.5% |
+-----------+----------+----------+--------+
|  Maximum  | 3,023 ms | 2,851 ms |  -5.7% |
+-----------+----------+----------+--------+

Tencent Video cold app launch time distribution

+-------------+------------+------------+
|  Time (ms)  |   Vanilla  |   Patched  |
+-------------+------------+------------+
| 1,250-1,499 |  3  (3.0%) |  7  (7.0%) |
+-------------+------------+------------+
| 1,500-1,749 | 32 (32.0%) | 33 (33.0%) |
+-------------+------------+------------+
| 1,750-1,999 | 39 (39.0%) | 36 (36.0%) |
+-------------+------------+------------+
| 2,000-2,249 | 12 (12.0%) | 15 (15.0%) |
+-------------+------------+------------+
| 2,250-2,499 |  7  (7.0%) |  4  (4.0%) |
+-------------+------------+------------+
| 2,500-2,749 |  5  (5.0%) |  3  (3.0%) |
+-------------+------------+------------+
| 2,750-2,999 |  1  (1.0%) |  2  (2.0%) |
+-------------+------------+------------+
| 3,000-3,249 |  1  (1.0%) |  0  (0.0%) |
+-------------+------------+------------+

Tencent Video main-thread mmap_lock wait statistics

+----------------------+----------------+---------------+--------+
|        Metric        |     Vanilla    |    Patched    | Change |
+----------------------+----------------+---------------+--------+
|    Total Wait Time   |  139.6 ms/run  |  66.4 ms/run  | -52.4% |
+----------------------+----------------+---------------+--------+
| Read-Lock Wait Count | 28.4 waits/run | 4.7 waits/run | -83.5% |
+----------------------+----------------+---------------+--------+

Across both workloads, the single retry under the per-VMA lock
substantially reduced mmap_lock read-side contention, leading to lower
app startup times at both the mean and the tail.

[1] https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef
[2] https://gist.github.com/zhr250/218ffe693f842346b56434483127422c

Signed-off-by: Hongru Zhang <zhanghongru@xiaomi.com>
Suggested-by: Barry Song <baohua@kernel.org>
Suggested-by: Suren Baghdasaryan <surenb@google.com>
Suggested-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
Tested-by: Nanzhe Zhao <zhaonanzhe@xiaomi.com>
---
Changes since RFC v5:
- Added measurements from 100 cold app launches per kernel for each of
  Baidu Tieba and Tencent Video on a Pixel 6 running Android 17
  Tested by Nanzhe Zhao. Thanks!
- Rebased onto mm-unstable; no code changes

Changes since RFC v4:
- Drop `VM_FAULT_MAY_USE_VMA_LOCK` and always retry once under the
  per-VMA lock, based on feedback from Lorenzo and Barry. Thanks!

Changes since RFC v3:
- Keep VM_FAULT_RETRY unchanged and add VM_FAULT_MAY_USE_VMA_LOCK as an
  advisory bit
- Bound VMA-lock retries with FAULT_FLAG_TRIED
- Opt in filemap_fault() and do_swap_page() to VM_FAULT_MAY_USE_VMA_LOCK
- Rebased on mm-unstable

Changes since RFC v2:
- Redesigned as a single blacklist-based patch (v2 was 5 per-path patches)
- Added retry_vma loop to all architectures (not just x86)
- Rebased on mm-unstable

Changes since RFC v1:
- collect tags from Pedro, Kunwu and Lian, thanks!
- handle case (2), for uptodate folios, don't retry PF

Link to RFC v5:
https://lore.kernel.org/all/20260814085300.399107-1-zhanghongru@xiaomi.com/

Link to RFC v4:
https://lore.kernel.org/all/20260804095135.45897-1-zhanghongru@xiaomi.com/

Link to RFC v3:
https://lore.kernel.org/all/20260626075019.1833065-1-zhanghongru@xiaomi.com/

Link to RFC v2:
https://lore.kernel.org/all/20260430040427.4672-1-baohua@kernel.org/

Link to RFC v1:
https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@gmail.com/

 arch/arm/mm/fault.c       | 8 ++++++++
 arch/arm64/mm/fault.c     | 8 ++++++++
 arch/loongarch/mm/fault.c | 8 ++++++++
 arch/powerpc/mm/fault.c   | 7 +++++++
 arch/riscv/mm/fault.c     | 8 ++++++++
 arch/s390/mm/fault.c      | 6 ++++++
 arch/x86/mm/fault.c       | 8 ++++++++
 7 files changed, 53 insertions(+)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index 0a09d4ff7718..70472744f5c5 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -344,6 +344,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	vm_fault_t fault;
 	unsigned int flags = FAULT_FLAG_DEFAULT;
 	vm_flags_t vm_flags = VM_ACCESS_FLAGS;
+	bool vma_lock_retried = false;
 
 	if (kprobe_page_fault(regs, fsr))
 		return 0;
@@ -395,6 +396,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+lock_vma:
 	vma = lock_vma_under_rcu(mm, addr);
 	if (!vma)
 		goto lock_mmap;
@@ -424,6 +426,12 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 			goto no_context;
 		return 0;
 	}
+
+	if (!vma_lock_retried) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
+
 lock_mmap:
 
 retry:
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 75c3e463df2e..c7fd6f485b16 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -614,6 +614,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 	struct vm_area_struct *vma;
 	int si_code;
 	int pkey = -1;
+	bool vma_lock_retried = false;
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;
@@ -682,6 +683,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 	if (!(mm_flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+lock_vma:
 	vma = lock_vma_under_rcu(mm, addr);
 	if (!vma)
 		goto lock_mmap;
@@ -728,6 +730,12 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 			goto no_context;
 		return 0;
 	}
+
+	if (!vma_lock_retried) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
+
 lock_mmap:
 
 retry:
diff --git a/arch/loongarch/mm/fault.c b/arch/loongarch/mm/fault.c
index 2c93d33356e5..ef6ea847b1e0 100644
--- a/arch/loongarch/mm/fault.c
+++ b/arch/loongarch/mm/fault.c
@@ -181,6 +181,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
 	struct mm_struct *mm = tsk->mm;
 	struct vm_area_struct *vma = NULL;
 	vm_fault_t fault;
+	bool vma_lock_retried = false;
 
 	if (kprobe_page_fault(regs, current->thread.trap_nr))
 		return;
@@ -219,6 +220,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+lock_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -265,6 +267,12 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
 			no_context(regs, write, address);
 		return;
 	}
+
+	if (!vma_lock_retried) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
+
 lock_mmap:
 
 retry:
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 806c74e0d5ab..06018b6d7086 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -422,6 +422,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
 	int is_write = page_fault_is_write(error_code);
 	vm_fault_t fault, major = 0;
 	bool kprobe_fault = kprobe_page_fault(regs, 11);
+	bool vma_lock_retried = false;
 
 	if (unlikely(debugger_fault_handler(regs) || kprobe_fault))
 		return 0;
@@ -487,6 +488,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+lock_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -517,6 +519,11 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
 	if (fault_signal_pending(fault, regs))
 		return user_mode(regs) ? 0 : SIGBUS;
 
+	if (!vma_lock_retried) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
+
 lock_mmap:
 
 	/* When running in the kernel we expect faults to occur only to
diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 04ed6f8acae4..ff861793dba9 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -284,6 +284,7 @@ void handle_page_fault(struct pt_regs *regs)
 	unsigned int flags = FAULT_FLAG_DEFAULT;
 	int code = SEGV_MAPERR;
 	vm_fault_t fault;
+	bool vma_lock_retried = false;
 
 	cause = regs->cause;
 	addr = regs->badaddr;
@@ -347,6 +348,7 @@ void handle_page_fault(struct pt_regs *regs)
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+lock_vma:
 	vma = lock_vma_under_rcu(mm, addr);
 	if (!vma)
 		goto lock_mmap;
@@ -376,6 +378,12 @@ void handle_page_fault(struct pt_regs *regs)
 			no_context(regs, addr);
 		return;
 	}
+
+	if (!vma_lock_retried) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
+
 lock_mmap:
 
 retry:
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 46d828926009..dcd1ba24497f 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -271,6 +271,7 @@ static void do_exception(struct pt_regs *regs, int access)
 	unsigned int flags;
 	vm_fault_t fault;
 	bool is_write;
+	bool vma_lock_retried = false;
 
 	/*
 	 * The instruction that caused the program check has
@@ -294,6 +295,7 @@ static void do_exception(struct pt_regs *regs, int access)
 		flags |= FAULT_FLAG_WRITE;
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
+lock_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -318,6 +320,10 @@ static void do_exception(struct pt_regs *regs, int access)
 			handle_fault_error_nolock(regs, 0);
 		return;
 	}
+	if (!vma_lock_retried) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
 lock_mmap:
 retry:
 	vma = lock_mm_and_find_vma(mm, address, regs);
diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index aa88370ce739..df10d5cea4ee 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1222,6 +1222,7 @@ void do_user_addr_fault(struct pt_regs *regs,
 	struct mm_struct *mm;
 	vm_fault_t fault;
 	unsigned int flags = FAULT_FLAG_DEFAULT;
+	bool vma_lock_retried = false;
 
 	tsk = current;
 	mm = tsk->mm;
@@ -1331,6 +1332,7 @@ void do_user_addr_fault(struct pt_regs *regs,
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+lock_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -1360,6 +1362,12 @@ void do_user_addr_fault(struct pt_regs *regs,
 						 ARCH_DEFAULT_PKEY);
 		return;
 	}
+
+	if (!vma_lock_retried) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
+
 lock_mmap:
 
 retry:

base-commit: 3628c3df6cd2797b34714d23113cd44cb30801e7
-- 
2.43.0

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Xueyuan Chen]

On Fri, Sep 11, 2026 at 10:56:13AM +0800, Hongru Zhang wrote:
>From: Hongru Zhang <zhanghongru@xiaomi.com>
>
>The per-VMA lock fault path releases the per-VMA lock when the folio
>cannot be locked. It then waits for the folio to become lockable
>without holding the VMA lock and retries the page fault. However,
>the retry always takes the mmap_lock instead of the per-VMA lock.
>This can cause serious lock contention when a writer is holding the
>mmap_lock at the same time.
>
>Add a single retry under the per-VMA lock in the architecture fault
>handler. This does not touch any page fault code in mm. The retry is
>very likely to succeed because the first page fault has already waited
>for the folio to become lockable. For example, this usually means that
>any required I/O has completed. This allows faults that can make
>progress on an immediate retry to stay on the per-VMA lock path,
>avoiding waits on the mmap_lock when it is write-contended. This
>reduces page-fault latency and mmap_lock contention. Some faults may
>retry unnecessarily, for example, those in __vmf_anon_prepare() or
>device-private fault handling, which require the mmap_lock. However,
>these cases are expected to be infrequent and only add one cheap
>per-VMA lock attempt. If the second attempt still returns
>VM_FAULT_RETRY, the fault continues through the existing mmap_lock
>path.
>

Hi Hongru,

I tested cold launch time of douyin (the China's TikTok) and Honor of
Kings on SM8975. Each number below is the average of 100 runs:

     douyin:          666.0 ms -> 563.0 ms, 103 ms faster (15.5%)
     Honor of Kings: 1169.5 ms -> 1117.0 ms, 53 ms faster (4.5%)

The data shows a clear win on real apps.

Tested-by: Xueyuan Chen <xueyuan.chen21@gmail.com>

Thanks,
Xueyuan

>Based on the stress model from Kunwu Chan and Wang Lian in RFC v2, we
>adapted a benchmark [1] to a 20-core Intel i7-12700 desktop by reducing
>the thread count and adjusting the memcg limits. The benchmark uses
>concurrent page faults under memcg pressure with parallel munmap to
>amplify mmap_lock read-write contention.
>
>Filemap throughput (higher is better)
>
>+---------+------------+---------------------+
>| Threads |  Vanilla   |       Patched       |
>+---------+------------+---------------------+
>|   40    | 1069.34 /s | 1400.13 /s (+30.9%) |
>+---------+------------+---------------------+
>|   60    | 1038.12 /s | 1683.37 /s (+62.2%) |
>+---------+------------+---------------------+
>|   80    | 1042.62 /s | 1767.83 /s (+69.6%) |
>+---------+------------+---------------------+
>
>mmap_lock contention count (lower is better)
>
>+---------+-----------+---------+-----------+
>| Threads |  Vanilla  | Patched | Reduction |
>+---------+-----------+---------+-----------+
>|   40    | 3,187,336 |  52,086 |  -98.4%   |
>+---------+-----------+---------+-----------+
>|   60    | 4,385,154 |  65,079 |  -98.5%   |
>+---------+-----------+---------+-----------+
>|   80    | 5,337,890 |  69,708 |  -98.7%   |
>+---------+-----------+---------+-----------+
>
>These results show that retrying once under the per-VMA lock keeps more
>file-backed faults on the fast path, improving throughput and reducing
>mmap_lock contention.
>
>Using benchmark [2], we tested this on a 20-core Intel i7-12700 desktop
>with a 2GB swapfile. The benchmark uses one pressure thread under memcg
>limits to keep a 128MB non-zero anonymous mapping under swap pressure,
>12 reader threads to fault it back in, and optional mmap writer threads
>to amplify mmap_lock read-write contention. Each test ran for 60 seconds
>and reported completed reader rounds per second under swap pressure.
>
>Swap throughput (higher is better)
>
>+--------------+-------------+---------------------------+
>| mmap writers |   Vanilla   |          Patched          |
>+--------------+-------------+---------------------------+
>|      0       | 17303.09 /s | 17899.48 /s       (+3.4%) |
>+--------------+-------------+---------------------------+
>|      4       | 12596.23 /s | 16095.20 /s      (+27.8%) |
>+--------------+-------------+---------------------------+
>|      8       |     0.58 /s | 15420.57 /s (+2658619.0%) |
>+--------------+-------------+---------------------------+
>
>With increasing mmap_lock write pressure, Vanilla degrades sharply and
>drops to near zero at eight writers. Patched kernel holds up much better.
>
>Performance was evaluated on a Pixel 6 running Android 17, using Baidu
>Tieba (com.baidu.tieba) and Tencent Video (com.tencent.qqlive), both
>very popular Android apps, as the workloads. For each workload, we
>performed 100 cold app launches with each kernel variant (vanilla and
>patched). Each run recorded cold app launch time, measured as the time
>to first frame, and the main thread's mmap_lock wait events. Shorter
>launch times indicate better performance.
>
>Baidu Tieba cold app launch time
>
>+-----------+----------+----------+--------+
>| Statistic |  Vanilla |  Patched | Change |
>+-----------+----------+----------+--------+
>|    Mean   | 3,672 ms | 3,580 ms |  -2.5% |
>+-----------+----------+----------+--------+
>|  Maximum  | 4,469 ms | 4,156 ms |  -7.0% |
>+-----------+----------+----------+--------+
>
>Baidu Tieba cold app launch time distribution
>
>+-------------+------------+------------+
>|  Time (ms)  |   Vanilla  |   Patched  |
>+-------------+------------+------------+
>| 2,750-2,999 |  0  (0.0%) |  1  (1.0%) |
>+-------------+------------+------------+
>| 3,000-3,249 |  8  (8.0%) | 14 (14.0%) |
>+-------------+------------+------------+
>| 3,250-3,499 | 17 (17.0%) | 30 (30.0%) |
>+-------------+------------+------------+
>| 3,500-3,749 | 38 (38.0%) | 27 (27.0%) |
>+-------------+------------+------------+
>| 3,750-3,999 | 26 (26.0%) | 18 (18.0%) |
>+-------------+------------+------------+
>| 4,000-4,249 |  8  (8.0%) | 10 (10.0%) |
>+-------------+------------+------------+
>| 4,250-4,499 |  3  (3.0%) |  0  (0.0%) |
>+-------------+------------+------------+
>
>Baidu Tieba main-thread mmap_lock wait statistics
>
>+----------------------+----------------+----------------+--------+
>|        Metric        |     Vanilla    |     Patched    | Change |
>+----------------------+----------------+----------------+--------+
>|    Total Wait Time   |  569.9 ms/run  |  469.9 ms/run  | -17.6% |
>+----------------------+----------------+----------------+--------+
>| Read-Lock Wait Count | 32.4 waits/run | 10.9 waits/run | -66.2% |
>+----------------------+----------------+----------------+--------+
>
>Tencent Video cold app launch time
>
>+-----------+----------+----------+--------+
>| Statistic |  Vanilla |  Patched | Change |
>+-----------+----------+----------+--------+
>|    Mean   | 1,907 ms | 1,840 ms |  -3.5% |
>+-----------+----------+----------+--------+
>|  Maximum  | 3,023 ms | 2,851 ms |  -5.7% |
>+-----------+----------+----------+--------+
>
>Tencent Video cold app launch time distribution
>
>+-------------+------------+------------+
>|  Time (ms)  |   Vanilla  |   Patched  |
>+-------------+------------+------------+
>| 1,250-1,499 |  3  (3.0%) |  7  (7.0%) |
>+-------------+------------+------------+
>| 1,500-1,749 | 32 (32.0%) | 33 (33.0%) |
>+-------------+------------+------------+
>| 1,750-1,999 | 39 (39.0%) | 36 (36.0%) |
>+-------------+------------+------------+
>| 2,000-2,249 | 12 (12.0%) | 15 (15.0%) |
>+-------------+------------+------------+
>| 2,250-2,499 |  7  (7.0%) |  4  (4.0%) |
>+-------------+------------+------------+
>| 2,500-2,749 |  5  (5.0%) |  3  (3.0%) |
>+-------------+------------+------------+
>| 2,750-2,999 |  1  (1.0%) |  2  (2.0%) |
>+-------------+------------+------------+
>| 3,000-3,249 |  1  (1.0%) |  0  (0.0%) |
>+-------------+------------+------------+
>
>Tencent Video main-thread mmap_lock wait statistics
>
>+----------------------+----------------+---------------+--------+
>|        Metric        |     Vanilla    |    Patched    | Change |
>+----------------------+----------------+---------------+--------+
>|    Total Wait Time   |  139.6 ms/run  |  66.4 ms/run  | -52.4% |
>+----------------------+----------------+---------------+--------+
>| Read-Lock Wait Count | 28.4 waits/run | 4.7 waits/run | -83.5% |
>+----------------------+----------------+---------------+--------+
>
>Across both workloads, the single retry under the per-VMA lock
>substantially reduced mmap_lock read-side contention, leading to lower
>app startup times at both the mean and the tail.
>
>[1] https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef
>[2] https://gist.github.com/zhr250/218ffe693f842346b56434483127422c
>
>Signed-off-by: Hongru Zhang <zhanghongru@xiaomi.com>
>Suggested-by: Barry Song <baohua@kernel.org>
>Suggested-by: Suren Baghdasaryan <surenb@google.com>
>Suggested-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
>Tested-by: Nanzhe Zhao <zhaonanzhe@xiaomi.com>
>---
>Changes since RFC v5:
>- Added measurements from 100 cold app launches per kernel for each of
>  Baidu Tieba and Tencent Video on a Pixel 6 running Android 17
>  Tested by Nanzhe Zhao. Thanks!
>- Rebased onto mm-unstable; no code changes
>
>Changes since RFC v4:
>- Drop `VM_FAULT_MAY_USE_VMA_LOCK` and always retry once under the
>  per-VMA lock, based on feedback from Lorenzo and Barry. Thanks!
>
>Changes since RFC v3:
>- Keep VM_FAULT_RETRY unchanged and add VM_FAULT_MAY_USE_VMA_LOCK as an
>  advisory bit
>- Bound VMA-lock retries with FAULT_FLAG_TRIED
>- Opt in filemap_fault() and do_swap_page() to VM_FAULT_MAY_USE_VMA_LOCK
>- Rebased on mm-unstable
>
>Changes since RFC v2:
>- Redesigned as a single blacklist-based patch (v2 was 5 per-path patches)
>- Added retry_vma loop to all architectures (not just x86)
>- Rebased on mm-unstable
>
>Changes since RFC v1:
>- collect tags from Pedro, Kunwu and Lian, thanks!
>- handle case (2), for uptodate folios, don't retry PF
>
>Link to RFC v5:
>https://lore.kernel.org/all/20260814085300.399107-1-zhanghongru@xiaomi.com/
>
>Link to RFC v4:
>https://lore.kernel.org/all/20260804095135.45897-1-zhanghongru@xiaomi.com/
>
>Link to RFC v3:
>https://lore.kernel.org/all/20260626075019.1833065-1-zhanghongru@xiaomi.com/
>
>Link to RFC v2:
>https://lore.kernel.org/all/20260430040427.4672-1-baohua@kernel.org/
>
>Link to RFC v1:
>https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@gmail.com/
>
> arch/arm/mm/fault.c       | 8 ++++++++
> arch/arm64/mm/fault.c     | 8 ++++++++
> arch/loongarch/mm/fault.c | 8 ++++++++
> arch/powerpc/mm/fault.c   | 7 +++++++
> arch/riscv/mm/fault.c     | 8 ++++++++
> arch/s390/mm/fault.c      | 6 ++++++
> arch/x86/mm/fault.c       | 8 ++++++++
> 7 files changed, 53 insertions(+)
>
>diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
>index 0a09d4ff7718..70472744f5c5 100644
>--- a/arch/arm/mm/fault.c
>+++ b/arch/arm/mm/fault.c
>@@ -344,6 +344,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> 	vm_fault_t fault;
> 	unsigned int flags = FAULT_FLAG_DEFAULT;
> 	vm_flags_t vm_flags = VM_ACCESS_FLAGS;
>+	bool vma_lock_retried = false;
> 
> 	if (kprobe_page_fault(regs, fsr))
> 		return 0;
>@@ -395,6 +396,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> 	if (!(flags & FAULT_FLAG_USER))
> 		goto lock_mmap;
> 
>+lock_vma:
> 	vma = lock_vma_under_rcu(mm, addr);
> 	if (!vma)
> 		goto lock_mmap;
>@@ -424,6 +426,12 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> 			goto no_context;
> 		return 0;
> 	}
>+
>+	if (!vma_lock_retried) {
>+		vma_lock_retried = true;
>+		goto lock_vma;
>+	}
>+
> lock_mmap:
> 
> retry:
>diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>index 75c3e463df2e..c7fd6f485b16 100644
>--- a/arch/arm64/mm/fault.c
>+++ b/arch/arm64/mm/fault.c
>@@ -614,6 +614,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> 	struct vm_area_struct *vma;
> 	int si_code;
> 	int pkey = -1;
>+	bool vma_lock_retried = false;
> 
> 	if (kprobe_page_fault(regs, esr))
> 		return 0;
>@@ -682,6 +683,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> 	if (!(mm_flags & FAULT_FLAG_USER))
> 		goto lock_mmap;
> 
>+lock_vma:
> 	vma = lock_vma_under_rcu(mm, addr);
> 	if (!vma)
> 		goto lock_mmap;
>@@ -728,6 +730,12 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> 			goto no_context;
> 		return 0;
> 	}
>+
>+	if (!vma_lock_retried) {
>+		vma_lock_retried = true;
>+		goto lock_vma;
>+	}
>+
> lock_mmap:
> 
> retry:
>diff --git a/arch/loongarch/mm/fault.c b/arch/loongarch/mm/fault.c
>index 2c93d33356e5..ef6ea847b1e0 100644
>--- a/arch/loongarch/mm/fault.c
>+++ b/arch/loongarch/mm/fault.c
>@@ -181,6 +181,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> 	struct mm_struct *mm = tsk->mm;
> 	struct vm_area_struct *vma = NULL;
> 	vm_fault_t fault;
>+	bool vma_lock_retried = false;
> 
> 	if (kprobe_page_fault(regs, current->thread.trap_nr))
> 		return;
>@@ -219,6 +220,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> 	if (!(flags & FAULT_FLAG_USER))
> 		goto lock_mmap;
> 
>+lock_vma:
> 	vma = lock_vma_under_rcu(mm, address);
> 	if (!vma)
> 		goto lock_mmap;
>@@ -265,6 +267,12 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> 			no_context(regs, write, address);
> 		return;
> 	}
>+
>+	if (!vma_lock_retried) {
>+		vma_lock_retried = true;
>+		goto lock_vma;
>+	}
>+
> lock_mmap:
> 
> retry:
>diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>index 806c74e0d5ab..06018b6d7086 100644
>--- a/arch/powerpc/mm/fault.c
>+++ b/arch/powerpc/mm/fault.c
>@@ -422,6 +422,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
> 	int is_write = page_fault_is_write(error_code);
> 	vm_fault_t fault, major = 0;
> 	bool kprobe_fault = kprobe_page_fault(regs, 11);
>+	bool vma_lock_retried = false;
> 
> 	if (unlikely(debugger_fault_handler(regs) || kprobe_fault))
> 		return 0;
>@@ -487,6 +488,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
> 	if (!(flags & FAULT_FLAG_USER))
> 		goto lock_mmap;
> 
>+lock_vma:
> 	vma = lock_vma_under_rcu(mm, address);
> 	if (!vma)
> 		goto lock_mmap;
>@@ -517,6 +519,11 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
> 	if (fault_signal_pending(fault, regs))
> 		return user_mode(regs) ? 0 : SIGBUS;
> 
>+	if (!vma_lock_retried) {
>+		vma_lock_retried = true;
>+		goto lock_vma;
>+	}
>+
> lock_mmap:
> 
> 	/* When running in the kernel we expect faults to occur only to
>diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
>index 04ed6f8acae4..ff861793dba9 100644
>--- a/arch/riscv/mm/fault.c
>+++ b/arch/riscv/mm/fault.c
>@@ -284,6 +284,7 @@ void handle_page_fault(struct pt_regs *regs)
> 	unsigned int flags = FAULT_FLAG_DEFAULT;
> 	int code = SEGV_MAPERR;
> 	vm_fault_t fault;
>+	bool vma_lock_retried = false;
> 
> 	cause = regs->cause;
> 	addr = regs->badaddr;
>@@ -347,6 +348,7 @@ void handle_page_fault(struct pt_regs *regs)
> 	if (!(flags & FAULT_FLAG_USER))
> 		goto lock_mmap;
> 
>+lock_vma:
> 	vma = lock_vma_under_rcu(mm, addr);
> 	if (!vma)
> 		goto lock_mmap;
>@@ -376,6 +378,12 @@ void handle_page_fault(struct pt_regs *regs)
> 			no_context(regs, addr);
> 		return;
> 	}
>+
>+	if (!vma_lock_retried) {
>+		vma_lock_retried = true;
>+		goto lock_vma;
>+	}
>+
> lock_mmap:
> 
> retry:
>diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
>index 46d828926009..dcd1ba24497f 100644
>--- a/arch/s390/mm/fault.c
>+++ b/arch/s390/mm/fault.c
>@@ -271,6 +271,7 @@ static void do_exception(struct pt_regs *regs, int access)
> 	unsigned int flags;
> 	vm_fault_t fault;
> 	bool is_write;
>+	bool vma_lock_retried = false;
> 
> 	/*
> 	 * The instruction that caused the program check has
>@@ -294,6 +295,7 @@ static void do_exception(struct pt_regs *regs, int access)
> 		flags |= FAULT_FLAG_WRITE;
> 	if (!(flags & FAULT_FLAG_USER))
> 		goto lock_mmap;
>+lock_vma:
> 	vma = lock_vma_under_rcu(mm, address);
> 	if (!vma)
> 		goto lock_mmap;
>@@ -318,6 +320,10 @@ static void do_exception(struct pt_regs *regs, int access)
> 			handle_fault_error_nolock(regs, 0);
> 		return;
> 	}
>+	if (!vma_lock_retried) {
>+		vma_lock_retried = true;
>+		goto lock_vma;
>+	}
> lock_mmap:
> retry:
> 	vma = lock_mm_and_find_vma(mm, address, regs);
>diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
>index aa88370ce739..df10d5cea4ee 100644
>--- a/arch/x86/mm/fault.c
>+++ b/arch/x86/mm/fault.c
>@@ -1222,6 +1222,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> 	struct mm_struct *mm;
> 	vm_fault_t fault;
> 	unsigned int flags = FAULT_FLAG_DEFAULT;
>+	bool vma_lock_retried = false;
> 
> 	tsk = current;
> 	mm = tsk->mm;
>@@ -1331,6 +1332,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> 	if (!(flags & FAULT_FLAG_USER))
> 		goto lock_mmap;
> 
>+lock_vma:
> 	vma = lock_vma_under_rcu(mm, address);
> 	if (!vma)
> 		goto lock_mmap;
>@@ -1360,6 +1362,12 @@ void do_user_addr_fault(struct pt_regs *regs,
> 						 ARCH_DEFAULT_PKEY);
> 		return;
> 	}
>+
>+	if (!vma_lock_retried) {
>+		vma_lock_retried = true;
>+		goto lock_vma;
>+	}
>+
> lock_mmap:
> 
> retry:
>
>base-commit: 3628c3df6cd2797b34714d23113cd44cb30801e7
>-- 
>2.43.0
>
>

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Lorenzo Stoakes (ARM)]

OK I finally got to this...

I fear we're going to go round in circles on this/have endless revisions of
the same thing.

So 2 things the patch needs to answer, more or less right away:

1. What are you actually optimising for?

2. Where does the hurt happen?

Your patch does neither, then goes on to arm wave away all the complexity
and presents a 'simple' solution that I feel brushes a lot under the rug.

Answer to 1 is 'cold app startup time for a specific set of apps'
2 is filemap_fault() AFAICT.

You should definitely be stating this upfront.

And I also seem to remember that zygote + multi-threaded apps = fork
blocking is part of the problem here, which is why there was push-back on
just holding the VMA lock over I/O.

Looking through filemap_fault() makes me want to cry and looking through
the other retry logic in the fault code makes me want to live on an island
with parakeets and coconuts forgetting any of this even exists...

The retry logic is fu.. err.. ndamentally a total mess.

Part of the problem is that each time this patch is respinned we lose
context on what is a complicated istuation.

VM_FAULT_RETRY can mean a million different things:

1. I dropped the lock and waited for the folio to be unlocked.

do_swap_page(), remove_device_exclusive_entry() -> folio_lock_or_retry()

2. I dropped the lock to start I/O it may or may not be ready when you come
   back.

filemap_fault() after do_sync_mmap_readahead() or page not uptodate
synchronous read with lock dropped <- what you are optimising for, or
shmem_falloc_wait() waiting for a hole punch.

The maybe_unlock_mmap_for_io() stuff.

3. I dropped the lock and nothing was waited for, OOM me on retry!

filemap_fault() allocation failure, gotta retry because we dropped the
lock!

4. I can't do this under the VMA lock, use an mmap lock

vmf_can_call_fault(), any vm_ops without ->map_pages, __vmf_anon_prepare()
when mmap_read_trylock() lost, also hugetlb_fault() horror shows.

Also device-private swap entries.

5. Userfaultfd! Because of course!

Userspace has to do something.

handle_userfault() after sleeping for uffd handler, retry should succeed.

ctx->released -> yield to releasing thread.

6. Fatal signal

Arch handlers rely on this, so retry also means 'check signals'. Fun.

7. Some driver insanity

It means all things to all people. Used for waiting on stuff like

LLM says:

- TTM at drivers/gpu/drm/ttm/ttm_bo_vm.c:62 and 144, dma_resv contention
  and GPU idle wait. Consumers in amdgpu, i915, nouveau, radeon and vmwgfx
  re-derive "was the reservation unlocked" from ret == VM_FAULT_RETRY &&
  !NOWAIT.

- xe_bo_cpu_fault_fastpath() at drivers/gpu/drm/xe/xe_bo.c:2021, where
  RETRY is the default return value, including for "runtime PM not active".

- panthor at drivers/gpu/drm/panthor/panthor_gem.c:831 onwards, where
  dma_resv_trylock() failure is RETRY and the same error is NOPAGE, SIGBUS
  or RETRY depending on mmap_lock_held.

- sgx_vepc_fault() at arch/x86/kernel/cpu/sgx/virt.c:91, EBUSY from
  __sgx_vepc_fault().  All of these call mmap_read_unlock() directly rather
  than release_fault_lock(). They are only safe because none has
  ->map_pages, so vmf_can_call_fault() bounces them before ->fault
  runs. The VMA-lock design rests on that proxy.

So yeah. All that. Wow.

8. Nothing!

LLM says:

  - page_mkwrite() returns in fs/exfat/file.c:951 (inode_trylock lost),
    fs/nfs/file.c:699, fs/netfs/buffered_write.c:576 and
    fs/orangefs/inode.c:635 onwards, including the combination
    VM_FAULT_LOCKED | VM_FAULT_RETRY. do_page_mkwrite() at mm/memory.c only
    passes through ERROR and NOPAGE, and do_shared_fault() and
    wp_page_shared() then ignore the returned value entirely. The write
    fault completes as if page_mkwrite had succeeded.

And maybe I'm missing some stuff too.

there's also places where retries are done which don't even involve
VM_FAULT_RETRY like migration_entry_wait() and pte_same() mismatches and
do_page_mkwrite() on truncation and some other places.

I think any patch maybe doesn't have to have a massive essay but it DOES
need to ack that three are a LOT of meanings and it's a TOTAL DISASTER.

Your perf numbers look very artificial - you create situations where you
KNOW from the code you'll get mmap lock contention, then have a lot of
threads contend and gosh darn it, well the contention is really high! :)

I mean - I feel like most of your perf numbers amount to 'contend locks
gets us lock contention' - and are thus useless really?

I think the bit that _matters_ is what you're actually after which is stuff
like:

> Tencent Video cold app launch time
>
> +-----------+----------+----------+--------+
> | Statistic |  Vanilla |  Patched | Change |
> +-----------+----------+----------+--------+
> |    Mean   | 1,907 ms | 1,840 ms |  -3.5% |
> +-----------+----------+----------+--------+
> |  Maximum  | 3,023 ms | 2,851 ms |  -5.7% |
> +-----------+----------+----------+--------+

Which is not earth-shattering to me.

Now, while the patch is _small_ it takes all of the above, all of the
complexity, all of the bigger questions about how to solve this and
compresses it down into 'just retry the VMA lock'.

(I note that you change the tried logic subtly too which is another thing
but never mind that).

I worry that we're not doing the REAL work here and this is just shoving
stuff under the rug for what look to be mediocre performance improvements
in a very specific workload and costing a bunch of paths (albeit not common
ones) pointless VMA lock retries.

And I worry that maybe that actually _adds_ complexity by the back door a
bit.

But I worry a LOT more that it's just a HACK (and I know Barry was nice
about the idea and I appreciate it but I have to be honest).

Instead of dealing with any of the above, we just leave the mess in place +
just retry the operation under VMA because we happen to know, for this ONE
workload, it works out better.

And PROBABLY it doesn't add too much overhead to anything else.

SO.

I've written too much again, let's sum it up.

1. WE HAVE TO DECIDE whether we want to accept the hack because it
   helps in a known case and probably doesn't harm any other cases.

2. WE HAVE TO FIX THIS DAMN MESS. Even if we take something like this WE
   HAVE TO FIX IT.

I'm inclined to rip out the whole retry thing altogether one way or another
but I leave that to Matthew to figure out :)


If we decide we DO want the hack, then DEAR LORD can we not have this
horrible duplication across arches? I seem to remember you agreed to take
that out ([0]), and maybe it's pending what Matthew wants to do, but is
there not a way to avoid that?

ALSO. It seem we are subtly changing how the tried flag stuff works and
that really needs to be considered carefully.

And FINALLY if we DO do that, the patch message MUST be honest about the
goals MUST state EXACTLY what it's trying to optimise, it MUST clearly
state that in a comment or whatever where the code is.

It must NOT wave its arms and pretend that it's some general solution for
fault retries the way it does now. It feels like it's hiding what it's
really intended for at that just adds yet more confusion to this mess.

I REALLY want to hear from Matthew on all this, I don't think we can move
ahead without his clear feedback.

--
Cheers, Lorenzo

[0]:https://lore.kernel.org/linux-mm/20260709084752.147379-1-zhanghongru@xiaomi.com/

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Matthew Wilcox]

On Mon, Sep 21, 2026 at 06:54:25PM +0100, Lorenzo Stoakes (ARM) wrote:
> OK I finally got to this...

Brave ;-)

> I fear we're going to go round in circles on this/have endless revisions of
> the same thing.
> 
> So 2 things the patch needs to answer, more or less right away:
> 
> 1. What are you actually optimising for?
> 
> 2. Where does the hurt happen?
> 
> Your patch does neither, then goes on to arm wave away all the complexity
> and presents a 'simple' solution that I feel brushes a lot under the rug.
> 
> Answer to 1 is 'cold app startup time for a specific set of apps'
> 2 is filemap_fault() AFAICT.
> 
> You should definitely be stating this upfront.
> 
> And I also seem to remember that zygote + multi-threaded apps = fork
> blocking is part of the problem here, which is why there was push-back on
> just holding the VMA lock over I/O.
> 
> Looking through filemap_fault() makes me want to cry and looking through
> the other retry logic in the fault code makes me want to live on an island
> with parakeets and coconuts forgetting any of this even exists...

I think that's the right response.  It's why all the feedback from people
who haven't even tried to follow the fault path (eg the session at LSFMM)
is completely useless.  The fault path is WAY TOO COMPLEX.  I'll have
more to say on this in two weeks at Plumbers.

> VM_FAULT_RETRY can mean a million different things:
> 
> 1. I dropped the lock and waited for the folio to be unlocked.
> 
> do_swap_page(), remove_device_exclusive_entry() -> folio_lock_or_retry()
> 
> 2. I dropped the lock to start I/O it may or may not be ready when you come
>    back.
> 
> filemap_fault() after do_sync_mmap_readahead() or page not uptodate
> synchronous read with lock dropped <- what you are optimising for, or
> shmem_falloc_wait() waiting for a hole punch.
> 
> The maybe_unlock_mmap_for_io() stuff.

I think these two are the same case?  At least by my reading, case 2
also waits for the folio to be unlocked (ie the read completed).
do_sync_mmap_readahead() doesn't do that, but we then call
__filemap_get_folio() which shuld return the appropriate folio,
then we call __folio_lock_killable() or __folio_lock().

> 3. I dropped the lock and nothing was waited for, OOM me on retry!
> 
> filemap_fault() allocation failure, gotta retry because we dropped the
> lock!

Oh, yeah, that's icky.  Hadn't even crawled onto my list of problems
with this function.

> 4. I can't do this under the VMA lock, use an mmap lock
> 
> vmf_can_call_fault(), any vm_ops without ->map_pages, __vmf_anon_prepare()
> when mmap_read_trylock() lost, also hugetlb_fault() horror shows.
> 
> Also device-private swap entries.

This one really is my fault / Suren's fault / your fault ;-)

We should not have overloaded VM_FAULT_RETRY for this case.  We should
have had a distinct VM_FAULT_NEEDS_MMAP_LOCK code.  But I am *scared*
to try to separate the two at this point.  It was hard to make sure we
got all the cases as we pushed it down, and now it's going to be worse.

But I don't think that hugetlb_fault() deserves any stick here:
         * We must check to release the per-VMA lock. __vmf_anon_prepare() in
         * hugetlb_wp() is the only way ret can be set to VM_FAULT_RETRY.
is the comment on the only two mentions of VM_FAULT_RETRY in hugetlb.

I do have a plan to get rid of those two mentions actually ...
but let's talk about in in Prague.

> 5. Userfaultfd! Because of course!
> 
> Userspace has to do something.
> 
> handle_userfault() after sleeping for uffd handler, retry should succeed.
> 
> ctx->released -> yield to releasing thread.

I find it really hard to reason about uffd.

> 6. Fatal signal
> 
> Arch handlers rely on this, so retry also means 'check signals'. Fun.
> 
> 7. Some driver insanity
> 
> It means all things to all people. Used for waiting on stuff like
> 
> LLM says:
> 
> - TTM at drivers/gpu/drm/ttm/ttm_bo_vm.c:62 and 144, dma_resv contention
>   and GPU idle wait. Consumers in amdgpu, i915, nouveau, radeon and vmwgfx
>   re-derive "was the reservation unlocked" from ret == VM_FAULT_RETRY &&
>   !NOWAIT.
> 
> - xe_bo_cpu_fault_fastpath() at drivers/gpu/drm/xe/xe_bo.c:2021, where
>   RETRY is the default return value, including for "runtime PM not active".
> 
> - panthor at drivers/gpu/drm/panthor/panthor_gem.c:831 onwards, where
>   dma_resv_trylock() failure is RETRY and the same error is NOPAGE, SIGBUS
>   or RETRY depending on mmap_lock_held.
> 
> - sgx_vepc_fault() at arch/x86/kernel/cpu/sgx/virt.c:91, EBUSY from
>   __sgx_vepc_fault().  All of these call mmap_read_unlock() directly rather
>   than release_fault_lock(). They are only safe because none has
>   ->map_pages, so vmf_can_call_fault() bounces them before ->fault
>   runs. The VMA-lock design rests on that proxy.
> 
> So yeah. All that. Wow.
> 
> 8. Nothing!
> 
> LLM says:
> 
>   - page_mkwrite() returns in fs/exfat/file.c:951 (inode_trylock lost),

That's just wrong.  It turns the pagefault path into a spin on
the inode lock!

>     fs/nfs/file.c:699, fs/netfs/buffered_write.c:576 and
>     fs/orangefs/inode.c:635 onwards, including the combination
>     VM_FAULT_LOCKED | VM_FAULT_RETRY.

I think your AI is confused.  I don't see how nfs_vm_page_mkwrite()
can return VM_FAULT_LOCKED | VM_FAULT_RETRY.  Nor netfs.  Orangefs
is wrong and needs to be fixed ... once I figure out what it actually
wants.

>     do_page_mkwrite() at mm/memory.c only
>     passes through ERROR and NOPAGE, and do_shared_fault() and
>     wp_page_shared() then ignore the returned value entirely. The write
>     fault completes as if page_mkwrite had succeeded.

That's probably OK.  I think?

> And maybe I'm missing some stuff too.

There's also the VM_FAULT_COMPLETED insanity ...

> But I worry a LOT more that it's just a HACK (and I know Barry was nice
> about the idea and I appreciate it but I have to be honest).
> 
> Instead of dealing with any of the above, we just leave the mess in place +
> just retry the operation under VMA because we happen to know, for this ONE
> workload, it works out better.
> 
> And PROBABLY it doesn't add too much overhead to anything else.
> 
> SO.
> 
> I've written too much again, let's sum it up.
> 
> 1. WE HAVE TO DECIDE whether we want to accept the hack because it
>    helps in a known case and probably doesn't harm any other cases.
> 
> 2. WE HAVE TO FIX THIS DAMN MESS. Even if we take something like this WE
>    HAVE TO FIX IT.
> 
> I'm inclined to rip out the whole retry thing altogether one way or another
> but I leave that to Matthew to figure out :)

I think there's a bathtub with a baby in it over there ...

There's considerable scope for simplification here.
lock_folio_maybe_drop_mmap() makes my eyeballs bleed every time I look
at it.  But we do need smoe kind of restart-the-fault mechanism.

> If we decide we DO want the hack, then DEAR LORD can we not have this
> horrible duplication across arches? I seem to remember you agreed to take
> that out ([0]), and maybe it's pending what Matthew wants to do, but is
> there not a way to avoid that?

We desperately need to move more of the fault handling path out of the
architectures.

> I REALLY want to hear from Matthew on all this, I don't think we can move
> ahead without his clear feedback.

Thanks for dragging me back into this ;-)

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Lorenzo Stoakes (ARM)]

On Mon, Sep 21, 2026 at 07:57:23PM +0100, Matthew Wilcox wrote:
> On Mon, Sep 21, 2026 at 06:54:25PM +0100, Lorenzo Stoakes (ARM) wrote:
> > OK I finally got to this...
>
> Brave ;-)

Or stupid? It's a blurry line ;)

>
> > I fear we're going to go round in circles on this/have endless revisions of
> > the same thing.
> >
> > So 2 things the patch needs to answer, more or less right away:
> >
> > 1. What are you actually optimising for?
> >
> > 2. Where does the hurt happen?
> >
> > Your patch does neither, then goes on to arm wave away all the complexity
> > and presents a 'simple' solution that I feel brushes a lot under the rug.
> >
> > Answer to 1 is 'cold app startup time for a specific set of apps'
> > 2 is filemap_fault() AFAICT.
> >
> > You should definitely be stating this upfront.
> >
> > And I also seem to remember that zygote + multi-threaded apps = fork
> > blocking is part of the problem here, which is why there was push-back on
> > just holding the VMA lock over I/O.
> >
> > Looking through filemap_fault() makes me want to cry and looking through
> > the other retry logic in the fault code makes me want to live on an island
> > with parakeets and coconuts forgetting any of this even exists...
>
> I think that's the right response.  It's why all the feedback from people

I mean, it's the instinctive move when you dig into all this again :) my eyes!
Burn it all down etc.

BUT. The other thing that came to mind was 'fix this bloody mess!'.

And I also worry with this patch that we might end up deferring NECESSARY work
to do that.

> who haven't even tried to follow the fault path (eg the session at LSFMM)
> is completely useless.  The fault path is WAY TOO COMPLEX.  I'll have

Yes. YES YES YES.

I was thinking honestly I'd look at this patch and come to the conclusion that
yes it is this simple and now I'm going to have to argue with mm people but
fine.

But no, I was just reminded how bloody complicated it all is.

> more to say on this in two weeks at Plumbers.

I look forward to it :)

>
> > VM_FAULT_RETRY can mean a million different things:
> >
> > 1. I dropped the lock and waited for the folio to be unlocked.
> >
> > do_swap_page(), remove_device_exclusive_entry() -> folio_lock_or_retry()
> >
> > 2. I dropped the lock to start I/O it may or may not be ready when you come
> >    back.
> >
> > filemap_fault() after do_sync_mmap_readahead() or page not uptodate
> > synchronous read with lock dropped <- what you are optimising for, or
> > shmem_falloc_wait() waiting for a hole punch.
> >
> > The maybe_unlock_mmap_for_io() stuff.
>
> I think these two are the same case?  At least by my reading, case 2
> also waits for the folio to be unlocked (ie the read completed).
> do_sync_mmap_readahead() doesn't do that, but we then call
> __filemap_get_folio() which shuld return the appropriate folio,
> then we call __folio_lock_killable() or __folio_lock().

Right yeah.

>
> > 3. I dropped the lock and nothing was waited for, OOM me on retry!
> >
> > filemap_fault() allocation failure, gotta retry because we dropped the
> > lock!
>
> Oh, yeah, that's icky.  Hadn't even crawled onto my list of problems
> with this function.

Yup. Man.

>
> > 4. I can't do this under the VMA lock, use an mmap lock
> >
> > vmf_can_call_fault(), any vm_ops without ->map_pages, __vmf_anon_prepare()
> > when mmap_read_trylock() lost, also hugetlb_fault() horror shows.
> >
> > Also device-private swap entries.
>
> This one really is my fault / Suren's fault / your fault ;-)

Mea culpa... We-a culpa?

>
> We should not have overloaded VM_FAULT_RETRY for this case.  We should
> have had a distinct VM_FAULT_NEEDS_MMAP_LOCK code.  But I am *scared*
> to try to separate the two at this point.  It was hard to make sure we
> got all the cases as we pushed it down, and now it's going to be worse.
>
> But I don't think that hugetlb_fault() deserves any stick here:
>          * We must check to release the per-VMA lock. __vmf_anon_prepare() in
>          * hugetlb_wp() is the only way ret can be set to VM_FAULT_RETRY.
> is the comment on the only two mentions of VM_FAULT_RETRY in hugetlb.

It does feel like we should just have

>
> I do have a plan to get rid of those two mentions actually ...
> but let's talk about in in Prague.

Yes, indeed.

>
> > 5. Userfaultfd! Because of course!
> >
> > Userspace has to do something.
> >
> > handle_userfault() after sleeping for uffd handler, retry should succeed.
> >
> > ctx->released -> yield to releasing thread.
>
> I find it really hard to reason about uffd.

Yeah me also.

The 2 fundamental issues with uffd (aside from code quality) is that:

a. It's the exception to a lot of rules
b. It duplicates its own logic

Similar to hugetlb.

And these problems grow as we add more functionality to mm.

I fear the device private stuff is becoming this a bit (albeit, not as bad).

>
> > 6. Fatal signal
> >
> > Arch handlers rely on this, so retry also means 'check signals'. Fun.
> >
> > 7. Some driver insanity
> >
> > It means all things to all people. Used for waiting on stuff like
> >
> > LLM says:
> >
> > - TTM at drivers/gpu/drm/ttm/ttm_bo_vm.c:62 and 144, dma_resv contention
> >   and GPU idle wait. Consumers in amdgpu, i915, nouveau, radeon and vmwgfx
> >   re-derive "was the reservation unlocked" from ret == VM_FAULT_RETRY &&
> >   !NOWAIT.
> >
> > - xe_bo_cpu_fault_fastpath() at drivers/gpu/drm/xe/xe_bo.c:2021, where
> >   RETRY is the default return value, including for "runtime PM not active".
> >
> > - panthor at drivers/gpu/drm/panthor/panthor_gem.c:831 onwards, where
> >   dma_resv_trylock() failure is RETRY and the same error is NOPAGE, SIGBUS
> >   or RETRY depending on mmap_lock_held.
> >
> > - sgx_vepc_fault() at arch/x86/kernel/cpu/sgx/virt.c:91, EBUSY from
> >   __sgx_vepc_fault().  All of these call mmap_read_unlock() directly rather
> >   than release_fault_lock(). They are only safe because none has
> >   ->map_pages, so vmf_can_call_fault() bounces them before ->fault
> >   runs. The VMA-lock design rests on that proxy.
> >
> > So yeah. All that. Wow.
> >
> > 8. Nothing!
> >
> > LLM says:
> >
> >   - page_mkwrite() returns in fs/exfat/file.c:951 (inode_trylock lost),
>
> That's just wrong.  It turns the pagefault path into a spin on
> the inode lock!

Oh lord. So it's... worse? :)

>
> >     fs/nfs/file.c:699, fs/netfs/buffered_write.c:576 and
> >     fs/orangefs/inode.c:635 onwards, including the combination
> >     VM_FAULT_LOCKED | VM_FAULT_RETRY.
>
> I think your AI is confused.  I don't see how nfs_vm_page_mkwrite()

1st rule of LLMs is they can always hallucinate/get things terrible wrong.

But yeah this was me being lazy but in my defence it was nearly 7pm and I was
still at the office with a long drive ahead of me ;)

> can return VM_FAULT_LOCKED | VM_FAULT_RETRY.  Nor netfs.  Orangefs
> is wrong and needs to be fixed ... once I figure out what it actually
> wants.

Well volunteered :>)

>
> >     do_page_mkwrite() at mm/memory.c only
> >     passes through ERROR and NOPAGE, and do_shared_fault() and
> >     wp_page_shared() then ignore the returned value entirely. The write
> >     fault completes as if page_mkwrite had succeeded.
>
> That's probably OK.  I think?

I think we probably need more flags, honestly. 'Retry' is too vague.

But on the other hand do we want a flag for every possible weird specific caes?

OK OK I'll wait for Prague for the simplification :)


>
> > And maybe I'm missing some stuff too.
>
> There's also the VM_FAULT_COMPLETED insanity ...
>
> > But I worry a LOT more that it's just a HACK (and I know Barry was nice
> > about the idea and I appreciate it but I have to be honest).
> >
> > Instead of dealing with any of the above, we just leave the mess in place +
> > just retry the operation under VMA because we happen to know, for this ONE
> > workload, it works out better.
> >
> > And PROBABLY it doesn't add too much overhead to anything else.
> >
> > SO.
> >
> > I've written too much again, let's sum it up.
> >
> > 1. WE HAVE TO DECIDE whether we want to accept the hack because it
> >    helps in a known case and probably doesn't harm any other cases.
> >
> > 2. WE HAVE TO FIX THIS DAMN MESS. Even if we take something like this WE
> >    HAVE TO FIX IT.
> >
> > I'm inclined to rip out the whole retry thing altogether one way or another
> > but I leave that to Matthew to figure out :)
>
> I think there's a bathtub with a baby in it over there ...

Yup as above, instinct to nuke from orbit etc. ;) but something (TM) needs to be
done.

>
> There's considerable scope for simplification here.
> lock_folio_maybe_drop_mmap() makes my eyeballs bleed every time I look
> at it.  But we do need smoe kind of restart-the-fault mechanism.

Right, yeah.

It does feel like there's a solution lurking here in the undergrowth, so again
Prague etc. etc.

>
> > If we decide we DO want the hack, then DEAR LORD can we not have this
> > horrible duplication across arches? I seem to remember you agreed to take
> > that out ([0]), and maybe it's pending what Matthew wants to do, but is
> > there not a way to avoid that?
>
> We desperately need to move more of the fault handling path out of the
> architectures.

That's definitely part of the solution!

>
> > I REALLY want to hear from Matthew on all this, I don't think we can move
> > ahead without his clear feedback.
>
> Thanks for dragging me back into this ;-)

No worries ;)

Though I do have to ask again - do we want this patch as-is as a temporary
solution? Or maybe just for x86/arm64 (god I hate the duplication of the arch
code)

I'm not sure that it does all that much harm?

It's not necessary either/or it can be an and. But I'd want your OK on taking
this.

--
Cheers, Lorenzo

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Matthew Wilcox]

On Tue, Sep 22, 2026 at 09:55:23AM +0100, Lorenzo Stoakes (ARM) wrote:
> On Mon, Sep 21, 2026 at 07:57:23PM +0100, Matthew Wilcox wrote:
> > more to say on this in two weeks at Plumbers.
> 
> I look forward to it :)

So while doing my slides, I realised that what we need to avoid doing
is (a) sleeping while holding the mmap_lock (b) returning RETRY while
holding the VMA lock

And that turns out to be as simple as this patch:

diff --git a/include/linux/mm.h b/include/linux/mm.h
index dd09c438fa23..94ed2333f8d8 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -723,6 +723,8 @@ enum {
  */
 static inline bool fault_flag_allow_retry_first(enum fault_flag flags)
 {
+	if (flags & FAULT_FLAG_VMA_LOCK)
+		return false;
 	return (flags & FAULT_FLAG_ALLOW_RETRY) &&
 	    (!(flags & FAULT_FLAG_TRIED));
 }

OK, this is a hack.  The function is spectacularly badly named, and
needs to be renamed before a patch can go upstream.  But this should
fix the contention on mmap_lock.

Could somebody try it?  I've verified it boots and runs some userspace
fine, but I don't have the workload to test the contention.

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Barry Song]

On Mon, Sep 28, 2026 at 6:55 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Tue, Sep 22, 2026 at 09:55:23AM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Mon, Sep 21, 2026 at 07:57:23PM +0100, Matthew Wilcox wrote:
> > > more to say on this in two weeks at Plumbers.
> >
> > I look forward to it :)
>
> So while doing my slides, I realised that what we need to avoid doing
> is (a) sleeping while holding the mmap_lock (b) returning RETRY while
> holding the VMA lock
>
> And that turns out to be as simple as this patch:
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index dd09c438fa23..94ed2333f8d8 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -723,6 +723,8 @@ enum {
>   */
>  static inline bool fault_flag_allow_retry_first(enum fault_flag flags)
>  {
> +       if (flags & FAULT_FLAG_VMA_LOCK)
> +               return false;
>         return (flags & FAULT_FLAG_ALLOW_RETRY) &&
>             (!(flags & FAULT_FLAG_TRIED));
>  }
>
> OK, this is a hack.  The function is spectacularly badly named, and
> needs to be renamed before a patch can go upstream.  But this should
> fix the contention on mmap_lock.

Thanks for your suggestion.
This is exactly what we did in Android Common Kernel before we had
Lorenzo's proposal (bypassing `fault_flag_allow_retry_first()`):

https://android.googlesource.com/kernel/common/+/1b9b045a586245cc1c29b2747c6586234c7f5bad%5E%21/#F2

At that time, we couldn't modify ACK with a GKI hook that introduced
a new flag. All previous versions of this patchset introduced new
flags, which would have broken the KMI.
So we had to choose between two evils:
1. mmap_lock contention
2. VMA lock contention
We chose the lesser evil — no. 2 — and sent the Android hook for
merging into ACK.

Now that we have Lorenzo's proposal, which doesn't require any new
flag, we are adding a new Android hook just to mimic what Lorenzo's
proposal already does:
https://android-review.googlesource.com/c/kernel/common/+/4307957/3/arch/arm64/mm/fault.c

Note that Lorenzo's proposal avoids mmap_lock contention without
introducing any new VMA lock contention. It also doesn't require a new
flag that would break KMI. So this is clearly the preferred approach.

>
> Could somebody try it?  I've verified it boots and runs some userspace
> fine, but I don't have the workload to test the contention.

Both Nanzhe and Hongru tested it before and reported the fork issue.

Best Regards
Barry

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Matthew Wilcox]

On Mon, Sep 28, 2026 at 10:48:41AM +0800, Barry Song wrote:
> On Mon, Sep 28, 2026 at 6:55 AM Matthew Wilcox <willy@infradead.org> wrote:
> > So while doing my slides, I realised that what we need to avoid doing
> > is (a) sleeping while holding the mmap_lock (b) returning RETRY while
> > holding the VMA lock
> >
> > And that turns out to be as simple as this patch:
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index dd09c438fa23..94ed2333f8d8 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -723,6 +723,8 @@ enum {
> >   */
> >  static inline bool fault_flag_allow_retry_first(enum fault_flag flags)
> >  {
> > +       if (flags & FAULT_FLAG_VMA_LOCK)
> > +               return false;
> >         return (flags & FAULT_FLAG_ALLOW_RETRY) &&
> >             (!(flags & FAULT_FLAG_TRIED));
> >  }
> >
> > OK, this is a hack.  The function is spectacularly badly named, and
> > needs to be renamed before a patch can go upstream.  But this should
> > fix the contention on mmap_lock.
> 
> Thanks for your suggestion.
> This is exactly what we did in Android Common Kernel before we had
> Lorenzo's proposal (bypassing `fault_flag_allow_retry_first()`):
> 
> https://android.googlesource.com/kernel/common/+/1b9b045a586245cc1c29b2747c6586234c7f5bad%5E%21/#F2

Looks like that one didn't cover __folio_lock_or_retry(), but that
doesn't invalidate your point.

> Note that Lorenzo's proposal avoids mmap_lock contention without
> introducing any new VMA lock contention. It also doesn't require a new
> flag that would break KMI. So this is clearly the preferred approach.

But it does retry multiple times in cases where we know the fault
will always fail (eg the fault is on a device-private VMA)

> > Could somebody try it?  I've verified it boots and runs some userspace
> > fine, but I don't have the workload to test the contention.
> 
> Both Nanzhe and Hongru tested it before and reported the fork issue.

So what I didn't realise is that fork() waits for page faults to finish.
I don't think that's necessary, so we can just stop doing that (whitespace
damaged):

diff --git a/mm/mmap.c b/mm/mmap.c
index 4bf26b0f1e6e..e79555247d3a 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1739,9 +1739,6 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
        for_each_vma(vmi, mpnt) {
                struct file *file;

-               retval = vma_start_write_killable(mpnt);
-               if (retval < 0)
-                       goto loop_out;
                if (vma_test(mpnt, VMA_DONTCOPY_BIT)) {
                        retval = vma_iter_clear_gfp(&vmi, mpnt->vm_start,
                                                    mpnt->vm_end, GFP_KERNEL);

I think this is safe.  I've booted a kernel with this change, and
everything seems to run fine.  Of course I don't have any multithreaded
applications which call fork() because that's a stupid way to write an
application, so it's not really tested.

My argument for why it's safe is that a thread which takes a page
fault during fork() might have taken the page fault either before or
after fork().  The faults will definitely happen in the parent process.
They may or may not have happened in the child process, which can't
possibly care whether or not they've happened.

The only difference I can think of being observable is that the child
may observe some later faults to have occurred, while some earlier faults
to have not occurred.  I have a hard time believing any application can
possibly depend on it.

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Lorenzo Stoakes (ARM)]

On Thu, Oct 01, 2026 at 10:12:16PM +0100, Matthew Wilcox wrote:
> So what I didn't realise is that fork() waits for page faults to finish.
> I don't think that's necessary, so we can just stop doing that (whitespace
> damaged):
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 4bf26b0f1e6e..e79555247d3a 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1739,9 +1739,6 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>         for_each_vma(vmi, mpnt) {
>                 struct file *file;
>
> -               retval = vma_start_write_killable(mpnt);
> -               if (retval < 0)
> -                       goto loop_out;
>                 if (vma_test(mpnt, VMA_DONTCOPY_BIT)) {
>                         retval = vma_iter_clear_gfp(&vmi, mpnt->vm_start,
>                                                     mpnt->vm_end, GFP_KERNEL);
>
> I think this is safe.  I've booted a kernel with this change, and

I'm afraid it's not safe :)

We already actually did this (by mistake I think?) before. See commit
fb49c455323f ("fork: lock VMAs of the parent process when forking") - we shipped
that in 6.4.0 -> 6.4.2 and it corrupted memory all over the place.

Reports + a consistent reproducer:

https://bugzilla.kernel.org/show_bug.cgi?id=217624
https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/

Also we have asserts for this lock in copy_page_range() and
copy_hugetlb_page_range() which will now fail (and possibly more further down
the stack?)

> My argument for why it's safe is that a thread which takes a page
> fault during fork() might have taken the page fault either before or
> after fork().  The faults will definitely happen in the parent process.
> They may or may not have happened in the child process, which can't
> possibly care whether or not they've happened.

The problem isn't the child it's the parent - copy_page_range() _modifies_ the
_parent_'s page tables in-place, on assumption that nothing else can access
them (for CoW).

And actually nowadays things are _even_ worse, you could also have concurrent
page faults, MADV_DONTNEED, MADV_FREE, MADV_GUARD_INSTALL/REMOVE, UFFDIO_COPY +
UFFDIO_MOVE (yikes) all of which might introduce new issues + would have to be
audited too.

So - it turns out a lot of stuff implicitly assumes VMA write lock now:

1. CoW - deferred TLB flush race

(The issue above)

https://lore.kernel.org/all/20230705171213.2843068-2-surenb@google.com/

This happens because __copy_present_ptes() does wrprotect_ptes() on the parent
without a TLB flush (that's deferred until the end of the fork).

But unfortunately without a VMA write lock something can then replace the page
table entry in the meantime and you can end up with data loss as a result.

Same issue exists with hugetlb wp too. And god only knows with uffd wp :)

2. THP - can destroy parent THP mappings

copy_pmd_range() does:

	if (pmd_is_huge(*src_pmd)) { copy_huge_pmd(); ... }
	if (pmd_none_or_clear_bad(src_pmd))
		continue;

Without the write lock, a concurrent fault can install a huge PMD via
do_huge_pmd_anonymous_page() or filemap_map_pmd()/do_set_pmd().

The issue is that pmd_bad() fires for leaf PMD (ugh) on at least x86 and arm64,
meaning that'll clear the page table entry.

I think Hugh left a comment alluding to this before:

	 * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
	 * error handling here, assume that exclusive mmap_lock on dst and src
	 * protects anon from unexpected THP transitions; ...

It's like the mess around pmd_trans_unstable() :(

3. copy_pte_range() skips the required pmd_same() recheck assuming write lock

It uses pte_offset_map_rw_nolock() with a dummy pmdval and:

	 * We already hold the exclusive mmap_lock, the copy_pte_range() and
	 * retract_page_tables() are using vma->anon_vma to be exclusive, so
	 * the PTE page is stable, and there is no need to get pmdval and do
	 * pmd_same() check.

(Probably needs updating for VMA write lock...)

4. The asserts

Mentioned above!

I suspect there's more as well.

Some of it could be changed, but it'd all be at the cost of behaving very
strangely (no VMA lock but mmap write lock) while manipulating things.

And I am not really happy with changing page table entries in the parent VMA
(for CoW) without a VMA write lock held.

I also wonder whether it would achieve all that much really - the issue is with
holding the VMA lock across I/O, anything else that tries to grab a write lock
is going to end up being the new thing that's blocked (mprotect, munmap, mremap,
mlock etc.)

I think doing something like this would need a very significant redesign and I'm
not convinced it's one that really achieves what we want given all the other
cases that'd get blocked by the VMA lock being held still.

--
Cheers, Lorenzo

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Barry Song]

On Fri, Oct 2, 2026 at 5:12 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Sep 28, 2026 at 10:48:41AM +0800, Barry Song wrote:
> > On Mon, Sep 28, 2026 at 6:55 AM Matthew Wilcox <willy@infradead.org> wrote:
> > > So while doing my slides, I realised that what we need to avoid doing
> > > is (a) sleeping while holding the mmap_lock (b) returning RETRY while
> > > holding the VMA lock
> > >
> > > And that turns out to be as simple as this patch:
> > >
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index dd09c438fa23..94ed2333f8d8 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -723,6 +723,8 @@ enum {
> > >   */
> > >  static inline bool fault_flag_allow_retry_first(enum fault_flag flags)
> > >  {
> > > +       if (flags & FAULT_FLAG_VMA_LOCK)
> > > +               return false;
> > >         return (flags & FAULT_FLAG_ALLOW_RETRY) &&
> > >             (!(flags & FAULT_FLAG_TRIED));
> > >  }
> > >
> > > OK, this is a hack.  The function is spectacularly badly named, and
> > > needs to be renamed before a patch can go upstream.  But this should
> > > fix the contention on mmap_lock.
> >
> > Thanks for your suggestion.
> > This is exactly what we did in Android Common Kernel before we had
> > Lorenzo's proposal (bypassing `fault_flag_allow_retry_first()`):
> >
> > https://android.googlesource.com/kernel/common/+/1b9b045a586245cc1c29b2747c6586234c7f5bad%5E%21/#F2
>
> Looks like that one didn't cover __folio_lock_or_retry(), but that
> doesn't invalidate your point.
Yep. `__folio_lock_or_retry()` will make the same thing true for anon
VMAs, so we intentionally made the hook valid only for file VMAs.Only touching the file retry path seems to involve less VMA
contention.

>
> > Note that Lorenzo's proposal avoids mmap_lock contention without
> > introducing any new VMA lock contention. It also doesn't require a new
> > flag that would break KMI. So this is clearly the preferred approach.
>
> But it does retry multiple times in cases where we know the fault
> will always fail (eg the fault is on a device-private VMA)
>
Right now, these might require a single extra retry for device-private
and `__vmf_anon_prepare()` cases. The commit log also mentions this.

"Some faults may
retry unnecessarily, for example, those in __vmf_anon_prepare() or
device-private fault handling, which require the mmap_lock. However,
these cases are expected to be infrequent and only add one cheap
per-VMA lock attempt."

If we do want to remove the extra VMA lock attempt, we might still
need an additional flag such as VM_FAULT_NEED_MMAP_LOCK:

diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 2cecf6ba6df7..2821e3bd7462 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -620,6 +620,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 	struct vm_area_struct *vma;
 	int si_code;
 	int pkey = -1;
+	bool vma_lock_retried = false;
 
 	if (kprobe_page_fault(regs, esr))
 		return 0;
@@ -688,6 +689,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 	if (!(mm_flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+lock_vma:
 	vma = lock_vma_under_rcu(mm, addr);
 	if (!vma)
 		goto lock_mmap;
@@ -734,6 +736,12 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 			goto no_context;
 		return 0;
 	}
+
+	if (!vma_lock_retried && !(fault & VM_FAULT_NEED_MMAP_LOCK)) {
+		vma_lock_retried = true;
+		goto lock_vma;
+	}
+
 lock_mmap:
 
 retry:
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 6141160ec652..baadab598d98 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1734,10 +1734,11 @@ enum vm_fault_reason {
 	VM_FAULT_NOPAGE         = (__force vm_fault_t)0x000100,
 	VM_FAULT_LOCKED         = (__force vm_fault_t)0x000200,
 	VM_FAULT_RETRY          = (__force vm_fault_t)0x000400,
-	VM_FAULT_FALLBACK       = (__force vm_fault_t)0x000800,
-	VM_FAULT_DONE_COW       = (__force vm_fault_t)0x001000,
-	VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x002000,
-	VM_FAULT_COMPLETED      = (__force vm_fault_t)0x004000,
+	VM_FAULT_NEED_MMAP_LOCK     = (__force vm_fault_t)0x000800,
+	VM_FAULT_FALLBACK       = (__force vm_fault_t)0x001000,
+	VM_FAULT_DONE_COW       = (__force vm_fault_t)0x002000,
+	VM_FAULT_NEEDDSYNC      = (__force vm_fault_t)0x004000,
+	VM_FAULT_COMPLETED      = (__force vm_fault_t)0x008000,
 	VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
 };
 
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index ddc631a388b9..64e08c6153cf 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1480,7 +1480,7 @@ vm_fault_t do_huge_pmd_device_private(struct vm_fault *vmf)
 
 	if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
 		vma_end_read(vma);
-		return VM_FAULT_RETRY;
+		return VM_FAULT_RETRY | VM_FAULT_NEED_MMAP_LOCK;
 	}
 
 	ptl = pmd_lock(vma->vm_mm, vmf->pmd);
diff --git a/mm/memory.c b/mm/memory.c
index 1f83a26f8733..89c8c8a52c16 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3980,7 +3980,7 @@ vm_fault_t __vmf_anon_prepare(struct vm_fault *vmf)
 		return 0;
 	if (vmf->flags & FAULT_FLAG_VMA_LOCK) {
 		if (!mmap_read_trylock(vma->vm_mm))
-			return VM_FAULT_RETRY;
+			return VM_FAULT_RETRY | VM_FAULT_NEED_MMAP_LOCK;
 	}
 	if (__anon_vma_prepare(vma))
 		ret = VM_FAULT_OOM;
@@ -4937,7 +4937,7 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
 				 * under VMA lock.
 				 */
 				vma_end_read(vma);
-				ret = VM_FAULT_RETRY;
+				ret = VM_FAULT_RETRY | VM_FAULT_NEED_MMAP_LOCK;
 				goto out;
 			}
 
-- 
2.39.3 (Apple Git-146)

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Matthew Wilcox]

On Fri, Oct 02, 2026 at 02:12:35PM +0100, Lorenzo Stoakes (ARM) wrote:
> On Thu, Oct 01, 2026 at 10:12:16PM +0100, Matthew Wilcox wrote:
> > So what I didn't realise is that fork() waits for page faults to finish.
> > I don't think that's necessary, so we can just stop doing that (whitespace
> > damaged):
> > I think this is safe.  I've booted a kernel with this change, and
> 
> I'm afraid it's not safe :)
> 
> We already actually did this (by mistake I think?) before. See commit
> fb49c455323f ("fork: lock VMAs of the parent process when forking") - we shipped
> that in 6.4.0 -> 6.4.2 and it corrupted memory all over the place.
> 
> Reports + a consistent reproducer:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=217624
> https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
> https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/

I wonder if we still need this.  The commit message says:

    A concurrent page fault on a page newly marked read-only by the page
    copy might trigger wp_page_copy() and a anon_vma_prepare(vma) on the
    source vma, defeating the anon_vma_clone() that wasn't done because the
    parent vma originally didn't have an anon_vma, but we now might end up
    copying a pte entry for a page that has one.

But we're holding the mmap_lock for write here, and we need to hold the
mmap_lock for read in order to change a vma->anon_vma (asserted in
__anon_vma_prepare() with mmap_assert_locked()).

At the time of this commit, lock_vma_under_rcu() contained:

        /* find_mergeable_anon_vma uses adjacent vmas which are not locked */
        if (!vma->anon_vma && !vma_is_tcp(vma))
                goto inval;

so we really did have to wait for read holders to finish the fault.
But commit a373baed5a9d deferred the check for a NULL anon VMA until we
actually needed it, so I think commit fb49c455323f's logic was invalidated
by a373baed5a9d.

Maybe I'm wrong.  This is all a bit out of my normal area.

> Also we have asserts for this lock in copy_page_range() and
> copy_hugetlb_page_range() which will now fail (and possibly more further down
> the stack?)

Possibly we have to adjust some assertions to check that either we have
the VMA locked or we have the MM write locked.  That doesn't seem
unreasonable.

> > My argument for why it's safe is that a thread which takes a page
> > fault during fork() might have taken the page fault either before or
> > after fork().  The faults will definitely happen in the parent process.
> > They may or may not have happened in the child process, which can't
> > possibly care whether or not they've happened.
> 
> The problem isn't the child it's the parent - copy_page_range() _modifies_ the
> _parent_'s page tables in-place, on assumption that nothing else can access
> them (for CoW).

I was perhaps unclear.  What I mean is that the child may or may not
observe them as having happened.

> And actually nowadays things are _even_ worse, you could also have concurrent
> page faults, MADV_DONTNEED, MADV_FREE, MADV_GUARD_INSTALL/REMOVE, UFFDIO_COPY +
> UFFDIO_MOVE (yikes) all of which might introduce new issues + would have to be
> audited too.

Yup.

> So - it turns out a lot of stuff implicitly assumes VMA write lock now:
> 
> 1. CoW - deferred TLB flush race
> 
> (The issue above)
> 
> https://lore.kernel.org/all/20230705171213.2843068-2-surenb@google.com/
> 
> This happens because __copy_present_ptes() does wrprotect_ptes() on the parent
> without a TLB flush (that's deferred until the end of the fork).
> 
> But unfortunately without a VMA write lock something can then replace the page
> table entry in the meantime and you can end up with data loss as a result.

Is it that fb49c455323f commit message is insufficient and we need to
prevent simultaneous fork/page fault for other reasons, then?

> Same issue exists with hugetlb wp too. And god only knows with uffd wp :)
> 
> 2. THP - can destroy parent THP mappings
> 
> copy_pmd_range() does:
> 
> 	if (pmd_is_huge(*src_pmd)) { copy_huge_pmd(); ... }
> 	if (pmd_none_or_clear_bad(src_pmd))
> 		continue;
> 
> Without the write lock, a concurrent fault can install a huge PMD via
> do_huge_pmd_anonymous_page() or filemap_map_pmd()/do_set_pmd().
> 
> The issue is that pmd_bad() fires for leaf PMD (ugh) on at least x86 and arm64,
> meaning that'll clear the page table entry.
> 
> I think Hugh left a comment alluding to this before:
> 
> 	 * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
> 	 * error handling here, assume that exclusive mmap_lock on dst and src
> 	 * protects anon from unexpected THP transitions; ...
> 
> It's like the mess around pmd_trans_unstable() :(

... could we fix this by taking the ptlock during the copy operation?

> 3. copy_pte_range() skips the required pmd_same() recheck assuming write lock
> 
> It uses pte_offset_map_rw_nolock() with a dummy pmdval and:
> 
> 	 * We already hold the exclusive mmap_lock, the copy_pte_range() and
> 	 * retract_page_tables() are using vma->anon_vma to be exclusive, so
> 	 * the PTE page is stable, and there is no need to get pmdval and do
> 	 * pmd_same() check.
> 
> (Probably needs updating for VMA write lock...)
> 
> 4. The asserts
> 
> Mentioned above!
> 
> I suspect there's more as well.
> 
> Some of it could be changed, but it'd all be at the cost of behaving very
> strangely (no VMA lock but mmap write lock) while manipulating things.
> 
> And I am not really happy with changing page table entries in the parent VMA
> (for CoW) without a VMA write lock held.
> 
> I also wonder whether it would achieve all that much really - the issue is with
> holding the VMA lock across I/O, anything else that tries to grab a write lock
> is going to end up being the new thing that's blocked (mprotect, munmap, mremap,
> mlock etc.)
> 
> I think doing something like this would need a very significant redesign and I'm
> not convinced it's one that really achieves what we want given all the other
> cases that'd get blocked by the VMA lock being held still.
> 
> --
> Cheers, Lorenzo

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Suren Baghdasaryan]

On Mon, Oct 19, 2026 at 1:53 PM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Fri, Oct 02, 2026 at 02:12:35PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Oct 01, 2026 at 10:12:16PM +0100, Matthew Wilcox wrote:
> > > So what I didn't realise is that fork() waits for page faults to finish.
> > > I don't think that's necessary, so we can just stop doing that (whitespace
> > > damaged):
> > > I think this is safe.  I've booted a kernel with this change, and
> >
> > I'm afraid it's not safe :)
> >
> > We already actually did this (by mistake I think?) before. See commit
> > fb49c455323f ("fork: lock VMAs of the parent process when forking") - we shipped
> > that in 6.4.0 -> 6.4.2 and it corrupted memory all over the place.
> >
> > Reports + a consistent reproducer:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=217624
> > https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
> > https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/
>
> I wonder if we still need this.  The commit message says:
>
>     A concurrent page fault on a page newly marked read-only by the page
>     copy might trigger wp_page_copy() and a anon_vma_prepare(vma) on the
>     source vma, defeating the anon_vma_clone() that wasn't done because the
>     parent vma originally didn't have an anon_vma, but we now might end up
>     copying a pte entry for a page that has one.
>
> But we're holding the mmap_lock for write here, and we need to hold the
> mmap_lock for read in order to change a vma->anon_vma (asserted in
> __anon_vma_prepare() with mmap_assert_locked()).
>
> At the time of this commit, lock_vma_under_rcu() contained:
>
>         /* find_mergeable_anon_vma uses adjacent vmas which are not locked */
>         if (!vma->anon_vma && !vma_is_tcp(vma))
>                 goto inval;
>
> so we really did have to wait for read holders to finish the fault.
> But commit a373baed5a9d deferred the check for a NULL anon VMA until we
> actually needed it, so I think commit fb49c455323f's logic was invalidated
> by a373baed5a9d.
>
> Maybe I'm wrong.  This is all a bit out of my normal area.
>
> > Also we have asserts for this lock in copy_page_range() and
> > copy_hugetlb_page_range() which will now fail (and possibly more further down
> > the stack?)
>
> Possibly we have to adjust some assertions to check that either we have
> the VMA locked or we have the MM write locked.  That doesn't seem
> unreasonable.
>
> > > My argument for why it's safe is that a thread which takes a page
> > > fault during fork() might have taken the page fault either before or
> > > after fork().  The faults will definitely happen in the parent process.
> > > They may or may not have happened in the child process, which can't
> > > possibly care whether or not they've happened.
> >
> > The problem isn't the child it's the parent - copy_page_range() _modifies_ the
> > _parent_'s page tables in-place, on assumption that nothing else can access
> > them (for CoW).
>
> I was perhaps unclear.  What I mean is that the child may or may not
> observe them as having happened.
>
> > And actually nowadays things are _even_ worse, you could also have concurrent
> > page faults, MADV_DONTNEED, MADV_FREE, MADV_GUARD_INSTALL/REMOVE, UFFDIO_COPY +
> > UFFDIO_MOVE (yikes) all of which might introduce new issues + would have to be
> > audited too.
>
> Yup.
>
> > So - it turns out a lot of stuff implicitly assumes VMA write lock now:
> >
> > 1. CoW - deferred TLB flush race
> >
> > (The issue above)
> >
> > https://lore.kernel.org/all/20230705171213.2843068-2-surenb@google.com/
> >
> > This happens because __copy_present_ptes() does wrprotect_ptes() on the parent
> > without a TLB flush (that's deferred until the end of the fork).
> >
> > But unfortunately without a VMA write lock something can then replace the page
> > table entry in the meantime and you can end up with data loss as a result.
>
> Is it that fb49c455323f commit message is insufficient and we need to
> prevent simultaneous fork/page fault for other reasons, then?

Yes, I think so. It refers to one of the reasons VMA lock is needed
during fork, but as Lorenzo points out, there are a number of other
reasons that require fork to protect VMAs. Note that some of those
reasons, like MADV_GUARD_INSTALL and UFFDIO_MOVE did not exist at the
time fb49c455323f was written.

Below is the list that AI pointed out, and I think Lorenzo covered
most if not all of them:

Without vma_start_write_killable(mpnt):
      1. In-flight VMA readers that locked mpnt before
mmap_write_lock_killable(oldmm) continue running concurrently with
dup_mmap().
      2. New VMA readers (page faults via lock_vma_under_rcu(),
madvise under MADVISE_VMA_READ_LOCK, userfaultfd via uffd_lock_vma() /
uffd_move_lock(), vma_start_read_unlocked(), etc.) continue to succeed
on mpnt while dup_mmap() duplicates its VMA metadata and page tables.

  (Note: When this lock was originally added in fb49c455323f, one
prominent trigger was a concurrent fault calling anon_vma_prepare() on
a VMA with mpnt->anon_vma == NULL while anon_vma_fork() /
vma_needs_copy() ran. Although __vmf_anon_prepare() now requires
mmap_read_trylock()
  when !vma->anon_vma, removing vma_start_write_killable(mpnt) today
still breaks several critical invariants.)
  ──────
  ### What would happen if we removed it

  1. **Destruction of live 2MB THP mappings ("bad pmd" -> pmd_clear())
in copy_pmd_range()**
  As noted in the comment in copy_pte_range(), copy_pmd_range()
assumes no concurrent THP transitions on src_pmd:
    if (pmd_is_huge(*src_pmd)) { ... }
    if (pmd_none_or_clear_bad(src_pmd))
        continue;
  On an anonymous VMA that already has mpnt->anon_vma != NULL, a
concurrent per-VMA-locked THP fault (do_huge_pmd_anonymous_page()) or
UFFDIO_MOVE can install a transparent huge PMD into *src_pmd right
after pmd_is_huge(*src_pmd) observed pmd_none.
pmd_none_or_clear_bad(src_pmd)
  then sees pmd_bad(*src_pmd) == true (a THP leaf PMD satisfies
pmd_bad()), prints "bad pmd ...", and calls pmd_clear_bad(src_pmd) —
wiping out the parent's live 2MB PMD without PTL, TLB flush, or
rmap/refcount/deposited-pgtable/RSS cleanup (corrupting parent memory
and
  triggering Bad rss-counter / nr_ptes bugs on exit).
      • The same race exists for huge PUDs in copy_pud_range()
(pud_none_or_clear_bad(src_pud)).
      • Similarly, copy_huge_pmd() reads special huge PMDs
(VM_PFNMAP/VM_MIXEDMAP) locklessly and skips rechecking *src_pmd under
src_ptl assuming it cannot change, so a concurrent MADV_DONTNEED under
MADVISE_VMA_READ_LOCK could leave the child mapping a stale/zapped PFN
PMD.
  2. **Silent CoW data corruption / lost parent writes due to deferred
flush_tlb_mm(oldmm)**
  __copy_present_ptes(), copy_huge_pmd(), and
copy_hugetlb_page_range() write-protect parent PTEs/PMDs without
flushing TLBs; dup_mmap() defers the flush to flush_tlb_mm(oldmm)
after all VMAs are copied. Normally, because each copied VMA remains
VMA-write-locked until
  mmap_write_unlock(oldmm), no parent thread can handle a CoW write
fault before flush_tlb_mm(oldmm) finishes.
  Without the VMA write lock, while CPU 0 is still in dup_mmap()
copying later VMAs:
      • CPU 1 (parent thread A) still holds a stale writable TLB entry
to old_folio (now CoW-shared with the child).
      • CPU 2 (parent thread B, no cached TLB entry) writes to the
same page, faults on the R/O PTE under per-VMA read lock, enters
wp_page_copy(), and copies old_folio into new_folio
(__wp_page_copy_user()) before taking PTL and calling
ptep_clear_flush().
      • Any store by CPU 1 through its stale writable TLB entry after
__wp_page_copy_user() and before ptep_clear_flush() lands directly in
old_folio while pte_same() still succeeds — losing thread A's write in
the parent (new_folio) and leaking it into the child (old_folio).
  3. Broken guard-page (MADV_GUARD_INSTALL) state and KCSAN race on vm_flags
  madvise(MADV_GUARD_INSTALL) runs under MADVISE_VMA_READ_LOCK and
atomically sets VMA_MAYBE_GUARD_BIT on mpnt via vma_set_atomic_flag()
before installing PTE_MARKER_GUARD entries:
      • vm_area_dup(mpnt) asserts
ASSERT_EXCLUSIVE_WRITER(orig->vm_flags), which would trip under KCSAN.
      • If vm_area_dup(mpnt) copies mpnt->vm_flags just before
MADV_GUARD_INSTALL sets VMA_MAYBE_GUARD_BIT and installs
PTE_MARKER_GUARD PTEs:
          • On a file-backed VMA without anon_vma, tmp misses
VM_MAYBE_GUARD (part of VM_COPY_ON_FORK), so vma_needs_copy() returns
false and drops the guard regions in the child.
          • On a VMA with anon_vma != NULL, copy_page_range() copies
the PTE_MARKER_GUARD entries into tmp, leaving the child VMA with
guard PTE markers without VM_MAYBE_GUARD set (breaking collapse checks
such as file_backed_vma_is_retractable() and future merges/forks).

  4. Inconsistent address-space snapshots against UFFDIO_MOVE and
tcp_zerocopy_receive()
      • When UFFD_FEATURE_EVENT_FORK is not enabled, dup_userfaultfd()
never increments octx->mmap_changing. Because move_pages()
(UFFDIO_MOVE) only takes per-VMA read locks (uffd_move_lock()), it
could move an exclusive folio from a not-yet-copied src address/VMA
into an
      already-copied dst address/VMA (losing the page at both
addresses in the child), or move a zero page from an already-copied
src to a not-yet-copied dst (duplicating it at both). Even with
UFFD_FEATURE_EVENT_FORK, octx->mmap_changing is only incremented once
dup_mmap()
      reaches the first VMA using octx, so earlier non-uffd VMAs rely
on VMA write-locking to prevent cross-VMA moves.
      • tcp_mmap() sets VM_MIXEDMAP (in VM_COPY_ON_FORK, without
VM_DONTCOPY), so copy_page_range() would race with
tcp_zerocopy_receive() zapping and inserting pages under
vma_start_read_unlocked().
  5. Immediate debug assertions (CONFIG_DEBUG_VM)
  Both copy_page_range() and copy_hugetlb_page_range() call
vma_assert_write_locked(src_vma) on every CoW VMA, which immediately
fires VM_WARN_ON_ONCE_VMA(!__is_vma_write_locked(vma), vma) because
oldmm->mm_lock_seq was bumped by mmap_write_lock_killable(oldmm) while
mpnt-
  >vm_lock_seq was never updated.

>
> > Same issue exists with hugetlb wp too. And god only knows with uffd wp :)
> >
> > 2. THP - can destroy parent THP mappings
> >
> > copy_pmd_range() does:
> >
> >       if (pmd_is_huge(*src_pmd)) { copy_huge_pmd(); ... }
> >       if (pmd_none_or_clear_bad(src_pmd))
> >               continue;
> >
> > Without the write lock, a concurrent fault can install a huge PMD via
> > do_huge_pmd_anonymous_page() or filemap_map_pmd()/do_set_pmd().
> >
> > The issue is that pmd_bad() fires for leaf PMD (ugh) on at least x86 and arm64,
> > meaning that'll clear the page table entry.
> >
> > I think Hugh left a comment alluding to this before:
> >
> >        * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
> >        * error handling here, assume that exclusive mmap_lock on dst and src
> >        * protects anon from unexpected THP transitions; ...
> >
> > It's like the mess around pmd_trans_unstable() :(
>
> ... could we fix this by taking the ptlock during the copy operation?
>
> > 3. copy_pte_range() skips the required pmd_same() recheck assuming write lock
> >
> > It uses pte_offset_map_rw_nolock() with a dummy pmdval and:
> >
> >        * We already hold the exclusive mmap_lock, the copy_pte_range() and
> >        * retract_page_tables() are using vma->anon_vma to be exclusive, so
> >        * the PTE page is stable, and there is no need to get pmdval and do
> >        * pmd_same() check.
> >
> > (Probably needs updating for VMA write lock...)
> >
> > 4. The asserts
> >
> > Mentioned above!
> >
> > I suspect there's more as well.
> >
> > Some of it could be changed, but it'd all be at the cost of behaving very
> > strangely (no VMA lock but mmap write lock) while manipulating things.
> >
> > And I am not really happy with changing page table entries in the parent VMA
> > (for CoW) without a VMA write lock held.
> >
> > I also wonder whether it would achieve all that much really - the issue is with
> > holding the VMA lock across I/O, anything else that tries to grab a write lock
> > is going to end up being the new thing that's blocked (mprotect, munmap, mremap,
> > mlock etc.)
> >
> > I think doing something like this would need a very significant redesign and I'm
> > not convinced it's one that really achieves what we want given all the other
> > cases that'd get blocked by the VMA lock being held still.
> >
> > --
> > Cheers, Lorenzo

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Suren Baghdasaryan]

On Mon, Oct 26, 2026 at 7:39 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Mon, Oct 19, 2026 at 1:53 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > On Fri, Oct 02, 2026 at 02:12:35PM +0100, Lorenzo Stoakes (ARM) wrote:
> > > On Thu, Oct 01, 2026 at 10:12:16PM +0100, Matthew Wilcox wrote:
> > > > So what I didn't realise is that fork() waits for page faults to finish.
> > > > I don't think that's necessary, so we can just stop doing that (whitespace
> > > > damaged):
> > > > I think this is safe.  I've booted a kernel with this change, and
> > >
> > > I'm afraid it's not safe :)
> > >
> > > We already actually did this (by mistake I think?) before. See commit
> > > fb49c455323f ("fork: lock VMAs of the parent process when forking") - we shipped
> > > that in 6.4.0 -> 6.4.2 and it corrupted memory all over the place.
> > >
> > > Reports + a consistent reproducer:
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=217624
> > > https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
> > > https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/
> >
> > I wonder if we still need this.  The commit message says:
> >
> >     A concurrent page fault on a page newly marked read-only by the page
> >     copy might trigger wp_page_copy() and a anon_vma_prepare(vma) on the
> >     source vma, defeating the anon_vma_clone() that wasn't done because the
> >     parent vma originally didn't have an anon_vma, but we now might end up
> >     copying a pte entry for a page that has one.
> >
> > But we're holding the mmap_lock for write here, and we need to hold the
> > mmap_lock for read in order to change a vma->anon_vma (asserted in
> > __anon_vma_prepare() with mmap_assert_locked()).
> >
> > At the time of this commit, lock_vma_under_rcu() contained:
> >
> >         /* find_mergeable_anon_vma uses adjacent vmas which are not locked */
> >         if (!vma->anon_vma && !vma_is_tcp(vma))
> >                 goto inval;
> >
> > so we really did have to wait for read holders to finish the fault.
> > But commit a373baed5a9d deferred the check for a NULL anon VMA until we
> > actually needed it, so I think commit fb49c455323f's logic was invalidated
> > by a373baed5a9d.
> >
> > Maybe I'm wrong.  This is all a bit out of my normal area.
> >
> > > Also we have asserts for this lock in copy_page_range() and
> > > copy_hugetlb_page_range() which will now fail (and possibly more further down
> > > the stack?)
> >
> > Possibly we have to adjust some assertions to check that either we have
> > the VMA locked or we have the MM write locked.  That doesn't seem
> > unreasonable.
> >
> > > > My argument for why it's safe is that a thread which takes a page
> > > > fault during fork() might have taken the page fault either before or
> > > > after fork().  The faults will definitely happen in the parent process.
> > > > They may or may not have happened in the child process, which can't
> > > > possibly care whether or not they've happened.
> > >
> > > The problem isn't the child it's the parent - copy_page_range() _modifies_ the
> > > _parent_'s page tables in-place, on assumption that nothing else can access
> > > them (for CoW).
> >
> > I was perhaps unclear.  What I mean is that the child may or may not
> > observe them as having happened.
> >
> > > And actually nowadays things are _even_ worse, you could also have concurrent
> > > page faults, MADV_DONTNEED, MADV_FREE, MADV_GUARD_INSTALL/REMOVE, UFFDIO_COPY +
> > > UFFDIO_MOVE (yikes) all of which might introduce new issues + would have to be
> > > audited too.
> >
> > Yup.
> >
> > > So - it turns out a lot of stuff implicitly assumes VMA write lock now:
> > >
> > > 1. CoW - deferred TLB flush race
> > >
> > > (The issue above)
> > >
> > > https://lore.kernel.org/all/20230705171213.2843068-2-surenb@google.com/
> > >
> > > This happens because __copy_present_ptes() does wrprotect_ptes() on the parent
> > > without a TLB flush (that's deferred until the end of the fork).
> > >
> > > But unfortunately without a VMA write lock something can then replace the page
> > > table entry in the meantime and you can end up with data loss as a result.
> >
> > Is it that fb49c455323f commit message is insufficient and we need to
> > prevent simultaneous fork/page fault for other reasons, then?
>
> Yes, I think so. It refers to one of the reasons VMA lock is needed
> during fork, but as Lorenzo points out, there are a number of other
> reasons that require fork to protect VMAs. Note that some of those
> reasons, like MADV_GUARD_INSTALL and UFFDIO_MOVE did not exist at the
> time fb49c455323f was written.

I also have to agree with Lorenzo that allowing fork to run
concurrently with page faults might only shift the mmap_lock
contention from fork to some other operation (mmap/mprotect/etc.), so
even if we fix all these cases we might end up with contention
somewhere else.

>
> Below is the list that AI pointed out, and I think Lorenzo covered
> most if not all of them:
>
> Without vma_start_write_killable(mpnt):
>       1. In-flight VMA readers that locked mpnt before
> mmap_write_lock_killable(oldmm) continue running concurrently with
> dup_mmap().
>       2. New VMA readers (page faults via lock_vma_under_rcu(),
> madvise under MADVISE_VMA_READ_LOCK, userfaultfd via uffd_lock_vma() /
> uffd_move_lock(), vma_start_read_unlocked(), etc.) continue to succeed
> on mpnt while dup_mmap() duplicates its VMA metadata and page tables.
>
>   (Note: When this lock was originally added in fb49c455323f, one
> prominent trigger was a concurrent fault calling anon_vma_prepare() on
> a VMA with mpnt->anon_vma == NULL while anon_vma_fork() /
> vma_needs_copy() ran. Although __vmf_anon_prepare() now requires
> mmap_read_trylock()
>   when !vma->anon_vma, removing vma_start_write_killable(mpnt) today
> still breaks several critical invariants.)
>   ──────
>   ### What would happen if we removed it
>
>   1. **Destruction of live 2MB THP mappings ("bad pmd" -> pmd_clear())
> in copy_pmd_range()**
>   As noted in the comment in copy_pte_range(), copy_pmd_range()
> assumes no concurrent THP transitions on src_pmd:
>     if (pmd_is_huge(*src_pmd)) { ... }
>     if (pmd_none_or_clear_bad(src_pmd))
>         continue;
>   On an anonymous VMA that already has mpnt->anon_vma != NULL, a
> concurrent per-VMA-locked THP fault (do_huge_pmd_anonymous_page()) or
> UFFDIO_MOVE can install a transparent huge PMD into *src_pmd right
> after pmd_is_huge(*src_pmd) observed pmd_none.
> pmd_none_or_clear_bad(src_pmd)
>   then sees pmd_bad(*src_pmd) == true (a THP leaf PMD satisfies
> pmd_bad()), prints "bad pmd ...", and calls pmd_clear_bad(src_pmd) —
> wiping out the parent's live 2MB PMD without PTL, TLB flush, or
> rmap/refcount/deposited-pgtable/RSS cleanup (corrupting parent memory
> and
>   triggering Bad rss-counter / nr_ptes bugs on exit).
>       • The same race exists for huge PUDs in copy_pud_range()
> (pud_none_or_clear_bad(src_pud)).
>       • Similarly, copy_huge_pmd() reads special huge PMDs
> (VM_PFNMAP/VM_MIXEDMAP) locklessly and skips rechecking *src_pmd under
> src_ptl assuming it cannot change, so a concurrent MADV_DONTNEED under
> MADVISE_VMA_READ_LOCK could leave the child mapping a stale/zapped PFN
> PMD.
>   2. **Silent CoW data corruption / lost parent writes due to deferred
> flush_tlb_mm(oldmm)**
>   __copy_present_ptes(), copy_huge_pmd(), and
> copy_hugetlb_page_range() write-protect parent PTEs/PMDs without
> flushing TLBs; dup_mmap() defers the flush to flush_tlb_mm(oldmm)
> after all VMAs are copied. Normally, because each copied VMA remains
> VMA-write-locked until
>   mmap_write_unlock(oldmm), no parent thread can handle a CoW write
> fault before flush_tlb_mm(oldmm) finishes.
>   Without the VMA write lock, while CPU 0 is still in dup_mmap()
> copying later VMAs:
>       • CPU 1 (parent thread A) still holds a stale writable TLB entry
> to old_folio (now CoW-shared with the child).
>       • CPU 2 (parent thread B, no cached TLB entry) writes to the
> same page, faults on the R/O PTE under per-VMA read lock, enters
> wp_page_copy(), and copies old_folio into new_folio
> (__wp_page_copy_user()) before taking PTL and calling
> ptep_clear_flush().
>       • Any store by CPU 1 through its stale writable TLB entry after
> __wp_page_copy_user() and before ptep_clear_flush() lands directly in
> old_folio while pte_same() still succeeds — losing thread A's write in
> the parent (new_folio) and leaking it into the child (old_folio).
>   3. Broken guard-page (MADV_GUARD_INSTALL) state and KCSAN race on vm_flags
>   madvise(MADV_GUARD_INSTALL) runs under MADVISE_VMA_READ_LOCK and
> atomically sets VMA_MAYBE_GUARD_BIT on mpnt via vma_set_atomic_flag()
> before installing PTE_MARKER_GUARD entries:
>       • vm_area_dup(mpnt) asserts
> ASSERT_EXCLUSIVE_WRITER(orig->vm_flags), which would trip under KCSAN.
>       • If vm_area_dup(mpnt) copies mpnt->vm_flags just before
> MADV_GUARD_INSTALL sets VMA_MAYBE_GUARD_BIT and installs
> PTE_MARKER_GUARD PTEs:
>           • On a file-backed VMA without anon_vma, tmp misses
> VM_MAYBE_GUARD (part of VM_COPY_ON_FORK), so vma_needs_copy() returns
> false and drops the guard regions in the child.
>           • On a VMA with anon_vma != NULL, copy_page_range() copies
> the PTE_MARKER_GUARD entries into tmp, leaving the child VMA with
> guard PTE markers without VM_MAYBE_GUARD set (breaking collapse checks
> such as file_backed_vma_is_retractable() and future merges/forks).
>
>   4. Inconsistent address-space snapshots against UFFDIO_MOVE and
> tcp_zerocopy_receive()
>       • When UFFD_FEATURE_EVENT_FORK is not enabled, dup_userfaultfd()
> never increments octx->mmap_changing. Because move_pages()
> (UFFDIO_MOVE) only takes per-VMA read locks (uffd_move_lock()), it
> could move an exclusive folio from a not-yet-copied src address/VMA
> into an
>       already-copied dst address/VMA (losing the page at both
> addresses in the child), or move a zero page from an already-copied
> src to a not-yet-copied dst (duplicating it at both). Even with
> UFFD_FEATURE_EVENT_FORK, octx->mmap_changing is only incremented once
> dup_mmap()
>       reaches the first VMA using octx, so earlier non-uffd VMAs rely
> on VMA write-locking to prevent cross-VMA moves.
>       • tcp_mmap() sets VM_MIXEDMAP (in VM_COPY_ON_FORK, without
> VM_DONTCOPY), so copy_page_range() would race with
> tcp_zerocopy_receive() zapping and inserting pages under
> vma_start_read_unlocked().
>   5. Immediate debug assertions (CONFIG_DEBUG_VM)
>   Both copy_page_range() and copy_hugetlb_page_range() call
> vma_assert_write_locked(src_vma) on every CoW VMA, which immediately
> fires VM_WARN_ON_ONCE_VMA(!__is_vma_write_locked(vma), vma) because
> oldmm->mm_lock_seq was bumped by mmap_write_lock_killable(oldmm) while
> mpnt-
>   >vm_lock_seq was never updated.
>
> >
> > > Same issue exists with hugetlb wp too. And god only knows with uffd wp :)
> > >
> > > 2. THP - can destroy parent THP mappings
> > >
> > > copy_pmd_range() does:
> > >
> > >       if (pmd_is_huge(*src_pmd)) { copy_huge_pmd(); ... }
> > >       if (pmd_none_or_clear_bad(src_pmd))
> > >               continue;
> > >
> > > Without the write lock, a concurrent fault can install a huge PMD via
> > > do_huge_pmd_anonymous_page() or filemap_map_pmd()/do_set_pmd().
> > >
> > > The issue is that pmd_bad() fires for leaf PMD (ugh) on at least x86 and arm64,
> > > meaning that'll clear the page table entry.
> > >
> > > I think Hugh left a comment alluding to this before:
> > >
> > >        * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
> > >        * error handling here, assume that exclusive mmap_lock on dst and src
> > >        * protects anon from unexpected THP transitions; ...
> > >
> > > It's like the mess around pmd_trans_unstable() :(
> >
> > ... could we fix this by taking the ptlock during the copy operation?
> >
> > > 3. copy_pte_range() skips the required pmd_same() recheck assuming write lock
> > >
> > > It uses pte_offset_map_rw_nolock() with a dummy pmdval and:
> > >
> > >        * We already hold the exclusive mmap_lock, the copy_pte_range() and
> > >        * retract_page_tables() are using vma->anon_vma to be exclusive, so
> > >        * the PTE page is stable, and there is no need to get pmdval and do
> > >        * pmd_same() check.
> > >
> > > (Probably needs updating for VMA write lock...)
> > >
> > > 4. The asserts
> > >
> > > Mentioned above!
> > >
> > > I suspect there's more as well.
> > >
> > > Some of it could be changed, but it'd all be at the cost of behaving very
> > > strangely (no VMA lock but mmap write lock) while manipulating things.
> > >
> > > And I am not really happy with changing page table entries in the parent VMA
> > > (for CoW) without a VMA write lock held.
> > >
> > > I also wonder whether it would achieve all that much really - the issue is with
> > > holding the VMA lock across I/O, anything else that tries to grab a write lock
> > > is going to end up being the new thing that's blocked (mprotect, munmap, mremap,
> > > mlock etc.)
> > >
> > > I think doing something like this would need a very significant redesign and I'm
> > > not convinced it's one that really achieves what we want given all the other
> > > cases that'd get blocked by the VMA lock being held still.
> > >
> > > --
> > > Cheers, Lorenzo

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Lorenzo Stoakes (ARM)]

(Thanks to Suren for his reply also, Replying to this mail to make it
easier for me to quote :)

On Mon, Oct 19, 2026 at 09:53:32PM +0100, Matthew Wilcox wrote:
> On Fri, Oct 02, 2026 at 02:12:35PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Oct 01, 2026 at 10:12:16PM +0100, Matthew Wilcox wrote:
> > > So what I didn't realise is that fork() waits for page faults to finish.
> > > I don't think that's necessary, so we can just stop doing that (whitespace
> > > damaged):
> > > I think this is safe.  I've booted a kernel with this change, and
> >
> > I'm afraid it's not safe :)
> >
> > We already actually did this (by mistake I think?) before. See commit
> > fb49c455323f ("fork: lock VMAs of the parent process when forking") - we shipped
> > that in 6.4.0 -> 6.4.2 and it corrupted memory all over the place.
> >
> > Reports + a consistent reproducer:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=217624
> > https://lore.kernel.org/all/dbdef34c-3a07-5951-e1ae-e9c6e3cdf51b@kernel.org/
> > https://lore.kernel.org/all/b198d649-f4bf-b971-31d0-e8433ec2a34c@applied-asynchrony.com/
>
> I wonder if we still need this.  The commit message says:
>
>     A concurrent page fault on a page newly marked read-only by the page
>     copy might trigger wp_page_copy() and a anon_vma_prepare(vma) on the
>     source vma, defeating the anon_vma_clone() that wasn't done because the
>     parent vma originally didn't have an anon_vma, but we now might end up
>     copying a pte entry for a page that has one.
>
> But we're holding the mmap_lock for write here, and we need to hold the
> mmap_lock for read in order to change a vma->anon_vma (asserted in
> __anon_vma_prepare() with mmap_assert_locked()).

I don't think that changed anything. Actually that paragraph is probably
just a bad example, since previous to a373baed5a9d lock_vma_under_rcu()
did:

	/*
	 * find_mergeable_anon_vma uses adjacent vmas which are not locked.
	 * This check must happen after vma_start_read(); otherwise, a
	 * concurrent mremap() with MREMAP_DONTUNMAP could dissociate the VMA
	 * from its anon_vma.
	 */
	if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma))
		goto inval_end_read;

So the example cited couldn't happen then either.

I think fundamentally the biggest issue is that, for CoW, we update the
_parent_ as well as the child to do wrprotect.

So you can end up with a stale TLB entry, which can result in data loss:

Thread A (Parent)          | Thread B (Parent)       | Thread C (Parent)
---------------------------|-------------------------|----------------------
< Fork >                   |                         | < has TLB entry >
copy_present_ptes()        | < no TLB entry >        |
  wrprotect parent page X  |                         |
  -> 2 mappings, refs      | *ptr = x                |
                           | < Page fault >          |
                           | do_wp_page()            |
                           |   < shared, so CoW >    |
                           |   wp_page_copy()        |
                           |     copy X -> new pg N  |
                           |                         | *ptr = x;
                           |                         | [ OOPS! stale TLB,
                           |                         |   lands in X not N ]
                           |     ptep_clear_flush()  |
                           |       < TLB flush >     |
                           |     parent PTE -> N     |
                           |     [ X now child-only ]|
                           |                         | ASSERT(*ptr == x);
flush_tlb_mm(oldmm)        |                         | // fails! N lacks x
  < far too late >         |                         |


David raised this at:
https://lore.kernel.org/all/7e3f35cc-59b9-bf12-b8b1-4ed78223844a@redhat.com/

So the real hazard here is doing the copying with concurrent faults.

I don't really think there's a sensible way around that, unfortunately.

The above is also likely what the bugzilla observed.

>
> At the time of this commit, lock_vma_under_rcu() contained:
>
>         /* find_mergeable_anon_vma uses adjacent vmas which are not locked */
>         if (!vma->anon_vma && !vma_is_tcp(vma))
>                 goto inval;
>
> so we really did have to wait for read holders to finish the fault.
> But commit a373baed5a9d deferred the check for a NULL anon VMA until we
> actually needed it, so I think commit fb49c455323f's logic was invalidated
> by a373baed5a9d.
>
> Maybe I'm wrong.  This is all a bit out of my normal area.
>
> > Also we have asserts for this lock in copy_page_range() and
> > copy_hugetlb_page_range() which will now fail (and possibly more further down
> > the stack?)
>
> Possibly we have to adjust some assertions to check that either we have
> the VMA locked or we have the MM write locked.  That doesn't seem
> unreasonable.

Well, no, because the VMA locks are designed such that an mmap write lock
doesn't exclude them, so that would be undermining their intent.

The thing is that we allow VMA read locks without an mmap read lock lock, so if
I hold an mmap write lock, I can have parallel reads of any VMA that is
not explicitly VMA write locked.

>
> > > My argument for why it's safe is that a thread which takes a page
> > > fault during fork() might have taken the page fault either before or
> > > after fork().  The faults will definitely happen in the parent process.
> > > They may or may not have happened in the child process, which can't
> > > possibly care whether or not they've happened.
> >
> > The problem isn't the child it's the parent - copy_page_range() _modifies_ the
> > _parent_'s page tables in-place, on assumption that nothing else can access
> > them (for CoW).
>
> I was perhaps unclear.  What I mean is that the child may or may not
> observe them as having happened.

Sure, but the issue is in the parent. CoW really scuppers things here :)

>
> > And actually nowadays things are _even_ worse, you could also have concurrent
> > page faults, MADV_DONTNEED, MADV_FREE, MADV_GUARD_INSTALL/REMOVE, UFFDIO_COPY +
> > UFFDIO_MOVE (yikes) all of which might introduce new issues + would have to be
> > audited too.
>
> Yup.
>
> > So - it turns out a lot of stuff implicitly assumes VMA write lock now:
> >
> > 1. CoW - deferred TLB flush race
> >
> > (The issue above)
> >
> > https://lore.kernel.org/all/20230705171213.2843068-2-surenb@google.com/
> >
> > This happens because __copy_present_ptes() does wrprotect_ptes() on the parent
> > without a TLB flush (that's deferred until the end of the fork).
> >
> > But unfortunately without a VMA write lock something can then replace the page
> > table entry in the meantime and you can end up with data loss as a result.
>
> Is it that fb49c455323f commit message is insufficient and we need to
> prevent simultaneous fork/page fault for other reasons, then?

Yeah, as above, the example given isn't great.

>
> > Same issue exists with hugetlb wp too. And god only knows with uffd wp :)
> >
> > 2. THP - can destroy parent THP mappings
> >
> > copy_pmd_range() does:
> >
> > 	if (pmd_is_huge(*src_pmd)) { copy_huge_pmd(); ... }
> > 	if (pmd_none_or_clear_bad(src_pmd))
> > 		continue;
> >
> > Without the write lock, a concurrent fault can install a huge PMD via
> > do_huge_pmd_anonymous_page() or filemap_map_pmd()/do_set_pmd().
> >
> > The issue is that pmd_bad() fires for leaf PMD (ugh) on at least x86 and arm64,
> > meaning that'll clear the page table entry.
> >
> > I think Hugh left a comment alluding to this before:
> >
> > 	 * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
> > 	 * error handling here, assume that exclusive mmap_lock on dst and src
> > 	 * protects anon from unexpected THP transitions; ...
> >
> > It's like the mess around pmd_trans_unstable() :(
>
> ... could we fix this by taking the ptlock during the copy operation?

Well, that's a long time to hold that lock, and while it might address the
pmd_none_or_clear_bad() thing, the CoW issue remains the same, since really
the issue there is TLB invalidation.

And the only way to address that is to just now allow faults to happen
during fork.

(I also fear there are other subtle bugs that might emerge from all this,
we do like to make _implicit_ assumptions about things).

Actually there's a pleasing thing you came up with at the time:

	Calling fork() from a multi-threaded program is fraught with
	danger.  It's a rare thing to do, and we don't need to optimise for
	it.  It does, of course, need to not crash.  But we can slow it
	down as much as we want to.  Slowing down single-threaded programs
	calling fork is much less acceptable.

https://lore.kernel.org/all/ZKQZnAfLRPVqnxiO@casper.infradead.org/

:))

It does feel a bit like we're bending over backwards for a broken use-case.

And anyway, even if we did somehow find a way to make all this work, I fear
we'd just move the contention somewhere else.

Perhaps the 'genius' hack of mine isn't so bad after all? :P

--
Cheers, Lorenzo

Re: [PATCH v6] mm: retry page faults once under the per-VMA lock

[Lorenzo Stoakes (ARM)]

On Tue, Oct 27, 2026 at 04:26:39PM +0000, Lorenzo Stoakes (ARM) wrote:
>
> Thread A (Parent)          | Thread B (Parent)       | Thread C (Parent)
> ---------------------------|-------------------------|----------------------
> < Fork >                   |                         | < has TLB entry >
> copy_present_ptes()        | < no TLB entry >        |
>   wrprotect parent page X  |                         |
>   -> 2 mappings, refs      | *ptr = x                |
>                            | < Page fault >          |
>                            | do_wp_page()            |
>                            |   < shared, so CoW >    |
>                            |   wp_page_copy()        |
>                            |     copy X -> new pg N  |
>                            |                         | *ptr = x;
>                            |                         | [ OOPS! stale TLB,
>                            |                         |   lands in X not N ]
>                            |     ptep_clear_flush()  |
>                            |       < TLB flush >     |
>                            |     parent PTE -> N     |
>                            |     [ X now child-only ]|
>                            |                         | ASSERT(*ptr == x);
> flush_tlb_mm(oldmm)        |                         | // fails! N lacks x
>   < far too late >         |                         |
>

Sigh, I drew this entirely manually, then had the LLM overlords 'correct' it,
but it also made it less clear :).

Thread C ends up writing to what-becomes-the-child-page X, whereas the parent
ends up with page N after CoW.

So a thread in the parent writes to a page of ordinary memory and then it
disappears under them, and the child ends up with corruption in their page.

--
Cheers, Lorenzo