Hi Sean,
On Thu, Apr 02, 2026 at 03:05:07PM -0700, Sean Christopherson wrote:
> On Thu, Apr 02, 2026, Mike Rapoport wrote:
>
> > +#ifdef CONFIG_USERFAULTFD
> > +static bool kvm_gmem_can_userfault(struct vm_area_struct *vma, vm_flags_t
> > vm_flags)
> > +{
> > + struct inode *inode = file_inode(vma->vm_file);
> > +
> > + /*
> > + * Only support userfaultfd for guest_memfd with INIT_SHARED flag.
> > + * This ensures the memory can be mapped to userspace.
> > + */
> > + if (!(GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED))
> > + return false;
>
> I'm not comfortable with this change. It works for now, but it's going to be
> wildly wrong when in-place conversion comes along. While I agree with the
> "Let's
> solve each problem in it's time :)"[*], the time for in-place conversion is
> now.
> In-place conversion isn't landing this cycle or next, but it's been in
> development
> for longer than UFFD support, and I'm not willing to punt solvable problems to
> that series, because it's plenty fat as is.
I'm not against solving it as a part of uffd support, but since we are very
close to the merge window, for now I asked Andrew to drop drop guest_memfd
patches from the set and only move forward with the refactoring of uffd and
shmem that has value on its own.
> Happily, IIUC, this is an easy problem to solve, and will have a nice side
> effect
> for the common UFFD code.
>
> My objection to an early, global "can_userfault()" check is that it's
> guaranteed
> to cause TOCTOU issues. E.g. for VM_UFFD_MISSING and VM_UFFD_MINOR, the
> check on
> whether or not a given address can be faulted in needs to happen in
> __do_userfault(),
> not broadly when VM_UFFD_MINOR is added to a VMA. Conceptually, that also
> better
> aligns the code with the "normal" user fault path in
> kvm_gmem_fault_user_mapping().
>
> diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
> index 6f33307c2780..8a2d0625ffa3 100644
> --- a/include/linux/userfaultfd_k.h
> +++ b/include/linux/userfaultfd_k.h
> @@ -82,8 +82,8 @@ extern vm_fault_t handle_userfault(struct vm_fault *vmf,
> unsigned long reason);
>
> /* VMA userfaultfd operations */
> struct vm_uffd_ops {
> - /* Checks if a VMA can support userfaultfd */
> - bool (*can_userfault)(struct vm_area_struct *vma, vm_flags_t
> vm_flags);
> + /* What UFFD flags/modes are supported. */
> + const vm_flags_t supported_uffd_flags;
VMA maintainers really didn't like a fields flag in vm_uffd_ops when it was
proposed earlier, but an indirect call may work.
> /*
> * Called to resolve UFFDIO_CONTINUE request.
> * Should return the folio found at pgoff in the VMA's pagecache if it
>
> with usage like:
>
> static const struct vm_uffd_ops shmem_uffd_ops = {
> .supported_uffd_flags = __VM_UFFD_FLAGS,
> .get_folio_noalloc = shmem_get_folio_noalloc,
> .alloc_folio = shmem_mfill_folio_alloc,
> .filemap_add = shmem_mfill_filemap_add,
> .filemap_remove = shmem_mfill_filemap_remove,
> };
>
> All in all, somelike like so (completely untested):
>
> ---
> include/linux/userfaultfd_k.h | 4 +-
> mm/filemap.c | 1 +
> mm/hugetlb.c | 8 +---
> mm/shmem.c | 7 +--
> mm/userfaultfd.c | 6 +--
> virt/kvm/guest_memfd.c | 80 ++++++++++++++++++++++++++++++++++-
> 6 files changed, 87 insertions(+), 19 deletions(-)
Let's revisit after -rc1 :)
--
Sincerely yours,
Mike.
