Re: [PATCH RFC v3 4/6] mm/gup: add get_user_page_vma() to fault in a page under a held lock

[Usama Arif <usama.arif@linux.dev>]

On Fri, 17 Jul 2026 13:00:34 -0400 Rik van Riel <riel@surriel.com> wrote:

> __access_remote_vm() needs a single page from a VMA it has already
> looked up and locked, faulting it in when necessary, under either the
> mmap lock or the per-VMA lock. get_user_pages_remote() does not fit: it
> hard codes the mmap lock and re-looks-up the VMA, neither of which is
> wanted here.
> 
> Add get_user_page_vma(), a simplified __get_user_pages() that walks the
> page tables with follow_page_mask(), faults a missing page in with
> faultin_page(), and on success returns it with a reference and the
> caller's lock still held. Like __get_user_pages() it runs
> check_vma_flags(), so callers need not pre-check the VMA.
> 
> A VM_IO/VM_PFNMAP VMA is the exception to that check: it can still hold
> COWed pages that have a struct page, so follow_page_mask() is allowed to
> look for one. Memory with no struct page -- a raw PFN, or a present PFN
> reported as -EEXIST -- is returned as -EFAULT, so the caller can reach it
> through vma->vm_ops->access().
> 
> The caller sets FOLL_VMA_LOCK when it holds the per-VMA lock rather than
> the mmap lock, which reaches the fault code as FAULT_FLAG_VMA_LOCK.
> 
> Anything that cannot complete under the per-VMA lock -- a dropped fault,
> a userfaultfd VMA (uffd assumes current is the faulting task), a hard
> error, or ->access() memory -- releases the lock and returns -EAGAIN,
> so the caller retries under the mmap lock.
> 
> faultin_page() reports these retries as -EAGAIN for both lock types;
> only the mmap caller records the dropped lock in *locked. A
> VM_FAULT_ERROR that decodes to no errno warns and returns -EFAULT
> rather than BUG(), since the mmap-lock retry produces the definitive
> result.
> 
> Assisted-by: Claude:claude-opus-4.8
> Signed-off-by: Rik van Riel <riel@surriel.com>
> ---
>  mm/gup.c      | 154 +++++++++++++++++++++++++++++++++++++++++++-------
>  mm/internal.h |   6 +-
>  2 files changed, 139 insertions(+), 21 deletions(-)
> 
> diff --git a/mm/gup.c b/mm/gup.c
> index 0692119b7904..69b834a71708 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1082,7 +1082,13 @@ static int get_gate_page(struct mm_struct *mm, unsigned long address,
>  /*
>   * mmap_lock must be held on entry.  If @flags has FOLL_UNLOCKABLE but not

Does the above line also need to be changed? mmap_lock is not held on entry in
faultin_page() if passing FOLL_VMA_LOCK, right?

>   * FOLL_NOWAIT, the mmap_lock may be released.  If it is, *@locked will be set
> - * to 0 and -EBUSY returned.
> + * to 0 and -EAGAIN returned.
> + *
> + * The return value does not depend on the lock type: a fault that made
> + * progress but needs a retry (VM_FAULT_RETRY / VM_FAULT_COMPLETED) is reported
> + * as -EAGAIN for both the mmap lock and the per-VMA lock (FOLL_VMA_LOCK). Only
> + * the *@locked side effect is lock-type specific, as the per-VMA lock path has
> + * no unlockable mmap_lock to drop.
>   */
>  static int faultin_page(struct vm_area_struct *vma,
>  		unsigned long address, unsigned int flags, bool unshare,
> @@ -1097,6 +1103,8 @@ static int faultin_page(struct vm_area_struct *vma,
>  		fault_flags |= FAULT_FLAG_WRITE;
>  	if (flags & FOLL_REMOTE)
>  		fault_flags |= FAULT_FLAG_REMOTE;
> +	if (flags & FOLL_VMA_LOCK)
> +		fault_flags |= FAULT_FLAG_VMA_LOCK;
>  	if (flags & FOLL_UNLOCKABLE) {
>  		fault_flags |= FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE;
>  		/*
> @@ -1125,41 +1133,147 @@ static int faultin_page(struct vm_area_struct *vma,
>  
>  	ret = handle_mm_fault(vma, address, fault_flags, NULL);
>  
> +	/*
> +	 * A fully completed fault (VM_FAULT_COMPLETED) or one that needs a retry
> +	 * (VM_FAULT_RETRY) has released the lock it was holding. Report both as
> +	 * -EAGAIN so the caller retries: the mmap lock caller retakes it here,
> +	 * the per-VMA lock caller (FOLL_VMA_LOCK) falls back to the mmap lock.
> +	 *
> +	 * Dropping the mmap lock is recorded in *@locked. There is no such lock
> +	 * to drop under the per-VMA lock, where @locked is not used, so leave it
> +	 * alone in that case.
> +	 */
>  	if (ret & VM_FAULT_COMPLETED) {
> -		/*
> -		 * With FAULT_FLAG_RETRY_NOWAIT we'll never release the
> -		 * mmap lock in the page fault handler. Sanity check this.
> -		 */
> -		WARN_ON_ONCE(fault_flags & FAULT_FLAG_RETRY_NOWAIT);
> -		*locked = 0;
> -
> -		/*
> -		 * We should do the same as VM_FAULT_RETRY, but let's not
> -		 * return -EBUSY since that's not reflecting the reality of
> -		 * what has happened - we've just fully completed a page
> -		 * fault, with the mmap lock released.  Use -EAGAIN to show
> -		 * that we want to take the mmap lock _again_.
> -		 */
> +		if (!(flags & FOLL_VMA_LOCK)) {
> +			/*
> +			 * With FAULT_FLAG_RETRY_NOWAIT we'll never release the
> +			 * mmap lock in the page fault handler. Sanity check this.
> +			 */
> +			WARN_ON_ONCE(fault_flags & FAULT_FLAG_RETRY_NOWAIT);
> +			*locked = 0;
> +		}
>  		return -EAGAIN;
>  	}
>  
>  	if (ret & VM_FAULT_ERROR) {
>  		int err = vm_fault_to_errno(ret, flags);
>  
> -		if (err)
> -			return err;
> -		BUG();
> +		/*
> +		 * VM_FAULT_ERROR always decodes to an errno; a zero here would
> +		 * mean handle_mm_fault() returned an unexpected combination.
> +		 * Report -EFAULT rather than crash: under the per-VMA lock the
> +		 * mmap lock retry produces the definitive result.
> +		 */
> +		VM_WARN_ON_ONCE(!err);
> +		return err ? err : -EFAULT;
>  	}
>  
>  	if (ret & VM_FAULT_RETRY) {
> -		if (!(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
> +		if (!(flags & FOLL_VMA_LOCK) &&
> +		    !(fault_flags & FAULT_FLAG_RETRY_NOWAIT))
>  			*locked = 0;
> -		return -EBUSY;
> +		return -EAGAIN;
>  	}
>  
>  	return 0;
>  }
>  
> +/*
> + * get_user_page_vma - get one page from @vma, whose lock the caller already
> + * holds: the mmap lock, or (with FOLL_VMA_LOCK) the per-VMA lock. Walks the
> + * page tables, faulting the page in if needed, and on success returns it with
> + * a reference and the lock still held.
> + *
> + * Runs check_vma_flags() like __get_user_pages(), so callers need not pre-check
> + * the VMA; most rejections are returned as their error. A VM_IO/VM_PFNMAP VMA
> + * is the exception: a COWed page with a struct page is returned, while a raw
> + * PFN has none and yields -EFAULT, to be reached via vma->vm_ops->access().
> + *
> + * Under FOLL_VMA_LOCK, anything that cannot be finished under the per-VMA lock
> + * (a dropped fault, userfaultfd, a hard error, or ->access() memory) releases
> + * the lock and returns -EAGAIN, so the caller retries under the mmap lock.
> + */
> +struct page *get_user_page_vma(struct vm_area_struct *vma, unsigned long addr,
> +			       unsigned int gup_flags)
> +{
> +	bool vma_locked = gup_flags & FOLL_VMA_LOCK;
> +	unsigned long page_mask;
> +	struct page *page;
> +	int locked = 1;
> +	bool pfnmap;
> +	int ret;
> +
> +	/*
> +	 * Validate the VMA up front, like __get_user_pages(). A VM_IO/VM_PFNMAP
> +	 * VMA is not rejected outright: it can hold COWed pages that have a
> +	 * struct page, so let follow_page_mask() look for one, and treat only
> +	 * its struct-page-less PFNs as unreachable. Any other rejection
> +	 * (secretmem, bad permissions, ...) is final.
> +	 */
> +	ret = check_vma_flags(vma, gup_flags);
> +	if (ret && !(vma->vm_flags & (VM_IO | VM_PFNMAP)))

Do you need to somehow restructure check_vma_flags()?

The first thing that check_vma_flags() does is:

	if (vm_flags & (VM_IO | VM_PFNMAP))
		return -EFAULT;

check_vma_flags() returns before evaluating VM_READ,
FOLL_FORCE, FOLL_ANON or the architecture permission check.

The code then ignores that early error and may return a COWed
page through follow_page_mask(). A non-FOLL_FORCE caller
can therefore read a COWed page from a PROT_NONE PFNMAP VMA.

> +		goto fail;
> +	pfnmap = ret;
> +
> +	for (;;) {
> +		if (fatal_signal_pending(current)) {
> +			ret = -EINTR;
> +			goto fail;
> +		}
> +		cond_resched();
> +
> +		page = follow_page_mask(vma, addr,
> +					gup_flags | FOLL_TOUCH | FOLL_GET,
> +					&page_mask);
> +		if (!IS_ERR_OR_NULL(page))
> +			return page;

__get_user_pages() does

	flush_anon_page(vma, subpage, start + j * PAGE_SIZE);
	flush_dcache_page(subpage);

before returning the page. Do you need to that here as well above?

> +
> +		/*
> +		 * No struct page: a raw PFN of a VM_IO/VM_PFNMAP VMA, whether
> +		 * seen by the up-front check (@pfnmap) or reported as -EEXIST
> +		 * for a present PFN. Return -EFAULT so the caller reaches it
> +		 * through vma->vm_ops->access().
> +		 */
> +		if (pfnmap || PTR_ERR(page) == -EEXIST) {
> +			ret = -EFAULT;
> +			goto fail;
> +		}
> +		/* A hard error from the walk itself. */
> +		if (page && PTR_ERR(page) != -EMLINK) {
> +			ret = PTR_ERR(page);
> +			goto fail;
> +		}
> +
> +		/*
> +		 * The page is not present, or needs unsharing. A remote fault
> +		 * under the per-VMA lock cannot deliver userfaultfd (which
> +		 * assumes current is the faulting task), so fall back for those.
> +		 */
> +		if (vma_locked && userfaultfd_armed(vma)) {
> +			ret = -EAGAIN;
> +			goto fail;
> +		}
> +		ret = faultin_page(vma, addr, gup_flags | FOLL_REMOTE | FOLL_GET,
> +				   PTR_ERR(page) == -EMLINK, &locked);
> +		if (ret == -EAGAIN)
> +			return ERR_PTR(-EAGAIN);	/* fault released the per-VMA lock */
> +		if (ret)
> +			goto fail;
> +	}
> +
> +fail:
> +	/*
> +	 * Under the per-VMA lock the caller cannot reach ->access() or act on a
> +	 * hard error (both need the mmap lock), so release the lock and have it
> +	 * retry there; the mmap-lock pass produces the definitive error.
> +	 */
> +	if (vma_locked) {
> +		vma_end_read(vma);
> +		return ERR_PTR(-EAGAIN);
> +	}
> +	return ERR_PTR(ret);
> +}
> +
>  /*
>   * Writing to file-backed mappings which require folio dirty tracking using GUP
>   * is a fundamentally broken operation, as kernel write access to GUP mappings
> diff --git a/mm/internal.h b/mm/internal.h
> index 181e79f1d6a2..0899a37907c1 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1595,6 +1595,8 @@ struct vm_struct *__get_vm_area_node(unsigned long size,
>   */
>  int __must_check try_grab_folio(struct folio *folio, int refs,
>  				unsigned int flags);
> +struct page *get_user_page_vma(struct vm_area_struct *vma, unsigned long addr,
> +			       unsigned int gup_flags);
>  
>  /*
>   * mm/huge_memory.c
> @@ -1641,11 +1643,13 @@ enum {
>  	FOLL_UNLOCKABLE = 1 << 21,
>  	/* VMA lookup+checks compatible with MADV_POPULATE_(READ|WRITE) */
>  	FOLL_MADV_POPULATE = 1 << 22,
> +	/* caller holds the per-VMA lock, not the mmap lock */
> +	FOLL_VMA_LOCK = 1 << 23,
>  };
>  
>  #define INTERNAL_GUP_FLAGS (FOLL_TOUCH | FOLL_TRIED | FOLL_REMOTE | FOLL_PIN | \
>  			    FOLL_FAST_ONLY | FOLL_UNLOCKABLE | \
> -			    FOLL_MADV_POPULATE)
> +			    FOLL_MADV_POPULATE | FOLL_VMA_LOCK)
>  
>  /*
>   * Indicates for which pages that are write-protected in the page table,
> -- 
> 2.53.0-Meta
> 
>