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 | org.kvack.linux-mm,org.kernel.vger.linux-kernel |
|---|---|
| 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