Re: [PATCH v4 0/4] mm/userfaultfd: modulize memory types

[Mike Rapoport <rppt@kernel.org>]

Hi Liam,

On Thu, Oct 30, 2025 at 01:13:24PM -0400, Liam R. Howlett wrote:
> * Peter Xu <peterx@redhat.com> [251021 12:28]:
> 
> ...
> 
> > Can you send some patches and show us the code, help everyone to support
> > guest-memfd minor fault, please?
> 
> Patches are here:
> 
> https://git.infradead.org/?p=users/jedix/linux-maple.git;a=shortlog;h=refs/heads/modularized_mem

It's really cool you picked up the gauntlet and invested this effort into
refactoring of uffd!

I agree that userfault code begs for cleanups after the sh^W stuff has been
piling over and over, but ...
 
> This is actually modularized memory types.  That means there is no
> hugetlb.h or shmem.h included in mm/userfaultfd.c code.
> 
> uffd_flag_t has been removed.  This was turning into a middleware and
> it is not necessary.  Neither is supported_ioctls.
> 
> hugetlb now uses the same functions as every other memory type,
> including anon memory.
> 
> Any memory type can change functionality without adding instructions or
> flags or anything to some other code.
> 
> This code passes uffd-unit-test and uffd-wp-mremap (skipped the swap
> tests).
> 
> guest-memfd can implement whatever it needs to (or use others
> implementations), like shmem_uffd_ops here:
> 
> static const struct vm_uffd_ops shmem_uffd_ops = {
>         .copy                   =       shmem_mfill_atomic_pte_copy,
>         .zeropage               =       shmem_mfill_atomic_pte_zeropage,
>         .cont                   =       shmem_mfill_atomic_pte_continue,
>         .poison                 =       mfill_atomic_pte_poison,
>         .writeprotect           =       uffd_writeprotect,
>         .is_dst_valid           =       shmem_is_dst_valid,
>         .increment              =       mfill_size,
>         .failed_do_unlock       =       uffd_failed_do_unlock,
>         .page_shift             =       uffd_page_shift,
>         .complete_register      =       uffd_complete_register,
> };   

... I don't think it's the right level of abstraction to add as uffd_ops to
vmap_ops.
 
As I see it, we have two levels where things are different: hugetlb vs
others at the very core of mfill_atomic() and then how different pte-based
types implement a single page operations, i.e copy/zeropage/continue.

So to separate hugetlb code from userfault we need something like 

	->get_parent_pagetable()
	->pagesize()
	->mfill_atomic_page()

and apparently something like your complete_register() and maybe
is_dst_valid().

But to provide hooks for shmem, anon and potentially guest_memfd() we
should be looking at callbacks to get a folio to populate a PTE, either for
copy or continue, and Peter's ->minor_get_folio() seems to me the right
level of abstraction to expose at vm_uffd_ops.

I believe we can extract ->get_folio() and ->put_folio() from
shmem_mfill_atomic_pte() and call them from mfill_atomic_pte_copy().

> Where guest-memfd needs to write the one function:
> guest_memfd_pte_continue(), from what I understand.
> 
> Obviously some of the shmem_ functions would need to be added to a
> header, or such.
> 
> And most of that can come from shmem_mfill_atomic_pte_continue(), from
> what I understand.  This is about 40 lines of code, but may require
> exposing some shmem functions to keep the code that compact.

This seems to me an overkill to implement MFILL_ATOMIC_CONTINUE for
guest_memfd().
I think it should be able to register a callback to provide a singe folio
at a given file offset if that folio is in the guest_memfd's page cache.
No reason for guest_memfd to start re-implementing locking, acquiring of
PMD and updating the page table, even if it only needs to call functions
from userfaultfd

> So we don't need to expose getting a folio to a module, or decode any
> special flags or whatever.  We just call the function that needs to be
> called on the vma that is found.

Agree about exposing flags to a module and about limiting API to functions
only.
 
> Thanks,
> Liam
> 

-- 
Sincerely yours,
Mike.