Re: [PATCH RFC 00/17] mm, kvm: allow uffd suppot in guest_memfd

[Mike Rapoport <rppt@kernel.org>]

On Mon, Feb 09, 2026 at 04:35:47PM +0100, David Hildenbrand (Arm) wrote:
> On 2/3/26 21:56, Peter Xu wrote:
>
> > +static vm_fault_t fault_process_userfaultfd(struct vm_fault *vmf)
> > +{
> > +	struct vm_area_struct *vma = vmf->vma;
> > +	struct inode *inode = file_inode(vma->vm_file);
> > +	/*
> > +	 * NOTE: we could double check this hook present when
> > +	 * UFFDIO_REGISTER on MISSING or MINOR for a file driver.
> > +	 */
> > +	struct folio *folio =
> > +	    vma->vm_ops->uffd_ops->get_folio_noalloc(inode, vmf->pgoff);
> > +
> > +	if (!IS_ERR_OR_NULL(folio)) {
> > +		/*
> > +		 * TODO: provide a flag for get_folio_noalloc() to avoid
> > +		 * locking (or even the extra reference?)
> > +		 */
> > +		folio_unlock(folio);
> > +		folio_put(folio);
> > +		if (userfaultfd_minor(vma))
> > +			return handle_userfault(vmf, VM_UFFD_MINOR);
> > +	} else {
> > +		return handle_userfault(vmf, VM_UFFD_MISSING);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >   /*
> >    * The mmap_lock must have been held on entry, and may have been
> >    * released depending on flags and vma->vm_ops->fault() return value.
> > @@ -5370,16 +5397,20 @@ static vm_fault_t __do_fault(struct vm_fault *vmf)
> >   			return VM_FAULT_OOM;
> >   	}
> > +	/*
> > +	 * If this is an userfaultfd trap, process it in advance before
> > +	 * triggering the genuine fault handler.
> > +	 */
> > +	if (userfaultfd_missing(vma) || userfaultfd_minor(vma)) {
> > +		ret = fault_process_userfaultfd(vmf);
> > +		if (ret)
> > +			return ret;
> > +	}

I agree this is neater than handling VM_FAULT_UFFD.
I'd just move the checks for userfaultfd_minor() and userfaultfd_missing()
inside fault_process_userfaultfd().

> > +
> >   	ret = vma->vm_ops->fault(vmf);
> >   	if (unlikely(ret & (VM_FAULT_ERROR | VM_FAULT_NOPAGE | VM_FAULT_RETRY |
> > -			    VM_FAULT_DONE_COW | VM_FAULT_UFFD_MINOR |
> > -			    VM_FAULT_UFFD_MISSING))) {
> > -		if (ret & VM_FAULT_UFFD_MINOR)
> > -			return handle_userfault(vmf, VM_UFFD_MINOR);
> > -		if (ret & VM_FAULT_UFFD_MISSING)
> > -			return handle_userfault(vmf, VM_UFFD_MISSING);
> > +			    VM_FAULT_DONE_COW)))
> >   		return ret;
> > -	}
> >   	folio = page_folio(vmf->page);
> >   	if (unlikely(PageHWPoison(vmf->page))) {
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index eafd7986fc2ec..5286f28b3e443 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -2484,13 +2484,6 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
> >   	fault_mm = vma ? vma->vm_mm : NULL;
> >   	folio = filemap_get_entry(inode->i_mapping, index);
> > -	if (folio && vma && userfaultfd_minor(vma)) {
> > -		if (!xa_is_value(folio))
> > -			folio_put(folio);
> > -		*fault_type = VM_FAULT_UFFD_MINOR;
> > -		return 0;
> > -	}
> > -
> >   	if (xa_is_value(folio)) {
> >   		error = shmem_swapin_folio(inode, index, &folio,
> >   					   sgp, gfp, vma, fault_type);
> > @@ -2535,11 +2528,6 @@ static int shmem_get_folio_gfp(struct inode *inode, pgoff_t index,
> >   	 * Fast cache lookup and swap lookup did not find it: allocate.
> >   	 */
> > -	if (vma && userfaultfd_missing(vma)) {
> > -		*fault_type = VM_FAULT_UFFD_MISSING;
> > -		return 0;
> > -	}
> > -
> >   	/* Find hugepage orders that are allowed for anonymous shmem and tmpfs. */
> >   	orders = shmem_allowable_huge_orders(inode, vma, index, write_end, false);
> >   	if (orders > 0) {
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 14cca057fc0ec..bd0de685f42f8 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -421,26 +421,6 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
> >   	folio = __filemap_get_folio(inode->i_mapping, vmf->pgoff,
> >   				    FGP_LOCK | FGP_ACCESSED, 0);
> > -	if (userfaultfd_armed(vmf->vma)) {
> > -		/*
> > -		 * If userfaultfd is registered in minor mode and a folio
> > -		 * exists, return VM_FAULT_UFFD_MINOR to trigger the
> > -		 * userfaultfd handler.
> > -		 */
> > -		if (userfaultfd_minor(vmf->vma) && !IS_ERR_OR_NULL(folio)) {
> > -			ret = VM_FAULT_UFFD_MINOR;
> > -			goto out_folio;
> > -		}
> > -
> > -		/*
> > -		 * Check if userfaultfd is registered in missing mode. If so,
> > -		 * check if a folio exists in the page cache. If not, return
> > -		 * VM_FAULT_UFFD_MISSING to trigger the userfaultfd handler.
> > -		 */
> > -		if (userfaultfd_missing(vmf->vma) && IS_ERR_OR_NULL(folio))
> > -			return VM_FAULT_UFFD_MISSING;
> > -	}
> > -
> >   	/* folio not in the pagecache, try to allocate */
> >   	if (IS_ERR(folio))
> >   		folio = __kvm_gmem_folio_alloc(inode, vmf->pgoff);
> 
> That looks better in general. We should likely find a better/more consistent
> name for fault_process_userfaultfd().

__do_userfault()? :)
 
> -- 
> Cheers,
> 
> David

-- 
Sincerely yours,
Mike.