[RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Hongru Zhang]

From: Hongru Zhang <zhanghongru@xiaomi.com>

Currently, when a page fault returns VM_FAULT_RETRY under the per-VMA
lock due to folio_lock() failing, mainly because the folio is under
I/O, the architecture fault handler unconditionally falls back to
retrying with mmap_lock. This leads to mmap_lock contention.

This patch introduces VM_FAULT_RETRY_HARD to mark the paths that
require mmap_lock. Retries now stay under the per-VMA lock by default,
and only marked paths fall back to the mmap_lock retry path.

Based on the stress model from Kunwu Chan and Wang Lian in v2, we
adapted a benchmark for a 20-core desktop environment (reduced thread
count, adjusted memcg limits, 20-core Intel i7-12700). The benchmark
uses concurrent page faults under memcg pressure (forcing reclaim and
VM_FAULT_RETRY) with parallel munmap to amplify mmap_lock read-write
contention.

Throughput (higher is better):
+---------+------------+------------+-------------+
| Threads |  Vanilla   |  Patched   | Improvement |
+---------+------------+------------+-------------+
|   40    | 1071.11 /s | 1301.11 /s |   +21.5%    |
+---------+------------+------------+-------------+
|   60    | 1043.15 /s | 1472.36 /s |   +41.1%    |
+---------+------------+------------+-------------+
|   80    | 1049.39 /s | 1665.65 /s |   +58.7%    |
+---------+------------+------------+-------------+

mmap_lock contention count (lower is better):
+---------+-----------+---------+-----------+
| Threads |  Vanilla  | Patched | Reduction |
+---------+-----------+---------+-----------+
|   40    | 3,217,904 |  51,201 |  -98.4%   |
+---------+-----------+---------+-----------+
|   60    | 4,419,149 |  55,711 |  -98.7%   |
+---------+-----------+---------+-----------+
|   80    | 5,395,730 |  66,184 |  -98.8%   |
+---------+-----------+---------+-----------+

Benchmark and test scripts:
https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef

Suggested-by: Barry Song <baohua@kernel.org>
Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Hongru Zhang <zhanghongru@xiaomi.com>
---
Changes since v2:
- v2 required each retry path to explicitly opt in to VMA lock retry
  (VM_FAULT_RETRY_VMA). This patch inverts the logic: retries default
  to VMA lock, and only paths requiring mmap_lock opt out
  (VM_FAULT_RETRY_HARD)
- This patch corresponds to v2 patch 1/5; the remaining optimizations
  will be submitted separately
- Rebased on mm-unstable

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

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

 arch/arm/mm/fault.c       | 4 ++++
 arch/arm64/mm/fault.c     | 4 ++++
 arch/loongarch/mm/fault.c | 4 ++++
 arch/powerpc/mm/fault.c   | 4 ++++
 arch/riscv/mm/fault.c     | 4 ++++
 arch/s390/mm/fault.c      | 4 ++++
 arch/x86/mm/fault.c       | 4 ++++
 include/linux/mm_types.h  | 9 +++++----
 mm/huge_memory.c          | 2 +-
 mm/memory.c               | 6 +++---
 10 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
index e62cc4be5adf..2f909fb4f7db 100644
--- a/arch/arm/mm/fault.c
+++ b/arch/arm/mm/fault.c
@@ -391,6 +391,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+retry_vma:
 	vma = lock_vma_under_rcu(mm, addr);
 	if (!vma)
 		goto lock_mmap;
@@ -420,6 +421,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 			goto no_context;
 		return 0;
 	}
+	if (!(fault & VM_FAULT_RETRY_HARD))
+		goto retry_vma;
+
 lock_mmap:
 
 retry:
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 739800835920..695a09486795 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -673,6 +673,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 	if (!(mm_flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+retry_vma:
 	vma = lock_vma_under_rcu(mm, addr);
 	if (!vma)
 		goto lock_mmap;
@@ -719,6 +720,9 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
 			goto no_context;
 		return 0;
 	}
+	if (!(fault & VM_FAULT_RETRY_HARD))
+		goto retry_vma;
+
 lock_mmap:
 
 retry:
diff --git a/arch/loongarch/mm/fault.c b/arch/loongarch/mm/fault.c
index 2c93d33356e5..d7dd55722e52 100644
--- a/arch/loongarch/mm/fault.c
+++ b/arch/loongarch/mm/fault.c
@@ -219,6 +219,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+retry_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -265,6 +266,9 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
 			no_context(regs, write, address);
 		return;
 	}
+	if (!(fault & VM_FAULT_RETRY_HARD))
+		goto retry_vma;
+
 lock_mmap:
 
 retry:
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 806c74e0d5ab..65ec9a8252e7 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -487,6 +487,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+retry_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -517,6 +518,9 @@ 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 (!(fault & VM_FAULT_RETRY_HARD))
+		goto retry_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..6c68414a1224 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -347,6 +347,7 @@ void handle_page_fault(struct pt_regs *regs)
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+retry_vma:
 	vma = lock_vma_under_rcu(mm, addr);
 	if (!vma)
 		goto lock_mmap;
@@ -376,6 +377,9 @@ void handle_page_fault(struct pt_regs *regs)
 			no_context(regs, addr);
 		return;
 	}
+	if (!(fault & VM_FAULT_RETRY_HARD))
+		goto retry_vma;
+
 lock_mmap:
 
 retry:
diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index 028aeb9c48d6..45a6c35044cc 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -294,6 +294,7 @@ static void do_exception(struct pt_regs *regs, int access)
 		flags |= FAULT_FLAG_WRITE;
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
+retry_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -318,6 +319,9 @@ static void do_exception(struct pt_regs *regs, int access)
 			handle_fault_error_nolock(regs, 0);
 		return;
 	}
+	if (!(fault & VM_FAULT_RETRY_HARD))
+		goto retry_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 63de8e8684f2..cb80070271d0 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -1322,6 +1322,7 @@ void do_user_addr_fault(struct pt_regs *regs,
 	if (!(flags & FAULT_FLAG_USER))
 		goto lock_mmap;
 
+retry_vma:
 	vma = lock_vma_under_rcu(mm, address);
 	if (!vma)
 		goto lock_mmap;
@@ -1351,6 +1352,9 @@ void do_user_addr_fault(struct pt_regs *regs,
 						 ARCH_DEFAULT_PKEY);
 		return;
 	}
+	if (!(fault & VM_FAULT_RETRY_HARD))
+		goto retry_vma;
+
 lock_mmap:
 
 retry:
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 5ef78617ce93..58db6f0af6fd 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -1665,10 +1665,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_RETRY_HARD     = (__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 94bd656eeaf8..ad0129d579bc 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1398,7 +1398,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_RETRY_HARD;
 	}
 
 	ptl = pmd_lock(vma->vm_mm, vmf->pmd);
diff --git a/mm/memory.c b/mm/memory.c
index ff338c2abe92..7f2a30b5efca 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3797,7 +3797,7 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
 	if (vma->vm_ops->map_pages || !(vmf->flags & FAULT_FLAG_VMA_LOCK))
 		return 0;
 	vma_end_read(vma);
-	return VM_FAULT_RETRY;
+	return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
 }
 
 /**
@@ -3824,7 +3824,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_RETRY_HARD;
 	}
 	if (__anon_vma_prepare(vma))
 		ret = VM_FAULT_OOM;
@@ -4778,7 +4778,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_RETRY_HARD;
 				goto out;
 			}
 

base-commit: 81652c5a65d4ae28e9b18c16ef917a40025c3653
-- 
2.43.0

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[David Hildenbrand (Arm)]

On 6/26/26 09:50, Hongru Zhang wrote:
> From: Hongru Zhang <zhanghongru@xiaomi.com>
> 
> Currently, when a page fault returns VM_FAULT_RETRY under the per-VMA
> lock due to folio_lock() failing, mainly because the folio is under
> I/O, the architecture fault handler unconditionally falls back to
> retrying with mmap_lock. This leads to mmap_lock contention.
> 
> This patch introduces VM_FAULT_RETRY_HARD to mark the paths that
> require mmap_lock. 

Really sorry, but what a bad name :)

Can we just embed the "no VMA lock" or "MMAP_LOCK" in the name somehow?

VM_FAULT_RETRY_NEED_MMAP_LOCK
VM_FAULT_RETRY_NO_VMA_LOCK


Something like that?


Also, we should document what the new flag means, and how it, for example, is
always accompanied by VM_FAULT_RETRY.

Best to be as clear as possible on the intended semantics.

-- 
Cheers,

David

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Lorenzo Stoakes]

+cc Vlata, the only one from the MEMORY MAPPING - LOCKING people excluded,
and Willy as he's been heavily involved. But see below you need to cc- way
more.

(nitty I know) Maybe subject should be 'mm: default to VMA lock on
VM_FAULT_RETRY' or something just to make it clear it's about the retry
path?

Also I guess you and Barry agreed to you taking this over? :)

You missed a million people from cc- by the way. This time I really cannot
be bothered to add them all as I usually do, but it is MANDATORY to cc
maintainers and really you should be cc'ing reviewers too every time.

Again I feel I need to write a bot that just autoreplies about this :)

$ scripts/get_maintainer.pl --nogit --nogit-fallback your-patch.patch
<... ~36 people in total ...>
< lists >
< etc. >

Please fix this on respin.

On Fri, Jun 26, 2026 at 03:50:19PM +0800, Hongru Zhang wrote:
> From: Hongru Zhang <zhanghongru@xiaomi.com>
>
> Currently, when a page fault returns VM_FAULT_RETRY under the per-VMA
> lock due to folio_lock() failing, mainly because the folio is under
> I/O, the architecture fault handler unconditionally falls back to
> retrying with mmap_lock. This leads to mmap_lock contention.
>
> This patch introduces VM_FAULT_RETRY_HARD to mark the paths that

I'm going to have to bikeshed the name :)

While I like the idea of 'retry hard' like the movie 'die hard' :P I think,
since this is explicitly about the mmap lock, we should reference that?

So maybe VM_FAULT_RETRY_MMAP?

or VM_FAULT_RETRY_HARDER_WITH_A_VENGEANCE? ;)

> require mmap_lock. Retries now stay under the per-VMA lock by default,
> and only marked paths fall back to the mmap_lock retry path.

I think the commit message should be much clearer about the logic here.

You are making it such that VM_FAULT_RETRY does its retry pass using a VMA
lock, but it retains the same 'retry and set FAULT_FLAG_TRIED' logic as
before.

HOWEVER.

On the VMA lock path the retry flag is only set if VM_FAULT_MAJOR. So now
you're allowing _unbounded_ VMA lock attempts (which is concerning and I
don't think intended by the lock code).

None of this is clear right now, you need to documment it.

>
> Based on the stress model from Kunwu Chan and Wang Lian in v2, we
> adapted a benchmark for a 20-core desktop environment (reduced thread
> count, adjusted memcg limits, 20-core Intel i7-12700). The benchmark
> uses concurrent page faults under memcg pressure (forcing reclaim and
> VM_FAULT_RETRY) with parallel munmap to amplify mmap_lock read-write
> contention.
>
> Throughput (higher is better):
> +---------+------------+------------+-------------+
> | Threads |  Vanilla   |  Patched   | Improvement |
> +---------+------------+------------+-------------+
> |   40    | 1071.11 /s | 1301.11 /s |   +21.5%    |
> +---------+------------+------------+-------------+
> |   60    | 1043.15 /s | 1472.36 /s |   +41.1%    |
> +---------+------------+------------+-------------+
> |   80    | 1049.39 /s | 1665.65 /s |   +58.7%    |
> +---------+------------+------------+-------------+
>
> mmap_lock contention count (lower is better):
> +---------+-----------+---------+-----------+
> | Threads |  Vanilla  | Patched | Reduction |
> +---------+-----------+---------+-----------+
> |   40    | 3,217,904 |  51,201 |  -98.4%   |
> +---------+-----------+---------+-----------+
> |   60    | 4,419,149 |  55,711 |  -98.7%   |
> +---------+-----------+---------+-----------+
> |   80    | 5,395,730 |  66,184 |  -98.8%   |
> +---------+-----------+---------+-----------+

Yeah I worry a bit about the unbounded nature of things here, the fact
we're seeing such a difference could also be offset by extreme busy waiting
with the unbounded VMA lock retries.

Let's experiment based on limiting this surely?

>
> Benchmark and test scripts:
> https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef
>
> Suggested-by: Barry Song <baohua@kernel.org>
> Suggested-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Hongru Zhang <zhanghongru@xiaomi.com>
> ---
> Changes since v2:
> - v2 required each retry path to explicitly opt in to VMA lock retry
>   (VM_FAULT_RETRY_VMA). This patch inverts the logic: retries default
>   to VMA lock, and only paths requiring mmap_lock opt out
>   (VM_FAULT_RETRY_HARD)
> - This patch corresponds to v2 patch 1/5; the remaining optimizations
>   will be submitted separately
> - Rebased on mm-unstable
>
> Link to v2:
> https://lore.kernel.org/all/20260430040427.4672-1-baohua@kernel.org/
>
> Link to v1:
> https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@gmail.com/

Hmm it's a bit odd for you to send an independent series and link to
somebody else's series as v1/v2 but anyway as long as Barry and you agree
with that approach that's fine!

>
>  arch/arm/mm/fault.c       | 4 ++++
>  arch/arm64/mm/fault.c     | 4 ++++
>  arch/loongarch/mm/fault.c | 4 ++++
>  arch/powerpc/mm/fault.c   | 4 ++++
>  arch/riscv/mm/fault.c     | 4 ++++
>  arch/s390/mm/fault.c      | 4 ++++
>  arch/x86/mm/fault.c       | 4 ++++
>  include/linux/mm_types.h  | 9 +++++----
>  mm/huge_memory.c          | 2 +-
>  mm/memory.c               | 6 +++---
>  10 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index e62cc4be5adf..2f909fb4f7db 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -391,6 +391,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  	if (!(flags & FAULT_FLAG_USER))
>  		goto lock_mmap;
>
> +retry_vma:
>  	vma = lock_vma_under_rcu(mm, addr);
>  	if (!vma)
>  		goto lock_mmap;
> @@ -420,6 +421,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
>  			goto no_context;
>  		return 0;
>  	}

OK so this is really important - and it seems to be the case regardless of
arch.

When VM_FAULT_RETRY is encountered under the VMA lock, we do:

	if (fault & VM_FAULT_MAJOR)
		flags |= FAULT_FLAG_TRIED;

Which then allows _under the mmap lock_ 2 more retries.

You're now (silently!) going to:

	if (!(fault & VM_FAULT_MAJOR))
		/* unbounded infinite VMA lock retries */

This seems very unwise (TM).

We should bound this somehow. A maximum number of retries? Only do the
minor fault thing once?

Could you benchmark how it looks at different retry counts? We could also
consider other strategies, but unbounded attempts here seems not good.

Another thing - is there any way to dedupe this code...? We're open coding
a lot of crap now in each of the arches and it's not good.

A series like this is the place to fix stuff like that, not keep adding to
the technical debt.

Hongru - could you see if that's possible as preceeding patches please?

> +	if (!(fault & VM_FAULT_RETRY_HARD))
> +		goto retry_vma;

Maybe it's worth being explicit that this is the VM_FAULT_RETRY path? Otherwise
it's not so clear.

You happen to only fall through here if VM_FAULT_RETRY is set.

So e.g.:

/**
 * fault_can_retry_vma_lock() - Are we able to retry a page fault holding
 * only the VMA lock?
 * @fault: The fault to check.
 *
 * We default to performing the VM_FAULT_RETRY/FAULT_FLAG_TRIED dance under
 * VMA lock. This allows us to avoid mmap contention on the likely case
 * that we encountered an ephemeral inability to VMA lock (e.g. the VMA was
 * write locked at the time).
 *
 * Returns: true if we can retry under the VMA lock, false otherwise.
 */
static bool fault_can_retry_vma_lock(vm_fault_t fault)
{
	VM_WARN_ON_ONCE(!(fault & VM_FAULT_RETRY));

	return fault & VM_FAULT_RETRY_HARD;
}

And put that somewhere all the arch fault logic can access.

Then:

	if (fault_can_retry_vma_lock(fault))
		goto retry_vma;

Is self-documenting, and the assert assures that no mistake was made with
it.

Same comments for all the arches, obviously.

> +
>  lock_mmap:
>
>  retry:
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 739800835920..695a09486795 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -673,6 +673,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
>  	if (!(mm_flags & FAULT_FLAG_USER))
>  		goto lock_mmap;
>
> +retry_vma:
>  	vma = lock_vma_under_rcu(mm, addr);
>  	if (!vma)
>  		goto lock_mmap;
> @@ -719,6 +720,9 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
>  			goto no_context;
>  		return 0;
>  	}
> +	if (!(fault & VM_FAULT_RETRY_HARD))
> +		goto retry_vma;
> +
>  lock_mmap:
>
>  retry:
> diff --git a/arch/loongarch/mm/fault.c b/arch/loongarch/mm/fault.c
> index 2c93d33356e5..d7dd55722e52 100644
> --- a/arch/loongarch/mm/fault.c
> +++ b/arch/loongarch/mm/fault.c
> @@ -219,6 +219,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
>  	if (!(flags & FAULT_FLAG_USER))
>  		goto lock_mmap;
>
> +retry_vma:
>  	vma = lock_vma_under_rcu(mm, address);
>  	if (!vma)
>  		goto lock_mmap;
> @@ -265,6 +266,9 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
>  			no_context(regs, write, address);
>  		return;
>  	}
> +	if (!(fault & VM_FAULT_RETRY_HARD))
> +		goto retry_vma;
> +
>  lock_mmap:
>
>  retry:
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 806c74e0d5ab..65ec9a8252e7 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -487,6 +487,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
>  	if (!(flags & FAULT_FLAG_USER))
>  		goto lock_mmap;
>
> +retry_vma:
>  	vma = lock_vma_under_rcu(mm, address);
>  	if (!vma)
>  		goto lock_mmap;
> @@ -517,6 +518,9 @@ 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 (!(fault & VM_FAULT_RETRY_HARD))
> +		goto retry_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..6c68414a1224 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -347,6 +347,7 @@ void handle_page_fault(struct pt_regs *regs)
>  	if (!(flags & FAULT_FLAG_USER))
>  		goto lock_mmap;
>
> +retry_vma:
>  	vma = lock_vma_under_rcu(mm, addr);
>  	if (!vma)
>  		goto lock_mmap;
> @@ -376,6 +377,9 @@ void handle_page_fault(struct pt_regs *regs)
>  			no_context(regs, addr);
>  		return;
>  	}
> +	if (!(fault & VM_FAULT_RETRY_HARD))
> +		goto retry_vma;
> +
>  lock_mmap:
>
>  retry:
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index 028aeb9c48d6..45a6c35044cc 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -294,6 +294,7 @@ static void do_exception(struct pt_regs *regs, int access)
>  		flags |= FAULT_FLAG_WRITE;
>  	if (!(flags & FAULT_FLAG_USER))
>  		goto lock_mmap;
> +retry_vma:
>  	vma = lock_vma_under_rcu(mm, address);
>  	if (!vma)
>  		goto lock_mmap;
> @@ -318,6 +319,9 @@ static void do_exception(struct pt_regs *regs, int access)
>  			handle_fault_error_nolock(regs, 0);
>  		return;
>  	}
> +	if (!(fault & VM_FAULT_RETRY_HARD))
> +		goto retry_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 63de8e8684f2..cb80070271d0 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -1322,6 +1322,7 @@ void do_user_addr_fault(struct pt_regs *regs,
>  	if (!(flags & FAULT_FLAG_USER))
>  		goto lock_mmap;
>
> +retry_vma:
>  	vma = lock_vma_under_rcu(mm, address);
>  	if (!vma)
>  		goto lock_mmap;
> @@ -1351,6 +1352,9 @@ void do_user_addr_fault(struct pt_regs *regs,
>  						 ARCH_DEFAULT_PKEY);
>  		return;
>  	}
> +	if (!(fault & VM_FAULT_RETRY_HARD))
> +		goto retry_vma;
> +
>  lock_mmap:
>
>  retry:
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ef78617ce93..58db6f0af6fd 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h

Please also update the comment block to add the new flag.

> @@ -1665,10 +1665,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_RETRY_HARD     = (__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,

Hmm I suppose there's no harm in renumbering here to keep the retries
together :)

I wonder if we shouldn't invert this VM_FAULT_RETRY | VM_FAULT_RETRY_HARD
thing though and instead have:

#define VM_FAULT_RETRY_MASK (VM_FAULT_RETRY | VM_FAULT_RETRY_MMAP)

And convert code that checks VM_FAULT_RETRY to check VM_FAULT_RETRY_MASK
instead, then there'd be no need for an awkward check/or confusion around
the need for VM_FAULT_RETRY to always be set in both cases.

You're already basically touching all the files that reference
VM_FAULT_RETRY so it's not actually much more churn.

>  	VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
>  };
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 94bd656eeaf8..ad0129d579bc 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1398,7 +1398,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_RETRY_HARD;

Why are we specifically wanting this for PMD device private entries?

You should add a comment, and the commit message should say why.

Actually, maybe in this common case of vmf->flags & FAULT_FLAG_VMA_LOCK ->
harder you can make a helper?

Like:

/** blahhhh some kdoc comment */
static vm_fault_t retry_under_mmap_if_vma_locked(struct vm_fault *vmf)
{
	if (!(vmf->flags & FAULT_FLAG_VMA_LOCK))
		return 0;

	vma_end_read(vma);
	return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
}

And:

	vm_fault_t ret;

	ret = retry_under_mmap_if_vma_locked(vmf);
	if (ret)
		return ret;

Or something like this.

>  	}
>
>  	ptl = pmd_lock(vma->vm_mm, vmf->pmd);
> diff --git a/mm/memory.c b/mm/memory.c
> index ff338c2abe92..7f2a30b5efca 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3797,7 +3797,7 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
>  	if (vma->vm_ops->map_pages || !(vmf->flags & FAULT_FLAG_VMA_LOCK))
>  		return 0;
>  	vma_end_read(vma);
> -	return VM_FAULT_RETRY;
> +	return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;

You could use my helper like above.

>  }
>
>  /**
> @@ -3824,7 +3824,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_RETRY_HARD;

This really makes me think VM_FAULT_RETRY_MMAP or something explicit would
be better, otherwise you have to add a comment explaining you're doing this
because you need the mmap read lock here since the VMA lock + speculative
mmap read lock acquisition didn't work out.

>  	}
>  	if (__anon_vma_prepare(vma))
>  		ret = VM_FAULT_OOM;
> @@ -4778,7 +4778,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_RETRY_HARD;

Again, helper above self-documents.

>  				goto out;
>  			}
>
>
> base-commit: 81652c5a65d4ae28e9b18c16ef917a40025c3653
> --
> 2.43.0
>

Have you audited every single VM_FAULT_RETRY case? Because getting this
wrong could be a serious problem and break things. You should include a
list of call sites in your patch and explain why it's fine in each case.

One case I noticed is uffd:

	ret = VM_FAULT_RETRY;
	...
	if (unlikely(READ_ONCE(ctx->released))) {
		/*
		 * If a concurrent release is detected, do not return
		 * VM_FAULT_SIGBUS or VM_FAULT_NOPAGE, but instead always
		 * return VM_FAULT_RETRY with lock released proactively.
.	...
		 * If we were to return VM_FAULT_NOPAGE, it would work for
		 * the fault path, in which the lock will be released
		 * later.  However for GUP, faultin_page() does nothing
		 * special on NOPAGE, so GUP would spin retrying without
		 * releasing the mmap read lock, causing possible livelock.
		 *
		 * Here only VM_FAULT_RETRY would make sure the mmap lock
		 * be released immediately, so that the thread concurrently
		 * releasing the userfault would always make progress.
		 */
		 release_fault_lock(vmf);
		 goto out;
	}

The comment needs updating (Suren?) to reflect the VMA lock, basically with
VMA lock held this is meaningless.

We're actually safe here in general as if the VMA lock is contended on a
competing write lock we drop to mmap lock.

A racing uffd release thread does an unconditional vma_start_write()
_after_ setting ctx->released:

static int userfaultfd_release(struct inode *inode, struct file *file)
{
	...
	WRITE_ONCE(ctx->released, true);
	userfaultfd_release_all(mm, ctx);
	...
}

userfaultfd_release_all()
<mmap write lock>
-> userfaultfd_clear_vma()
-> vma_modify_flags_uffd()
-> vma_modify()
-> vma_merge_existing_range()   -> vma_start_write() LIVELOCK
OR split_vma() -> __split_vma() -> vma_start_write() LIVELOCK

So yeah, not sure on this one, maybe we're fine just to use VMA lock still?

Suren - any thoughts?

You definitely need to explain your reasoning in the commit message at
least.

Also commit 29a22b9e08d7 ("mm: handle userfaults under VMA lock") did fail
to update this comment to reflect VMA locks - we should fix that too -
Suren? :)


Also another thought - should we allow FAULT_FLAG_RETRY_NOWAIT to retry
with the VMA lock? As this could also be some unbounded spinning? But then
again, if we bound the spinning that might be ok.

Thanks, Lorenzo

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Hongru Zhang]

Hi David, Lorenzo,

Thanks a lot for the detailed review.

Yes, Barry and I discussed this offline, and I am taking over the series from
this version, with his agreement.

I agree that VM_FAULT_RETRY_HARD is a poor name. I will rename it to
something that makes the mmap_lock requirement explicit, and document the
semantics more clearly, including how it relates to VM_FAULT_RETRY.

Lorenzo, I agree that the unbounded VMA-lock retry needs to be addressed. I
will look into ways to bound or otherwise avoid unbounded retries under the
VMA lock, and benchmark the candidate approaches.

I will also look into deduplicating the arch fault-handler changes by adding
common helpers, so the arch code does not have to open-code this logic.

For the retry sites, I will audit all sites that return VM_FAULT_RETRY and
document the reasoning in the next version. In particular, I will look closely
at the userfaultfd released path, do_huge_pmd_device_private(), and the
FAULT_FLAG_RETRY_NOWAIT case, and update the stale comments where needed.

I will also fix the subject and make sure the next respin includes the full
maintainer/reviewer list reported by get_maintainer.pl.

Thanks,
Hongru

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Suren Baghdasaryan]

On Tue, Jul 7, 2026 at 9:52 AM Lorenzo Stoakes <ljs@kernel.org> wrote:
>
> +cc Vlata, the only one from the MEMORY MAPPING - LOCKING people excluded,
> and Willy as he's been heavily involved. But see below you need to cc- way
> more.
>
> (nitty I know) Maybe subject should be 'mm: default to VMA lock on
> VM_FAULT_RETRY' or something just to make it clear it's about the retry
> path?
>
> Also I guess you and Barry agreed to you taking this over? :)
>
> You missed a million people from cc- by the way. This time I really cannot
> be bothered to add them all as I usually do, but it is MANDATORY to cc
> maintainers and really you should be cc'ing reviewers too every time.
>
> Again I feel I need to write a bot that just autoreplies about this :)
>
> $ scripts/get_maintainer.pl --nogit --nogit-fallback your-patch.patch
> <... ~36 people in total ...>
> < lists >
> < etc. >
>
> Please fix this on respin.
>
> On Fri, Jun 26, 2026 at 03:50:19PM +0800, Hongru Zhang wrote:
> > From: Hongru Zhang <zhanghongru@xiaomi.com>
> >
> > Currently, when a page fault returns VM_FAULT_RETRY under the per-VMA
> > lock due to folio_lock() failing, mainly because the folio is under
> > I/O, the architecture fault handler unconditionally falls back to
> > retrying with mmap_lock. This leads to mmap_lock contention.
> >
> > This patch introduces VM_FAULT_RETRY_HARD to mark the paths that
>
> I'm going to have to bikeshed the name :)
>
> While I like the idea of 'retry hard' like the movie 'die hard' :P I think,
> since this is explicitly about the mmap lock, we should reference that?
>
> So maybe VM_FAULT_RETRY_MMAP?
>
> or VM_FAULT_RETRY_HARDER_WITH_A_VENGEANCE? ;)
>
> > require mmap_lock. Retries now stay under the per-VMA lock by default,
> > and only marked paths fall back to the mmap_lock retry path.
>
> I think the commit message should be much clearer about the logic here.
>
> You are making it such that VM_FAULT_RETRY does its retry pass using a VMA
> lock, but it retains the same 'retry and set FAULT_FLAG_TRIED' logic as
> before.
>
> HOWEVER.
>
> On the VMA lock path the retry flag is only set if VM_FAULT_MAJOR. So now
> you're allowing _unbounded_ VMA lock attempts (which is concerning and I
> don't think intended by the lock code).
>
> None of this is clear right now, you need to documment it.
>
> >
> > Based on the stress model from Kunwu Chan and Wang Lian in v2, we
> > adapted a benchmark for a 20-core desktop environment (reduced thread
> > count, adjusted memcg limits, 20-core Intel i7-12700). The benchmark
> > uses concurrent page faults under memcg pressure (forcing reclaim and
> > VM_FAULT_RETRY) with parallel munmap to amplify mmap_lock read-write
> > contention.
> >
> > Throughput (higher is better):
> > +---------+------------+------------+-------------+
> > | Threads |  Vanilla   |  Patched   | Improvement |
> > +---------+------------+------------+-------------+
> > |   40    | 1071.11 /s | 1301.11 /s |   +21.5%    |
> > +---------+------------+------------+-------------+
> > |   60    | 1043.15 /s | 1472.36 /s |   +41.1%    |
> > +---------+------------+------------+-------------+
> > |   80    | 1049.39 /s | 1665.65 /s |   +58.7%    |
> > +---------+------------+------------+-------------+
> >
> > mmap_lock contention count (lower is better):
> > +---------+-----------+---------+-----------+
> > | Threads |  Vanilla  | Patched | Reduction |
> > +---------+-----------+---------+-----------+
> > |   40    | 3,217,904 |  51,201 |  -98.4%   |
> > +---------+-----------+---------+-----------+
> > |   60    | 4,419,149 |  55,711 |  -98.7%   |
> > +---------+-----------+---------+-----------+
> > |   80    | 5,395,730 |  66,184 |  -98.8%   |
> > +---------+-----------+---------+-----------+
>
> Yeah I worry a bit about the unbounded nature of things here, the fact
> we're seeing such a difference could also be offset by extreme busy waiting
> with the unbounded VMA lock retries.
>
> Let's experiment based on limiting this surely?
>
> >
> > Benchmark and test scripts:
> > https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef
> >
> > Suggested-by: Barry Song <baohua@kernel.org>
> > Suggested-by: Suren Baghdasaryan <surenb@google.com>
> > Signed-off-by: Hongru Zhang <zhanghongru@xiaomi.com>
> > ---
> > Changes since v2:
> > - v2 required each retry path to explicitly opt in to VMA lock retry
> >   (VM_FAULT_RETRY_VMA). This patch inverts the logic: retries default
> >   to VMA lock, and only paths requiring mmap_lock opt out
> >   (VM_FAULT_RETRY_HARD)
> > - This patch corresponds to v2 patch 1/5; the remaining optimizations
> >   will be submitted separately
> > - Rebased on mm-unstable
> >
> > Link to v2:
> > https://lore.kernel.org/all/20260430040427.4672-1-baohua@kernel.org/
> >
> > Link to v1:
> > https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@gmail.com/
>
> Hmm it's a bit odd for you to send an independent series and link to
> somebody else's series as v1/v2 but anyway as long as Barry and you agree
> with that approach that's fine!
>
> >
> >  arch/arm/mm/fault.c       | 4 ++++
> >  arch/arm64/mm/fault.c     | 4 ++++
> >  arch/loongarch/mm/fault.c | 4 ++++
> >  arch/powerpc/mm/fault.c   | 4 ++++
> >  arch/riscv/mm/fault.c     | 4 ++++
> >  arch/s390/mm/fault.c      | 4 ++++
> >  arch/x86/mm/fault.c       | 4 ++++
> >  include/linux/mm_types.h  | 9 +++++----
> >  mm/huge_memory.c          | 2 +-
> >  mm/memory.c               | 6 +++---
> >  10 files changed, 37 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> > index e62cc4be5adf..2f909fb4f7db 100644
> > --- a/arch/arm/mm/fault.c
> > +++ b/arch/arm/mm/fault.c
> > @@ -391,6 +391,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, addr);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -420,6 +421,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> >                       goto no_context;
> >               return 0;
> >       }
>
> OK so this is really important - and it seems to be the case regardless of
> arch.
>
> When VM_FAULT_RETRY is encountered under the VMA lock, we do:
>
>         if (fault & VM_FAULT_MAJOR)
>                 flags |= FAULT_FLAG_TRIED;
>
> Which then allows _under the mmap lock_ 2 more retries.
>
> You're now (silently!) going to:
>
>         if (!(fault & VM_FAULT_MAJOR))
>                 /* unbounded infinite VMA lock retries */
>
> This seems very unwise (TM).
>
> We should bound this somehow. A maximum number of retries? Only do the
> minor fault thing once?
>
> Could you benchmark how it looks at different retry counts? We could also
> consider other strategies, but unbounded attempts here seems not good.
>
> Another thing - is there any way to dedupe this code...? We're open coding
> a lot of crap now in each of the arches and it's not good.
>
> A series like this is the place to fix stuff like that, not keep adding to
> the technical debt.
>
> Hongru - could you see if that's possible as preceeding patches please?
>
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
>
> Maybe it's worth being explicit that this is the VM_FAULT_RETRY path? Otherwise
> it's not so clear.
>
> You happen to only fall through here if VM_FAULT_RETRY is set.
>
> So e.g.:
>
> /**
>  * fault_can_retry_vma_lock() - Are we able to retry a page fault holding
>  * only the VMA lock?
>  * @fault: The fault to check.
>  *
>  * We default to performing the VM_FAULT_RETRY/FAULT_FLAG_TRIED dance under
>  * VMA lock. This allows us to avoid mmap contention on the likely case
>  * that we encountered an ephemeral inability to VMA lock (e.g. the VMA was
>  * write locked at the time).
>  *
>  * Returns: true if we can retry under the VMA lock, false otherwise.
>  */
> static bool fault_can_retry_vma_lock(vm_fault_t fault)
> {
>         VM_WARN_ON_ONCE(!(fault & VM_FAULT_RETRY));
>
>         return fault & VM_FAULT_RETRY_HARD;
> }
>
> And put that somewhere all the arch fault logic can access.
>
> Then:
>
>         if (fault_can_retry_vma_lock(fault))
>                 goto retry_vma;
>
> Is self-documenting, and the assert assures that no mistake was made with
> it.
>
> Same comments for all the arches, obviously.
>
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > index 739800835920..695a09486795 100644
> > --- a/arch/arm64/mm/fault.c
> > +++ b/arch/arm64/mm/fault.c
> > @@ -673,6 +673,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> >       if (!(mm_flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, addr);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -719,6 +720,9 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> >                       goto no_context;
> >               return 0;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/loongarch/mm/fault.c b/arch/loongarch/mm/fault.c
> > index 2c93d33356e5..d7dd55722e52 100644
> > --- a/arch/loongarch/mm/fault.c
> > +++ b/arch/loongarch/mm/fault.c
> > @@ -219,6 +219,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -265,6 +266,9 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> >                       no_context(regs, write, address);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > index 806c74e0d5ab..65ec9a8252e7 100644
> > --- a/arch/powerpc/mm/fault.c
> > +++ b/arch/powerpc/mm/fault.c
> > @@ -487,6 +487,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -517,6 +518,9 @@ 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 (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_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..6c68414a1224 100644
> > --- a/arch/riscv/mm/fault.c
> > +++ b/arch/riscv/mm/fault.c
> > @@ -347,6 +347,7 @@ void handle_page_fault(struct pt_regs *regs)
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, addr);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -376,6 +377,9 @@ void handle_page_fault(struct pt_regs *regs)
> >                       no_context(regs, addr);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> > index 028aeb9c48d6..45a6c35044cc 100644
> > --- a/arch/s390/mm/fault.c
> > +++ b/arch/s390/mm/fault.c
> > @@ -294,6 +294,7 @@ static void do_exception(struct pt_regs *regs, int access)
> >               flags |= FAULT_FLAG_WRITE;
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -318,6 +319,9 @@ static void do_exception(struct pt_regs *regs, int access)
> >                       handle_fault_error_nolock(regs, 0);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_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 63de8e8684f2..cb80070271d0 100644
> > --- a/arch/x86/mm/fault.c
> > +++ b/arch/x86/mm/fault.c
> > @@ -1322,6 +1322,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> >       if (!(flags & FAULT_FLAG_USER))
> >               goto lock_mmap;
> >
> > +retry_vma:
> >       vma = lock_vma_under_rcu(mm, address);
> >       if (!vma)
> >               goto lock_mmap;
> > @@ -1351,6 +1352,9 @@ void do_user_addr_fault(struct pt_regs *regs,
> >                                                ARCH_DEFAULT_PKEY);
> >               return;
> >       }
> > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > +             goto retry_vma;
> > +
> >  lock_mmap:
> >
> >  retry:
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 5ef78617ce93..58db6f0af6fd 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
>
> Please also update the comment block to add the new flag.
>
> > @@ -1665,10 +1665,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_RETRY_HARD     = (__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,
>
> Hmm I suppose there's no harm in renumbering here to keep the retries
> together :)
>
> I wonder if we shouldn't invert this VM_FAULT_RETRY | VM_FAULT_RETRY_HARD
> thing though and instead have:
>
> #define VM_FAULT_RETRY_MASK (VM_FAULT_RETRY | VM_FAULT_RETRY_MMAP)
>
> And convert code that checks VM_FAULT_RETRY to check VM_FAULT_RETRY_MASK
> instead, then there'd be no need for an awkward check/or confusion around
> the need for VM_FAULT_RETRY to always be set in both cases.
>
> You're already basically touching all the files that reference
> VM_FAULT_RETRY so it's not actually much more churn.
>
> >       VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
> >  };
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 94bd656eeaf8..ad0129d579bc 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -1398,7 +1398,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_RETRY_HARD;
>
> Why are we specifically wanting this for PMD device private entries?
>
> You should add a comment, and the commit message should say why.
>
> Actually, maybe in this common case of vmf->flags & FAULT_FLAG_VMA_LOCK ->
> harder you can make a helper?
>
> Like:
>
> /** blahhhh some kdoc comment */
> static vm_fault_t retry_under_mmap_if_vma_locked(struct vm_fault *vmf)
> {
>         if (!(vmf->flags & FAULT_FLAG_VMA_LOCK))
>                 return 0;
>
>         vma_end_read(vma);
>         return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
> }
>
> And:
>
>         vm_fault_t ret;
>
>         ret = retry_under_mmap_if_vma_locked(vmf);
>         if (ret)
>                 return ret;
>
> Or something like this.
>
> >       }
> >
> >       ptl = pmd_lock(vma->vm_mm, vmf->pmd);
> > diff --git a/mm/memory.c b/mm/memory.c
> > index ff338c2abe92..7f2a30b5efca 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -3797,7 +3797,7 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
> >       if (vma->vm_ops->map_pages || !(vmf->flags & FAULT_FLAG_VMA_LOCK))
> >               return 0;
> >       vma_end_read(vma);
> > -     return VM_FAULT_RETRY;
> > +     return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
>
> You could use my helper like above.
>
> >  }
> >
> >  /**
> > @@ -3824,7 +3824,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_RETRY_HARD;
>
> This really makes me think VM_FAULT_RETRY_MMAP or something explicit would
> be better, otherwise you have to add a comment explaining you're doing this
> because you need the mmap read lock here since the VMA lock + speculative
> mmap read lock acquisition didn't work out.
>
> >       }
> >       if (__anon_vma_prepare(vma))
> >               ret = VM_FAULT_OOM;
> > @@ -4778,7 +4778,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_RETRY_HARD;
>
> Again, helper above self-documents.
>
> >                               goto out;
> >                       }
> >
> >
> > base-commit: 81652c5a65d4ae28e9b18c16ef917a40025c3653
> > --
> > 2.43.0
> >
>
> Have you audited every single VM_FAULT_RETRY case? Because getting this
> wrong could be a serious problem and break things. You should include a
> list of call sites in your patch and explain why it's fine in each case.

That's my main worry with this patch as well. I think a better
approach would keep VM_FAULT_RETRY as is (falling back to mmap_lock),
introduce VM_FAULT_RETRY_VMA (or whatever is best to indicate a retry
with VMA lock) and then convert individual VM_FAULT_RETRY cases to
VM_FAULT_RETRY_VMA with proper reasoning and explanation why it is
safe. Less churn, gradual conversion, and happy reviewers.

>
> One case I noticed is uffd:
>
>         ret = VM_FAULT_RETRY;
>         ...
>         if (unlikely(READ_ONCE(ctx->released))) {
>                 /*
>                  * If a concurrent release is detected, do not return
>                  * VM_FAULT_SIGBUS or VM_FAULT_NOPAGE, but instead always
>                  * return VM_FAULT_RETRY with lock released proactively.
> .       ...
>                  * If we were to return VM_FAULT_NOPAGE, it would work for
>                  * the fault path, in which the lock will be released
>                  * later.  However for GUP, faultin_page() does nothing
>                  * special on NOPAGE, so GUP would spin retrying without
>                  * releasing the mmap read lock, causing possible livelock.
>                  *
>                  * Here only VM_FAULT_RETRY would make sure the mmap lock
>                  * be released immediately, so that the thread concurrently
>                  * releasing the userfault would always make progress.
>                  */
>                  release_fault_lock(vmf);
>                  goto out;
>         }
>
> The comment needs updating (Suren?) to reflect the VMA lock, basically with
> VMA lock held this is meaningless.

Right, I missed it when enabling per-VMA locks for userfaultfd faults
in [1]. The comment is true if we already fell back to mmap_lock but
not if we are handling the fault under VMA lock. I should amend it.

[1] https://lore.kernel.org/all/20230630211957.1341547-1-surenb@google.com/

>
> We're actually safe here in general as if the VMA lock is contended on a
> competing write lock we drop to mmap lock.
>
> A racing uffd release thread does an unconditional vma_start_write()
> _after_ setting ctx->released:
>
> static int userfaultfd_release(struct inode *inode, struct file *file)
> {
>         ...
>         WRITE_ONCE(ctx->released, true);
>         userfaultfd_release_all(mm, ctx);
>         ...
> }
>
> userfaultfd_release_all()
> <mmap write lock>
> -> userfaultfd_clear_vma()
> -> vma_modify_flags_uffd()
> -> vma_modify()
> -> vma_merge_existing_range()   -> vma_start_write() LIVELOCK
> OR split_vma() -> __split_vma() -> vma_start_write() LIVELOCK
>
> So yeah, not sure on this one, maybe we're fine just to use VMA lock still?
>
> Suren - any thoughts?

If we retry without falling back to mmap_lock then
userfaultfd_release() can end up write-locking mmap_lock but losing
the race for VMA lock to the retrying pagefault handler. This means it
would block while holding mmap_lock for write - not good.
With pagefault handler falling back to mmap_lock, either
userfaultfd_release() wins the race for mmap_lock or pagefault
handler, so I think we are safe.

>
> You definitely need to explain your reasoning in the commit message at
> least.
>
> Also commit 29a22b9e08d7 ("mm: handle userfaults under VMA lock") did fail
> to update this comment to reflect VMA locks - we should fix that too -
> Suren? :)

Yes, I will correct my mistake and send a small fixup.

>
>
> Also another thought - should we allow FAULT_FLAG_RETRY_NOWAIT to retry
> with the VMA lock? As this could also be some unbounded spinning? But then
> again, if we bound the spinning that might be ok.

Hmm. If we do that, let's make it a separate series. Too many moving
parts is never a good idea.

>
> Thanks, Lorenzo

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Barry Song]

On Fri, Jul 10, 2026 at 10:54 PM Suren Baghdasaryan <surenb@google.com> wrote:
[...]
> > Have you audited every single VM_FAULT_RETRY case? Because getting this
> > wrong could be a serious problem and break things. You should include a
> > list of call sites in your patch and explain why it's fine in each case.
>
> That's my main worry with this patch as well. I think a better
> approach would keep VM_FAULT_RETRY as is (falling back to mmap_lock),
> introduce VM_FAULT_RETRY_VMA (or whatever is best to indicate a retry
> with VMA lock) and then convert individual VM_FAULT_RETRY cases to
> VM_FAULT_RETRY_VMA with proper reasoning and explanation why it is
> safe. Less churn, gradual conversion, and happy reviewers.
>

I am not really sure there are any happy reviewers here.
VM_FAULT_RETRY_VMA and the gradual migration to per-VMA lock retry
are exactly what was done in the previous versions, even since RFC v1[1] :-)

[1] https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@gmail.com/

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Suren Baghdasaryan]

On Fri, Jul 10, 2026 at 4:37 PM Barry Song <baohua@kernel.org> wrote:
>
> On Fri, Jul 10, 2026 at 10:54 PM Suren Baghdasaryan <surenb@google.com> wrote:
> [...]
> > > Have you audited every single VM_FAULT_RETRY case? Because getting this
> > > wrong could be a serious problem and break things. You should include a
> > > list of call sites in your patch and explain why it's fine in each case.
> >
> > That's my main worry with this patch as well. I think a better
> > approach would keep VM_FAULT_RETRY as is (falling back to mmap_lock),
> > introduce VM_FAULT_RETRY_VMA (or whatever is best to indicate a retry
> > with VMA lock) and then convert individual VM_FAULT_RETRY cases to
> > VM_FAULT_RETRY_VMA with proper reasoning and explanation why it is
> > safe. Less churn, gradual conversion, and happy reviewers.
> >
>
> I am not really sure there are any happy reviewers here.
> VM_FAULT_RETRY_VMA and the gradual migration to per-VMA lock retry
> are exactly what was done in the previous versions, even since RFC v1[1] :-)

Right and the pushback you received was not due to that approach,
right? So, why change something that nobody complained about? Unless I
missed another discussion asking to change this approach, in which
case I would really like to read it and understand the reasons.

>
> [1] https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@gmail.com/

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Suren Baghdasaryan]

On Fri, Jul 10, 2026 at 7:54 AM Suren Baghdasaryan <surenb@google.com> wrote:
>
> On Tue, Jul 7, 2026 at 9:52 AM Lorenzo Stoakes <ljs@kernel.org> wrote:
> >
> > +cc Vlata, the only one from the MEMORY MAPPING - LOCKING people excluded,
> > and Willy as he's been heavily involved. But see below you need to cc- way
> > more.
> >
> > (nitty I know) Maybe subject should be 'mm: default to VMA lock on
> > VM_FAULT_RETRY' or something just to make it clear it's about the retry
> > path?
> >
> > Also I guess you and Barry agreed to you taking this over? :)
> >
> > You missed a million people from cc- by the way. This time I really cannot
> > be bothered to add them all as I usually do, but it is MANDATORY to cc
> > maintainers and really you should be cc'ing reviewers too every time.
> >
> > Again I feel I need to write a bot that just autoreplies about this :)
> >
> > $ scripts/get_maintainer.pl --nogit --nogit-fallback your-patch.patch
> > <... ~36 people in total ...>
> > < lists >
> > < etc. >
> >
> > Please fix this on respin.
> >
> > On Fri, Jun 26, 2026 at 03:50:19PM +0800, Hongru Zhang wrote:
> > > From: Hongru Zhang <zhanghongru@xiaomi.com>
> > >
> > > Currently, when a page fault returns VM_FAULT_RETRY under the per-VMA
> > > lock due to folio_lock() failing, mainly because the folio is under
> > > I/O, the architecture fault handler unconditionally falls back to
> > > retrying with mmap_lock. This leads to mmap_lock contention.
> > >
> > > This patch introduces VM_FAULT_RETRY_HARD to mark the paths that
> >
> > I'm going to have to bikeshed the name :)
> >
> > While I like the idea of 'retry hard' like the movie 'die hard' :P I think,
> > since this is explicitly about the mmap lock, we should reference that?
> >
> > So maybe VM_FAULT_RETRY_MMAP?
> >
> > or VM_FAULT_RETRY_HARDER_WITH_A_VENGEANCE? ;)
> >
> > > require mmap_lock. Retries now stay under the per-VMA lock by default,
> > > and only marked paths fall back to the mmap_lock retry path.
> >
> > I think the commit message should be much clearer about the logic here.
> >
> > You are making it such that VM_FAULT_RETRY does its retry pass using a VMA
> > lock, but it retains the same 'retry and set FAULT_FLAG_TRIED' logic as
> > before.
> >
> > HOWEVER.
> >
> > On the VMA lock path the retry flag is only set if VM_FAULT_MAJOR. So now
> > you're allowing _unbounded_ VMA lock attempts (which is concerning and I
> > don't think intended by the lock code).
> >
> > None of this is clear right now, you need to documment it.
> >
> > >
> > > Based on the stress model from Kunwu Chan and Wang Lian in v2, we
> > > adapted a benchmark for a 20-core desktop environment (reduced thread
> > > count, adjusted memcg limits, 20-core Intel i7-12700). The benchmark
> > > uses concurrent page faults under memcg pressure (forcing reclaim and
> > > VM_FAULT_RETRY) with parallel munmap to amplify mmap_lock read-write
> > > contention.
> > >
> > > Throughput (higher is better):
> > > +---------+------------+------------+-------------+
> > > | Threads |  Vanilla   |  Patched   | Improvement |
> > > +---------+------------+------------+-------------+
> > > |   40    | 1071.11 /s | 1301.11 /s |   +21.5%    |
> > > +---------+------------+------------+-------------+
> > > |   60    | 1043.15 /s | 1472.36 /s |   +41.1%    |
> > > +---------+------------+------------+-------------+
> > > |   80    | 1049.39 /s | 1665.65 /s |   +58.7%    |
> > > +---------+------------+------------+-------------+
> > >
> > > mmap_lock contention count (lower is better):
> > > +---------+-----------+---------+-----------+
> > > | Threads |  Vanilla  | Patched | Reduction |
> > > +---------+-----------+---------+-----------+
> > > |   40    | 3,217,904 |  51,201 |  -98.4%   |
> > > +---------+-----------+---------+-----------+
> > > |   60    | 4,419,149 |  55,711 |  -98.7%   |
> > > +---------+-----------+---------+-----------+
> > > |   80    | 5,395,730 |  66,184 |  -98.8%   |
> > > +---------+-----------+---------+-----------+
> >
> > Yeah I worry a bit about the unbounded nature of things here, the fact
> > we're seeing such a difference could also be offset by extreme busy waiting
> > with the unbounded VMA lock retries.
> >
> > Let's experiment based on limiting this surely?
> >
> > >
> > > Benchmark and test scripts:
> > > https://gist.github.com/zhr250/c36c2c54d9351df37e12fd072d4926ef
> > >
> > > Suggested-by: Barry Song <baohua@kernel.org>
> > > Suggested-by: Suren Baghdasaryan <surenb@google.com>
> > > Signed-off-by: Hongru Zhang <zhanghongru@xiaomi.com>
> > > ---
> > > Changes since v2:
> > > - v2 required each retry path to explicitly opt in to VMA lock retry
> > >   (VM_FAULT_RETRY_VMA). This patch inverts the logic: retries default
> > >   to VMA lock, and only paths requiring mmap_lock opt out
> > >   (VM_FAULT_RETRY_HARD)
> > > - This patch corresponds to v2 patch 1/5; the remaining optimizations
> > >   will be submitted separately
> > > - Rebased on mm-unstable
> > >
> > > Link to v2:
> > > https://lore.kernel.org/all/20260430040427.4672-1-baohua@kernel.org/
> > >
> > > Link to v1:
> > > https://lore.kernel.org/all/20251127011438.6918-1-21cnbao@gmail.com/
> >
> > Hmm it's a bit odd for you to send an independent series and link to
> > somebody else's series as v1/v2 but anyway as long as Barry and you agree
> > with that approach that's fine!
> >
> > >
> > >  arch/arm/mm/fault.c       | 4 ++++
> > >  arch/arm64/mm/fault.c     | 4 ++++
> > >  arch/loongarch/mm/fault.c | 4 ++++
> > >  arch/powerpc/mm/fault.c   | 4 ++++
> > >  arch/riscv/mm/fault.c     | 4 ++++
> > >  arch/s390/mm/fault.c      | 4 ++++
> > >  arch/x86/mm/fault.c       | 4 ++++
> > >  include/linux/mm_types.h  | 9 +++++----
> > >  mm/huge_memory.c          | 2 +-
> > >  mm/memory.c               | 6 +++---
> > >  10 files changed, 37 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> > > index e62cc4be5adf..2f909fb4f7db 100644
> > > --- a/arch/arm/mm/fault.c
> > > +++ b/arch/arm/mm/fault.c
> > > @@ -391,6 +391,7 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> > >       if (!(flags & FAULT_FLAG_USER))
> > >               goto lock_mmap;
> > >
> > > +retry_vma:
> > >       vma = lock_vma_under_rcu(mm, addr);
> > >       if (!vma)
> > >               goto lock_mmap;
> > > @@ -420,6 +421,9 @@ do_page_fault(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
> > >                       goto no_context;
> > >               return 0;
> > >       }
> >
> > OK so this is really important - and it seems to be the case regardless of
> > arch.
> >
> > When VM_FAULT_RETRY is encountered under the VMA lock, we do:
> >
> >         if (fault & VM_FAULT_MAJOR)
> >                 flags |= FAULT_FLAG_TRIED;
> >
> > Which then allows _under the mmap lock_ 2 more retries.
> >
> > You're now (silently!) going to:
> >
> >         if (!(fault & VM_FAULT_MAJOR))
> >                 /* unbounded infinite VMA lock retries */
> >
> > This seems very unwise (TM).
> >
> > We should bound this somehow. A maximum number of retries? Only do the
> > minor fault thing once?
> >
> > Could you benchmark how it looks at different retry counts? We could also
> > consider other strategies, but unbounded attempts here seems not good.
> >
> > Another thing - is there any way to dedupe this code...? We're open coding
> > a lot of crap now in each of the arches and it's not good.
> >
> > A series like this is the place to fix stuff like that, not keep adding to
> > the technical debt.
> >
> > Hongru - could you see if that's possible as preceeding patches please?
> >
> > > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > > +             goto retry_vma;
> >
> > Maybe it's worth being explicit that this is the VM_FAULT_RETRY path? Otherwise
> > it's not so clear.
> >
> > You happen to only fall through here if VM_FAULT_RETRY is set.
> >
> > So e.g.:
> >
> > /**
> >  * fault_can_retry_vma_lock() - Are we able to retry a page fault holding
> >  * only the VMA lock?
> >  * @fault: The fault to check.
> >  *
> >  * We default to performing the VM_FAULT_RETRY/FAULT_FLAG_TRIED dance under
> >  * VMA lock. This allows us to avoid mmap contention on the likely case
> >  * that we encountered an ephemeral inability to VMA lock (e.g. the VMA was
> >  * write locked at the time).
> >  *
> >  * Returns: true if we can retry under the VMA lock, false otherwise.
> >  */
> > static bool fault_can_retry_vma_lock(vm_fault_t fault)
> > {
> >         VM_WARN_ON_ONCE(!(fault & VM_FAULT_RETRY));
> >
> >         return fault & VM_FAULT_RETRY_HARD;
> > }
> >
> > And put that somewhere all the arch fault logic can access.
> >
> > Then:
> >
> >         if (fault_can_retry_vma_lock(fault))
> >                 goto retry_vma;
> >
> > Is self-documenting, and the assert assures that no mistake was made with
> > it.
> >
> > Same comments for all the arches, obviously.
> >
> > > +
> > >  lock_mmap:
> > >
> > >  retry:
> > > diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> > > index 739800835920..695a09486795 100644
> > > --- a/arch/arm64/mm/fault.c
> > > +++ b/arch/arm64/mm/fault.c
> > > @@ -673,6 +673,7 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> > >       if (!(mm_flags & FAULT_FLAG_USER))
> > >               goto lock_mmap;
> > >
> > > +retry_vma:
> > >       vma = lock_vma_under_rcu(mm, addr);
> > >       if (!vma)
> > >               goto lock_mmap;
> > > @@ -719,6 +720,9 @@ static int __kprobes do_page_fault(unsigned long far, unsigned long esr,
> > >                       goto no_context;
> > >               return 0;
> > >       }
> > > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > > +             goto retry_vma;
> > > +
> > >  lock_mmap:
> > >
> > >  retry:
> > > diff --git a/arch/loongarch/mm/fault.c b/arch/loongarch/mm/fault.c
> > > index 2c93d33356e5..d7dd55722e52 100644
> > > --- a/arch/loongarch/mm/fault.c
> > > +++ b/arch/loongarch/mm/fault.c
> > > @@ -219,6 +219,7 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> > >       if (!(flags & FAULT_FLAG_USER))
> > >               goto lock_mmap;
> > >
> > > +retry_vma:
> > >       vma = lock_vma_under_rcu(mm, address);
> > >       if (!vma)
> > >               goto lock_mmap;
> > > @@ -265,6 +266,9 @@ static void __kprobes __do_page_fault(struct pt_regs *regs,
> > >                       no_context(regs, write, address);
> > >               return;
> > >       }
> > > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > > +             goto retry_vma;
> > > +
> > >  lock_mmap:
> > >
> > >  retry:
> > > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> > > index 806c74e0d5ab..65ec9a8252e7 100644
> > > --- a/arch/powerpc/mm/fault.c
> > > +++ b/arch/powerpc/mm/fault.c
> > > @@ -487,6 +487,7 @@ static int ___do_page_fault(struct pt_regs *regs, unsigned long address,
> > >       if (!(flags & FAULT_FLAG_USER))
> > >               goto lock_mmap;
> > >
> > > +retry_vma:
> > >       vma = lock_vma_under_rcu(mm, address);
> > >       if (!vma)
> > >               goto lock_mmap;
> > > @@ -517,6 +518,9 @@ 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 (!(fault & VM_FAULT_RETRY_HARD))
> > > +             goto retry_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..6c68414a1224 100644
> > > --- a/arch/riscv/mm/fault.c
> > > +++ b/arch/riscv/mm/fault.c
> > > @@ -347,6 +347,7 @@ void handle_page_fault(struct pt_regs *regs)
> > >       if (!(flags & FAULT_FLAG_USER))
> > >               goto lock_mmap;
> > >
> > > +retry_vma:
> > >       vma = lock_vma_under_rcu(mm, addr);
> > >       if (!vma)
> > >               goto lock_mmap;
> > > @@ -376,6 +377,9 @@ void handle_page_fault(struct pt_regs *regs)
> > >                       no_context(regs, addr);
> > >               return;
> > >       }
> > > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > > +             goto retry_vma;
> > > +
> > >  lock_mmap:
> > >
> > >  retry:
> > > diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> > > index 028aeb9c48d6..45a6c35044cc 100644
> > > --- a/arch/s390/mm/fault.c
> > > +++ b/arch/s390/mm/fault.c
> > > @@ -294,6 +294,7 @@ static void do_exception(struct pt_regs *regs, int access)
> > >               flags |= FAULT_FLAG_WRITE;
> > >       if (!(flags & FAULT_FLAG_USER))
> > >               goto lock_mmap;
> > > +retry_vma:
> > >       vma = lock_vma_under_rcu(mm, address);
> > >       if (!vma)
> > >               goto lock_mmap;
> > > @@ -318,6 +319,9 @@ static void do_exception(struct pt_regs *regs, int access)
> > >                       handle_fault_error_nolock(regs, 0);
> > >               return;
> > >       }
> > > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > > +             goto retry_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 63de8e8684f2..cb80070271d0 100644
> > > --- a/arch/x86/mm/fault.c
> > > +++ b/arch/x86/mm/fault.c
> > > @@ -1322,6 +1322,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> > >       if (!(flags & FAULT_FLAG_USER))
> > >               goto lock_mmap;
> > >
> > > +retry_vma:
> > >       vma = lock_vma_under_rcu(mm, address);
> > >       if (!vma)
> > >               goto lock_mmap;
> > > @@ -1351,6 +1352,9 @@ void do_user_addr_fault(struct pt_regs *regs,
> > >                                                ARCH_DEFAULT_PKEY);
> > >               return;
> > >       }
> > > +     if (!(fault & VM_FAULT_RETRY_HARD))
> > > +             goto retry_vma;
> > > +
> > >  lock_mmap:
> > >
> > >  retry:
> > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > > index 5ef78617ce93..58db6f0af6fd 100644
> > > --- a/include/linux/mm_types.h
> > > +++ b/include/linux/mm_types.h
> >
> > Please also update the comment block to add the new flag.
> >
> > > @@ -1665,10 +1665,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_RETRY_HARD     = (__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,
> >
> > Hmm I suppose there's no harm in renumbering here to keep the retries
> > together :)
> >
> > I wonder if we shouldn't invert this VM_FAULT_RETRY | VM_FAULT_RETRY_HARD
> > thing though and instead have:
> >
> > #define VM_FAULT_RETRY_MASK (VM_FAULT_RETRY | VM_FAULT_RETRY_MMAP)
> >
> > And convert code that checks VM_FAULT_RETRY to check VM_FAULT_RETRY_MASK
> > instead, then there'd be no need for an awkward check/or confusion around
> > the need for VM_FAULT_RETRY to always be set in both cases.
> >
> > You're already basically touching all the files that reference
> > VM_FAULT_RETRY so it's not actually much more churn.
> >
> > >       VM_FAULT_HINDEX_MASK    = (__force vm_fault_t)0x0f0000,
> > >  };
> > >
> > > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > > index 94bd656eeaf8..ad0129d579bc 100644
> > > --- a/mm/huge_memory.c
> > > +++ b/mm/huge_memory.c
> > > @@ -1398,7 +1398,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_RETRY_HARD;
> >
> > Why are we specifically wanting this for PMD device private entries?
> >
> > You should add a comment, and the commit message should say why.
> >
> > Actually, maybe in this common case of vmf->flags & FAULT_FLAG_VMA_LOCK ->
> > harder you can make a helper?
> >
> > Like:
> >
> > /** blahhhh some kdoc comment */
> > static vm_fault_t retry_under_mmap_if_vma_locked(struct vm_fault *vmf)
> > {
> >         if (!(vmf->flags & FAULT_FLAG_VMA_LOCK))
> >                 return 0;
> >
> >         vma_end_read(vma);
> >         return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
> > }
> >
> > And:
> >
> >         vm_fault_t ret;
> >
> >         ret = retry_under_mmap_if_vma_locked(vmf);
> >         if (ret)
> >                 return ret;
> >
> > Or something like this.
> >
> > >       }
> > >
> > >       ptl = pmd_lock(vma->vm_mm, vmf->pmd);
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index ff338c2abe92..7f2a30b5efca 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -3797,7 +3797,7 @@ static inline vm_fault_t vmf_can_call_fault(const struct vm_fault *vmf)
> > >       if (vma->vm_ops->map_pages || !(vmf->flags & FAULT_FLAG_VMA_LOCK))
> > >               return 0;
> > >       vma_end_read(vma);
> > > -     return VM_FAULT_RETRY;
> > > +     return VM_FAULT_RETRY | VM_FAULT_RETRY_HARD;
> >
> > You could use my helper like above.
> >
> > >  }
> > >
> > >  /**
> > > @@ -3824,7 +3824,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_RETRY_HARD;
> >
> > This really makes me think VM_FAULT_RETRY_MMAP or something explicit would
> > be better, otherwise you have to add a comment explaining you're doing this
> > because you need the mmap read lock here since the VMA lock + speculative
> > mmap read lock acquisition didn't work out.
> >
> > >       }
> > >       if (__anon_vma_prepare(vma))
> > >               ret = VM_FAULT_OOM;
> > > @@ -4778,7 +4778,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_RETRY_HARD;
> >
> > Again, helper above self-documents.
> >
> > >                               goto out;
> > >                       }
> > >
> > >
> > > base-commit: 81652c5a65d4ae28e9b18c16ef917a40025c3653
> > > --
> > > 2.43.0
> > >
> >
> > Have you audited every single VM_FAULT_RETRY case? Because getting this
> > wrong could be a serious problem and break things. You should include a
> > list of call sites in your patch and explain why it's fine in each case.
>
> That's my main worry with this patch as well. I think a better
> approach would keep VM_FAULT_RETRY as is (falling back to mmap_lock),
> introduce VM_FAULT_RETRY_VMA (or whatever is best to indicate a retry
> with VMA lock) and then convert individual VM_FAULT_RETRY cases to
> VM_FAULT_RETRY_VMA with proper reasoning and explanation why it is
> safe. Less churn, gradual conversion, and happy reviewers.
>
> >
> > One case I noticed is uffd:
> >
> >         ret = VM_FAULT_RETRY;
> >         ...
> >         if (unlikely(READ_ONCE(ctx->released))) {
> >                 /*
> >                  * If a concurrent release is detected, do not return
> >                  * VM_FAULT_SIGBUS or VM_FAULT_NOPAGE, but instead always
> >                  * return VM_FAULT_RETRY with lock released proactively.
> > .       ...
> >                  * If we were to return VM_FAULT_NOPAGE, it would work for
> >                  * the fault path, in which the lock will be released
> >                  * later.  However for GUP, faultin_page() does nothing
> >                  * special on NOPAGE, so GUP would spin retrying without
> >                  * releasing the mmap read lock, causing possible livelock.
> >                  *
> >                  * Here only VM_FAULT_RETRY would make sure the mmap lock
> >                  * be released immediately, so that the thread concurrently
> >                  * releasing the userfault would always make progress.
> >                  */
> >                  release_fault_lock(vmf);
> >                  goto out;
> >         }
> >
> > The comment needs updating (Suren?) to reflect the VMA lock, basically with
> > VMA lock held this is meaningless.
>
> Right, I missed it when enabling per-VMA locks for userfaultfd faults
> in [1]. The comment is true if we already fell back to mmap_lock but
> not if we are handling the fault under VMA lock. I should amend it.
>
> [1] https://lore.kernel.org/all/20230630211957.1341547-1-surenb@google.com/
>
> >
> > We're actually safe here in general as if the VMA lock is contended on a
> > competing write lock we drop to mmap lock.
> >
> > A racing uffd release thread does an unconditional vma_start_write()
> > _after_ setting ctx->released:
> >
> > static int userfaultfd_release(struct inode *inode, struct file *file)
> > {
> >         ...
> >         WRITE_ONCE(ctx->released, true);
> >         userfaultfd_release_all(mm, ctx);
> >         ...
> > }
> >
> > userfaultfd_release_all()
> > <mmap write lock>
> > -> userfaultfd_clear_vma()
> > -> vma_modify_flags_uffd()
> > -> vma_modify()
> > -> vma_merge_existing_range()   -> vma_start_write() LIVELOCK
> > OR split_vma() -> __split_vma() -> vma_start_write() LIVELOCK
> >
> > So yeah, not sure on this one, maybe we're fine just to use VMA lock still?
> >
> > Suren - any thoughts?
>
> If we retry without falling back to mmap_lock then
> userfaultfd_release() can end up write-locking mmap_lock but losing
> the race for VMA lock to the retrying pagefault handler. This means it
> would block while holding mmap_lock for write - not good.

I was thinking more about this... Writer blocking on a VMA write lock
while holding write side of the mmap_lock is what all VMA modifyers
do, so userfaultfd_release() doing that should be fine. As long as the
usual locking order of taking mmap_lock first then locking VMA is
followed, we should be ok.

> With pagefault handler falling back to mmap_lock, either
> userfaultfd_release() wins the race for mmap_lock or pagefault
> handler, so I think we are safe.
>
> >
> > You definitely need to explain your reasoning in the commit message at
> > least.
> >
> > Also commit 29a22b9e08d7 ("mm: handle userfaults under VMA lock") did fail
> > to update this comment to reflect VMA locks - we should fix that too -
> > Suren? :)
>
> Yes, I will correct my mistake and send a small fixup.
>
> >
> >
> > Also another thought - should we allow FAULT_FLAG_RETRY_NOWAIT to retry
> > with the VMA lock? As this could also be some unbounded spinning? But then
> > again, if we bound the spinning that might be ok.
>
> Hmm. If we do that, let's make it a separate series. Too many moving
> parts is never a good idea.
>
> >
> > Thanks, Lorenzo

Re: [RFC PATCH v3] mm: retry page faults under per-VMA lock when mmap_lock is not required

[Matthew Wilcox]

On Tue, Jul 07, 2026 at 05:52:31PM +0100, Lorenzo Stoakes wrote:
> +cc Vlata, the only one from the MEMORY MAPPING - LOCKING people excluded,
> and Willy as he's been heavily involved. But see below you need to cc- way
> more.

Ugh, I hadn't seen this.

No.

I've been looking into this problem, and as usual it has turned into a
yak-shaving exercise.  I think I'm five yaks deep at this point:

 - Need to start by simplifying filemap_fault
   https://lore.kernel.org/linux-mm/20260625195040.2508362-1-willy@infradead.org/
 - To do that, need to improve how we handle unlocking for faults
   (not posted because ...)
 - To do that, need to redo the hugetlb fault handler
   (not posted because ...)
 - To do that, need to get Jane's series landed
   https://lore.kernel.org/linux-mm/20260617172534.1740152-1-jane.chu@oracle.com/
   https://lore.kernel.org/linux-mm/20260709184740.1286561-1-willy@infradead.org/
 - To do that, need to redo how we handle hwpoison of hugetlb code
   https://lore.kernel.org/linux-mm/alEpBqExnyvBnnxi@casper.infradead.org/

So yeah, I'm not impressed by the approach taken in this patch which
adds more complexity on top of a system that's creaking at the seams
with existing complexity.