* [PATCH v6] mm: retry page faults once under the per-VMA lock
@ 2026-09-11 2:56 Hongru Zhang
2026-09-21 17:54 ` Lorenzo Stoakes (ARM)
0 siblings, 1 reply; 5+ messages in thread
From: Hongru Zhang @ 2026-09-11 2:56 UTC (permalink / raw)
To: akpm
Cc: linux-mm, linux-kernel, baohua, surenb, ljs, david, liam, mhocko,
rppt, shakeel.butt, vbabka, willy, zhaonanzhe, linux,
catalin.marinas, will, mark.rutland, linux-arm-kernel,
chenhuacai, kernel, loongarch, maddy, mpe, npiggin, chleroy,
linuxppc-dev, pjw, palmer, aou, alex, linux-riscv, agordeev,
gerald.schaefer, hca, gor, borntraeger, svens, linux-s390,
dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa,
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6] mm: retry page faults once under the per-VMA lock
2026-09-11 2:56 [PATCH v6] mm: retry page faults once under the per-VMA lock Hongru Zhang
@ 2026-09-21 17:54 ` Lorenzo Stoakes (ARM)
2026-09-21 18:57 ` Matthew Wilcox
2026-09-23 9:53 ` Hongru Zhang
0 siblings, 2 replies; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-21 17:54 UTC (permalink / raw)
To: Hongru Zhang
Cc: akpm, linux-mm, linux-kernel, baohua, surenb, david, liam,
mhocko, rppt, shakeel.butt, vbabka, willy, zhaonanzhe, linux,
catalin.marinas, will, mark.rutland, linux-arm-kernel,
chenhuacai, kernel, loongarch, maddy, mpe, npiggin, chleroy,
linuxppc-dev, pjw, palmer, aou, alex, linux-riscv, agordeev,
gerald.schaefer, hca, gor, borntraeger, svens, linux-s390,
dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa,
Hongru Zhang
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/
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6] mm: retry page faults once under the per-VMA lock
2026-09-21 17:54 ` Lorenzo Stoakes (ARM)
@ 2026-09-21 18:57 ` Matthew Wilcox
2026-09-22 8:55 ` Lorenzo Stoakes (ARM)
2026-09-23 9:53 ` Hongru Zhang
1 sibling, 1 reply; 5+ messages in thread
From: Matthew Wilcox @ 2026-09-21 18:57 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: Hongru Zhang, akpm, linux-mm, linux-kernel, baohua, surenb,
david, liam, mhocko, rppt, shakeel.butt, vbabka, zhaonanzhe,
linux, catalin.marinas, will, mark.rutland, linux-arm-kernel,
chenhuacai, kernel, loongarch, maddy, mpe, npiggin, chleroy,
linuxppc-dev, pjw, palmer, aou, alex, linux-riscv, agordeev,
gerald.schaefer, hca, gor, borntraeger, svens, linux-s390,
dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa,
Hongru Zhang
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 ;-)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6] mm: retry page faults once under the per-VMA lock
2026-09-21 18:57 ` Matthew Wilcox
@ 2026-09-22 8:55 ` Lorenzo Stoakes (ARM)
0 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-22 8:55 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Hongru Zhang, akpm, linux-mm, linux-kernel, baohua, surenb,
david, liam, mhocko, rppt, shakeel.butt, vbabka, zhaonanzhe,
linux, catalin.marinas, will, mark.rutland, linux-arm-kernel,
chenhuacai, kernel, loongarch, maddy, mpe, npiggin, chleroy,
linuxppc-dev, pjw, palmer, aou, alex, linux-riscv, agordeev,
gerald.schaefer, hca, gor, borntraeger, svens, linux-s390,
dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa,
Hongru Zhang
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v6] mm: retry page faults once under the per-VMA lock
2026-09-21 17:54 ` Lorenzo Stoakes (ARM)
2026-09-21 18:57 ` Matthew Wilcox
@ 2026-09-23 9:53 ` Hongru Zhang
1 sibling, 0 replies; 5+ messages in thread
From: Hongru Zhang @ 2026-09-23 9:53 UTC (permalink / raw)
To: Lorenzo Stoakes (ARM)
Cc: akpm, linux-mm, linux-kernel, baohua, surenb, david, liam,
mhocko, rppt, shakeel.butt, vbabka, willy, zhaonanzhe, linux,
catalin.marinas, will, mark.rutland, linux-arm-kernel,
chenhuacai, kernel, loongarch, maddy, mpe, npiggin, chleroy,
linuxppc-dev, pjw, palmer, aou, alex, linux-riscv, agordeev,
gerald.schaefer, hca, gor, borntraeger, svens, linux-s390,
dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa,
Hongru Zhang
[-- Attachment #1: Type: text/plain, Size: 8024 bytes --]
On Tue, Sep 22, 2026 at 1:54 AM Lorenzo Stoakes (ARM) <ljs@kernel.org> wrote:
> 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.
Thank you for the detailed review, Lorenzo, and thank you Matthew for
the follow-up. The analysis of the existing VM_FAULT_RETRY semantics is
very helpful.
Let us answer these two questions directly.
The problem we are addressing is `mmap_lock` contention caused by the
per-VMA lock falling back to `mmap_lock` on `VM_FAULT_RETRY`.
The core pattern is both `mmap_lock` waiters blocking progress and a
priority inversion: page fault threads hold the read lock while
blocking on I/O (`filemap_fault`, `do_swap_page`); a writer
(`mmap`/`munmap`/`mprotect` from the linker or runtime) waits for the
write lock; and all subsequent readers, which may still require
`mmap_lock` in the page fault retry path, including the UI main thread,
queue behind the writer. For the priority inversion issue, the main
thread can stall until the entire chain is resolved.
This contention has different consequences depending on context:
- During app cold startup, it slows down launch time.
- During runtime, on a 16.67ms vsync budget, it directly causes dropped
frames — one of the most user-visible performance issues on Android.
- It also degrades throughput for any concurrent page fault workload
under write pressure (as the benchmarks below show).
> 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?
We would not say the microbenchmarks are purely artificial. The filemap
benchmark [1] specifically mimics the scenario Matthew described earlier,
where pages can be reclaimed while a page fault is being retried,
resulting in another round of I/O. Under concurrent `mmap` write
pressure, the VMA lock fallback to `mmap_lock` turns this into severe
`mmap_lock` contention. In fact, waiting on `mmap_lock` for an extended
period during the page fault retry can give reclaim a sufficient window
to reclaim the page again.
The swap benchmark [2] exercises the `do_swap_page` path under the same
conditions. These are not purely arbitrary contention generators, but
simplified models of the real problem. While the effect may be amplified
compared with some real workloads with less contention, they reproduce
the exact fault paths where the problem occurs in a simpler way.
That said, we agree the real-world impact is what matters, and we have
more data to present.
> 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.
The impact of `mmap_lock` contention on cold startup time can vary
significantly across apps, depending on their I/O volume, number of
concurrent threads, and library loading patterns. For example, Vivo
independently tested Douyin (TikTok China) cold launch on SM8975 and
observed a 15.5% improvement (666 → 563 ms, average of 100 runs) [3].
Douyin's heavier concurrent memory activity makes it more sensitive to
this contention.
> 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'.
>
> 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.
This is not a hack for one workload. The priority-inversion pattern
described above is inherent to any multi-threaded process with concurrent
page faults and mmap operations — which covers essentially all modern
Android apps (zygote fork + GC + JIT + IO threads).
> 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.
To supplement the cold startup data we presented earlier, here is frame
drop data that we did not have in previous versions. We traced mmap_lock
contention on the UI main thread across 17 popular Android apps during
typical scroll/swipe interactions, 10 runs each:
Overall:
Vanilla: 722 contention events, 12,315 ms total contention time
Patched: 235 contention events, 4,284 ms total contention time
Change: -67.5% events, -65.2% total time
Per-app breakdown (sorted by time reduction):
App Vanilla(cnt/ms) Patched(cnt/ms) Count Time
-----------------------------------------------------------------------
NetEase Music 50 / 521 ms 0 / 0 ms -100.0% -100.0%
Meituan 13 / 81 ms 0 / 0 ms -100.0% -100.0%
Tencent Video 158 / 2,452 ms 17 / 177 ms -89.2% -92.8%
Toutiao 30 / 289 ms 5 / 24 ms -83.3% -91.6%
Douyin 41 / 572 ms 6 / 95 ms -85.4% -83.4%
Douyin Lite 28 / 565 ms 10 / 118 ms -64.3% -79.1%
Pipixia 18 / 533 ms 6 / 127 ms -66.7% -76.2%
Amap 10 / 46 ms 3 / 15 ms -70.0% -67.7%
JD.com 80 / 3,145 ms 32 / 1,172 ms -60.0% -62.7%
Kuaishou Lite 75 / 978 ms 11 / 366 ms -85.3% -62.6%
WeChat 1 / 17 ms 2 / 6 ms +100.0% -61.5%
Alipay 14 / 274 ms 8 / 108 ms -42.9% -60.5%
Douyu 2 / 16 ms 2 / 8 ms +0.0% -46.4%
Kuaishou 55 / 686 ms 22 / 432 ms -60.0% -37.0%
Baidu Tieba 57 / 1,441 ms 42 / 1,010 ms -26.3% -29.9%
Weibo 52 / 445 ms 36 / 382 ms -30.8% -14.2%
Bilibili 38 / 257 ms 33 / 243 ms -13.2% -5.2%
All 17 apps show reduction in main-thread contention time, with 12
of them seeing >60% reduction. These are not synthetic benchmarks —
they are real apps running on real devices.
I have also attached a Perfetto diagram showing how the main thread
waiting for `mmap_lock` during a page fault retry can lead to missed
frames.
[1] https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef
[2] https://gist.github.com/zhr250/218ffe693f842346b56434483127422c
[3] https://lore.kernel.org/linux-mm/20260915130544.3774446-1-xueyuan.chen@vivo.com/
Thanks,
Hongru
[-- Attachment #2: perfetto.jpg --]
[-- Type: image/jpeg, Size: 460382 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-23 9:54 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-11 2:56 [PATCH v6] mm: retry page faults once under the per-VMA lock Hongru Zhang
2026-09-21 17:54 ` Lorenzo Stoakes (ARM)
2026-09-21 18:57 ` Matthew Wilcox
2026-09-22 8:55 ` Lorenzo Stoakes (ARM)
2026-09-23 9:53 ` Hongru Zhang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®