Re: [PATCH v2 1/2] mm/mempolicy: use vm_normal_folio_pmd() in queue_folios_pmd()

[Lorenzo Stoakes (ARM)]

On Fri, Sep 11, 2026 at 11:48:32PM -0400, Gregory Price wrote:
> mmap a VM_PFNMAP region whose ->huge_fault installs a PMD through
> vmf_insert_pfn_pmd() - a vfio-pci MMIO BAR does this - then
>
> 	mbind(p, len, MPOL_BIND, &mask, maxnode, MPOL_MF_STRICT);
>
> With a stand-in module for the driver:
>
>   BUG: unable to handle page fault for address: fffff96dc0000008
>   RIP: 0010:queue_folios_pte_range+0xaf/0x440
>    walk_pgd_range+0x52b/0xaf0
>    __walk_page_range+0x6a/0x1d0
>    walk_page_range_mm_unsafe+0x193/0x230
>    queue_pages_range+0x64/0xa0
>    do_mbind+0x25e/0x640
>
> queue_folios_pmd(), inlined above, calls pmd_folio() on that PMD.  The pfn

This makes me wonder if pmd_folio() is just generally defunct in favour of
vm_normal_folio_pmd()?

> is raw MMIO with no memmap entry, so the folio lands in unpopulated
> vmemmap.  Neither guard stops the walk:
>
>   walk_page_test()         skips VM_PFNMAP, but queue_pages_walk_ops
>                            supplies ->test_walk, so it never runs
>   queue_pages_test_walk()  honours vma_migratable(), but only while
>                            MPOL_MF_STRICT is clear
>
> A VM_MIXEDMAP vma needs neither flag, being vma_migratable(), so plain
> mbind(MPOL_MF_MOVE) reaches this too - and there the bad folio carries on
> into migrate_folio_add() and folio_isolate_lru().  mshv_vtl_low is such a
> mapping.
>
> Use vm_normal_folio_pmd() and skip on NULL, as the PTE loop in
> queue_folios_pte_range() already does with vm_normal_folio().  On the NULL
> path, retain ACTION_CONTINUE handling for the huge zero PMD.
>
> mbind(MPOL_MF_STRICT) over a PMD mapped VM_PFNMAP region now returns 0
> rather than -EIO.  The PTE loop already returned 0 there.
>
> Fixes: 3c8e44c9b369 ("mm: mark special bits for huge pfn mappings when inject")
> Reported-by: sashiko-bot <sashiko-bot@xxxxxxxxxx>
> Closes: https://sashiko.dev/#/patchset/20260817220810.1175596-1-gourry%40gourry.net
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
> Acked-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> ---
>  mm/mempolicy.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 2ad0a5f18280..8fc8a975657e 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -667,7 +667,8 @@ static inline bool queue_folio_required(struct folio *folio,
>  	return node_isset(nid, *qp->nmask) == !(flags & MPOL_MF_INVERT);
>  }
>
> -static void queue_folios_pmd(pmd_t *pmd, struct mm_walk *walk)
> +static void queue_folios_pmd(pmd_t *pmd, unsigned long addr,
> +			     struct mm_walk *walk)
>  {
>  	struct folio *folio;
>  	struct queue_pages *qp = walk->private;
> @@ -678,13 +679,14 @@ static void queue_folios_pmd(pmd_t *pmd, struct mm_walk *walk)
>  			qp->nr_failed++;
>  		return;
>  	}
> -	folio = pmd_folio(pmdval);
> -	if (folio_is_zone_device(folio))
> -		return;
> -	if (is_huge_zero_folio(folio)) {
> -		walk->action = ACTION_CONTINUE;
> +	folio = vm_normal_folio_pmd(walk->vma, addr, pmdval);
> +	if (!folio) {
> +		if (is_huge_zero_pmd(pmdval))
> +			walk->action = ACTION_CONTINUE;

Since you've already identified that this is a leaf PMD, is there really any
difference between setting walk->action to ACTION_CONTINUE or not?

There's nothing to descend to in the subtree here so either way you continue to
the next PMD entry don't you?

So there's a pmd_entry specified for both set of walk ops:

static const struct mm_walk_ops queue_pages_walk_ops = {
	.hugetlb_entry		= queue_folios_hugetlb,
	.pmd_entry		= queue_folios_pte_range,
	.test_walk		= queue_pages_test_walk,
	.walk_lock		= PGWALK_RDLOCK,
};

static const struct mm_walk_ops queue_pages_lock_vma_walk_ops = {
	.hugetlb_entry		= queue_folios_hugetlb,
	.pmd_entry		= queue_folios_pte_range,
	.test_walk		= queue_pages_test_walk,
	.walk_lock		= PGWALK_WRLOCK,
};

But no pte_entry or install_pte handlers.

Reading walk_pmd_range() in mm/pagewalk.c:

static int walk_pmd_range(pud_t *pud, unsigned long addr, unsigned long end,
			  struct mm_walk *walk)
{
	bool has_handler = ops->pte_entry;
	...
	bool has_install = ops->install_pte;
	...

	do {
again:
		walk->action = ACTION_SUBTREE;
		...
		if (ops->pmd_entry)
			err = ops->pmd_entry(pmd, addr, next, walk);
		if (err)
			break;

		if (walk->action == ACTION_AGAIN)
			goto again;
		if (walk->action == ACTION_CONTINUE)
			continue;

		if (!has_handler) { /* No handlers for lower page tables. */
			if (!has_install)
				continue; /* Nothing to do. */
			...
		}
	...
	} while (pmd++, addr = next, addr != end);
	...
}

It seems that setting ACTION_CONTINUE or not makes no difference.

This is really just a long way of saying maybe you could just do:

	folio = vm_normal_folio_pmd(walk->vma, addr, pmdval);
	if (!folio)
		return;

? :)

>  		return;
>  	}
> +	if (folio_is_zone_device(folio))
> +		return;
>  	if (!queue_folio_required(folio, qp))
>  		return;
>  	if (!(qp->flags & (MPOL_MF_MOVE | MPOL_MF_MOVE_ALL)) ||
> @@ -717,7 +719,7 @@ static int queue_folios_pte_range(pmd_t *pmd, unsigned long addr,
>
>  	ptl = pmd_trans_huge_lock(pmd, vma);
>  	if (ptl) {
> -		queue_folios_pmd(pmd, walk);
> +		queue_folios_pmd(pmd, addr, walk);
>  		spin_unlock(ptl);
>  		goto out;
>  	}
> --
> 2.55.0
>

--
Cheers, Lorenzo