Re: [RFC PATCH v1 1/2] x86/mm: use VMA lock for kernel faults on user addresses

On Tue, Aug 04, 2026 at 06:37:31AM +0800, Barry Song wrote:
> On Mon, Aug 3, 2026 at 11:18 PM Lorenzo Stoakes (ARM) <[email protected]> wrote:
> >
> > On Sun, Aug 02, 2026 at 03:40:17PM +0800, Barry Song (Xiaomi) wrote:
> > > Use the VMA lock for kernel faults on user addresses. This also
> > > makes the existing code below meaningful:
> > >
> > >   /* Quick path to respond to signals */
> > >   if (fault_signal_pending(fault, regs)) {
> > >           if (!user_mode(regs))
> > >                   kernelmode_fixup_or_oops(regs, error_code, address,
> > >                                            SIGBUS, BUS_ADRERR,
> > >                                            ARCH_DEFAULT_PKEY);
> > >           return;
> > >   }
> >
> > Hmm yeah :)
> >
> > Bit weird it uses user_mode(regs) and the early exit uses the just-set
> > 'flags & FAULT_FLAG_USER' too.
> >
> > Some horrible duplication here too... the user_mod_regs() etc. code is just
> > duplicated in the mmap lock path.
> >
> > I think you mentioned it on Suren/Dave's series but vma_start_read_unlocked()
> > would avoid all this and could lead to a nicely red patch.
>
> vma_start_read_unlocked() could completely avoid falling back to
> mmap_lock for binder. For page faults, however, there are still
> paths that must retry under mmap_lock, such as
> __vmf_anon_prepare() and swapcache_is_device_private().
> So we may not end up with something as clean as binder, but I
> think we can still achieve a cleaner design than what we have
> today.
>

Right, and also fault retrying (though Hongru's series is looking at giving at
least 1 more try under VMA lock there).

Let's maybe wait for that to settle first and see how that looks :)

> >
> > >
> > > Right now, the code above is dead because !user_mode always
> > > takes the mmap_lock path.
> > >
> > > Co-developed-by: Bo Zhang <[email protected]>
> > > Signed-off-by: Bo Zhang <[email protected]>
> > > Signed-off-by: Barry Song (Xiaomi) <[email protected]>
> > > ---
> > >  arch/x86/mm/fault.c | 3 ---
> > >  1 file changed, 3 deletions(-)
> > >
> > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> > > index 45b99c3b1442..c22b74e0eeaf 100644
> > > --- a/arch/x86/mm/fault.c
> > > +++ b/arch/x86/mm/fault.c
> > > @@ -1328,9 +1328,6 @@ void do_user_addr_fault(struct pt_regs *regs,
> > >       }
> > >  #endif
> > >
> > > -     if (!(flags & FAULT_FLAG_USER))
> > > -             goto lock_mmap;
> > > -
> >
> > I'm not sure why kernel faults of userland memory were excluded initially
> > (Suren?)
> >
> > Given that it's OK to take the mmap lock here and we're necessarily in process
> > context anyway surely it's OK to take the VMA lock?
> >
> > It looks fine to me but want Suren's input.
> >
> > Also given vma_start_read_unlocked() is coming maybe that's better for a
> > cleanup.
> >
> > And finally - Willy is working on a grand clean up of this stuff _I think_ so
> > you probably should coordinate with him also?
>
> I have no problem waiting for Matthew's work. It is the right
> thing to refactor the duplicated code across architectures for
> the VMA lock page fault path.

Seems sensible thanks!

>
> Thanks
> Barry

--
Cheers, Lorenzo