Re: [PATCH v2 1/4] mm: Introduce vm_uffd_ops API

[Peter Xu]

On Mon, Sep 22, 2025 at 07:20:50PM +0200, David Hildenbrand wrote:
> On 18.09.25 20:20, Peter Xu wrote:
> > On Thu, Sep 18, 2025 at 07:53:46PM +0200, David Hildenbrand wrote:
> > > Re Nikita: If we could just reuse fault() for userfaultfd purposes, that
> > > might actually be pretty nice.
> > 
> > I commented on that.
> > 
> > https://lore.kernel.org/all/aEiwHjl4tsUt98sh@x1.local/
> > 
> > That'll need to leak FAULT_FLAG_USERFAULT_CONTINUE which isn't necessary,
> > make it extremely hard to know when to set the flag, and comlicates the
> > fault path which isn't necessary.
> 
> I agree that FAULT_FLAG_USERFAULT_CONTINUE would be a very weird thing to
> have.
> 
> I was wondering whether it could be abstracted in a cleaner way, similar to
> what you described with the "NO_USERFAULT", but possibly taking it one step
> further (if possible ...).
> 
> In your reply you also mentioned "whether we can also avoid reusing fault()
> but instead resolve the page faults using the vm_ops hook too".
> 
> So it kind-of is a special type of page fault, but the question would be how
> that could be integrated more cleanly. And as you also point out, the
> question would be which other users it might really have.
> 
> In GUP we achieve not triggering userfaultfd by not setting
> FAULT_FLAG_ALLOW_RETRY.
> 
> But it's rather that not allowing to retry (drop+retake locks) makes it
> impossible to call into userfaultfd.

Right.

> 
> So not sure if abstracting/reusing that would make sense.

Direct reuse is unlikely.  The current semantics of FAULT_FLAG_ALLOW_RETRY
is very specific, meanwhile handle_userfaultfd() actually has code to warn
already when it's not set..

In this context, jumping into handle_userfault() is already wrong because
this is about a request to fetch the page cache without being trappable.
So if there'll be a new flag, it'll need to bypass handle_userfault().

But then if we'll need a new flag anyway.. IMHO it'll still be cleaner with
uffd_get_folio().  It then sticks together with the uffd description of the
memory type when one wants to opt-in with MINOR faults, which should also
explicitly invoked only in userfaultfd minor fault reslutions (hence, no
chance of breaking any form of fault() either..).

Thanks,

-- 
Peter Xu