Re: [RFC PATCH v4 1/3] mm: allow page faults to request VMA-lock retry

Barry Song <[email protected]> Wed, 5 Aug 2026 05:13:49 +0800
Newsgroups gmane.linux.kernel,gmane.linux.kernel.mm
Message-ID <CAGsJ_4x_ed4m57-8rZ53PBoGy-aa-z1thOvGOp7RAb2qP28dKw@mail.gmail.com>
On Tue, Aug 4, 2026 at 8:32 PM Lorenzo Stoakes (ARM) <[email protected]> wrote:
>
> On Tue, Aug 04, 2026 at 05:52:19PM +0800, Hongru Zhang wrote:
> > From: Hongru Zhang <[email protected]>
> >
> > Page faults handled under the per-VMA lock currently fall back to the
> > mmap_lock path whenever handle_mm_fault() returns VM_FAULT_RETRY. This
> > means that lower-level fault handlers have no way to tell the
> > architecture fault handler that the retry can safely continue under the
> > per-VMA lock.
> >
> > Add VM_FAULT_MAY_USE_VMA_LOCK as an advisory bit that can be returned
>
> I don't love that name or that faulting retry behaviour is _modified_ by a
> value that indicates fault resolution state... ugh.
>
> It's kinda confusing things 'VM_FAULT_RETRY' is 'you have to retry this
> fault'.
>
> 'VM_FAULT_MAY_...' is starting to bring in effectively configuration
> options into it and that's kinda horrible.
>
> I mean is there any reason we shouldn't ALWAYS do this if a VMA lock was
> used?
>
> It's not too expensive to do a single retry with the VMA lock before
> falling back to the mmap lock.
>
> So maybe simplify like that?
>
> And like that this series becomes a single patch right?

This is a brilliant idea. That's a genius insight, Lorenzo.

I guess the conceptual model could simply be:

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index 45b99c3b1442..3592bcc9bbd7 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;

+vma_lock:
        vma = lock_vma_under_rcu(mm, address);
        if (!vma)
                goto lock_mmap;
@@ -1352,6 +1354,11 @@ void do_user_addr_fault(struct pt_regs *regs,
        if (fault & VM_FAULT_MAJOR)
                flags |= FAULT_FLAG_TRIED;

+       if (!vma_lock_retried) {
+               vma_lock_retried = true;
+               goto vma_lock;
+       }
+
        /* Quick path to respond to signals */
        if (fault_signal_pending(fault, regs)) {
                if (!user_mode(regs))

Nothing else needs to change then. I wonder if there is a cleaner
way to implement the idea, but it is really stunning.

Best Regards
Barry