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

David Hildenbrand
2025-09-22T17:20:56Z

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.

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

>
> I think Mike's comment was spot on, that the new API is literally
> do_fault() for shmem, but only used in userfaultfd context so it's even an
> oneliner.

Right, special fault handling for userfaultfd.

>
> I do not maintain mm, so above is only my two cents, so I don't make
> decisions. Personally I still prefer the current approach of keep the mm
> main fault path clean.

Well, as clean as things like FAULT_FLAG_ALLOW_RETRY are. :)

In any case, reading Liams reply, it sounded like that there is a path
forward, so I don't particularly care, just wanted to mention what I had
in mind regarding FAULT_FLAG_ALLOW_RETRY.

--
Cheers

David / dhildenb