Re: [PATCH 1/4] mm: mincore: try per-VMA lock firstly and use walk_page_range_vma()

["Zi Yan" <ziy@nvidia.com>]

On Wed Jun 17, 2026 at 4:26 AM EDT, Kefeng Wang wrote:
> The mincore syscall currently takes mmap lock for the entire
> duration of the VMA lookup and page table walk. This creates
> a global contention point with page faults and other mmap_lock
> holders in multi-threaded applications.
>
> The mincore is a read-only operation that only queries page
> residency from a single VMA, making it an ideal candidate for
> per-VMA locking, so try per-vma lock firstly and use the
> walk_page_range_vma() in do_mincore() to eliminates an unnecessary
> find_vma() lookup.
>
> Unlike walk_page_range(), walk_page_range_vma() does not call
> walk_page_test(), which handles VM_PFNMAP by invoking ->pte_hole()
> to skip the page table walk. Without this check, PFNMAP PTEs
> would be treated as present by mincore_pte_range(), changing
> the returned residency status. Handle VM_PFNMAP explicitly in
> do_mincore() to preserve the original behavior.
>
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  mm/mincore.c | 71 +++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 53 insertions(+), 18 deletions(-)
>
> diff --git a/mm/mincore.c b/mm/mincore.c
> index 296f2e3922b5..a786a073feab 100644
> --- a/mm/mincore.c
> +++ b/mm/mincore.c
> @@ -12,6 +12,7 @@
>  #include <linux/gfp.h>
>  #include <linux/pagewalk.h>
>  #include <linux/mman.h>
> +#include <linux/mmap_lock.h>
>  #include <linux/syscalls.h>
>  #include <linux/swap.h>
>  #include <linux/leafops.h>
> @@ -232,34 +233,47 @@ static inline bool can_do_mincore(struct vm_area_struct *vma)
>  	       file_permission(vma->vm_file, MAY_WRITE) == 0;
>  }
>  
> -static const struct mm_walk_ops mincore_walk_ops = {
> -	.pmd_entry		= mincore_pte_range,
> -	.pte_hole		= mincore_unmapped_range,
> -	.hugetlb_entry		= mincore_hugetlb,
> -	.walk_lock		= PGWALK_RDLOCK,
> -};
> -
>  /*
>   * Do a chunk of "sys_mincore()". We've already checked
> - * all the arguments, we hold the mmap semaphore: we should
> - * just return the amount of info we're asked for.
> + * all the arguments, we should just return the amount of
> + * info we're asked for. The vma is already looked up and
> + * locked; vma_locked indicates whether the per-VMA lock
> + * or mmap_read_lock is held.
>   */
> -static long do_mincore(unsigned long addr, unsigned long pages, unsigned char *vec)
> +static long do_mincore(struct vm_area_struct *vma, unsigned long addr,
> +		unsigned long pages, unsigned char *vec, bool vma_locked)

vma_locked is confusing me, since I thought vma_locked == false means
vma is not locked, but it actually means mmap_lock is taken instead. But
I am not sure an enum vma_lock_state {VMA_LOCKED, MM_LOCKED} is needed
here.

>  {
> -	struct vm_area_struct *vma;
>  	unsigned long end;
>  	int err;
> +	struct mm_walk_ops mincore_walk_ops = {
> +		.pmd_entry		= mincore_pte_range,
> +		.pte_hole		= mincore_unmapped_range,
> +		.hugetlb_entry		= mincore_hugetlb,
> +		.walk_lock		= vma_locked ?
> +				  PGWALK_VMA_RDLOCK_VERIFY : PGWALK_RDLOCK,

An unrelated comment about PGWALK_RDLOCK. Maybe PGWALK_MM_RDLOCK_VERIFY
is a better name since the code just verifies mmap_lock, unlike
PGWALK_WRLOCK, which requires vma_start_write(). PGWALK_WRLOCK_VERIFY
might be better named as PGWALK_VMA_WRLOCK_VERIFY.

Otherwise, LGTM.

Acked-by: Zi Yan <ziy@nvidia.com>


-- 
Best Regards,
Yan, Zi