Re: [PATCH v17 04/12] mm/hmm: do the plumbing for HMM to participate in migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/hmm: hmm_vma_walk_pmd() skips remaining PTEs on folio split failure
--

commit 467255657c2d81a197c74dccb6095cef3e48051a
Author: Mika Penttilä <mpenttil@redhat.com>

mm/hmm: do the plumbing for HMM to participate in migration

This commit prepares hmm_range_fault() and the HMM pagewalk callbacks for
migration collection by adding PMD and PTE locking during the walk and wiring
in the migration preparation hooks.

> diff --git a/mm/hmm.c b/mm/hmm.c
> index 9220e6648636d..854d2c090b20c 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c

[ ... ]

> @@ -532,24 +739,65 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
>  	 * recover.
>  	 */
>  	if (pmd_bad(pmd)) {
> -		if (hmm_range_need_fault(hmm_vma_walk, hmm_pfns, npages, 0))
> +		if (hmm_range_need_fault(hmm_vma_walk, hmm_pfns_start, npages, 0))
>  			return -EFAULT;
> -		return hmm_pfns_fill(start, end, range, HMM_PFN_ERROR);
> +		return hmm_pfns_fill(start, end, hmm_vma_walk, HMM_PFN_ERROR);
>  	}
>  
> -	ptep = pte_offset_map(pmdp, addr);
> -	if (!ptep)
> +	if (minfo) {
> +		ptep = pte_offset_map_lock(mm, pmdp, addr, &hmm_vma_walk->ptl);
> +		if (ptep)
> +			hmm_vma_walk->ptelocked = true;
> +	} else {
> +		ptep = pte_offset_map(pmdp, addr);
> +	}
> +	if (!ptep) {
> +		addr = start;
>  		goto again;
> +	}
> +
>  	for (; addr < end; addr += PAGE_SIZE, ptep++, hmm_pfns++) {
> -		int r;
>  
>  		r = hmm_vma_handle_pte(walk, addr, end, pmdp, ptep, hmm_pfns);
>  		if (r) {
> -			/* hmm_vma_handle_pte() did pte_unmap() */
> +			/* hmm_vma_handle_pte() did pte_unmap() / pte_unmap_unlock */
> +			if (unmapped)
> +				flush_tlb_range(walk->vma, start, addr);
>  			return r;
>  		}
> +
> +		r = hmm_vma_handle_migrate_prepare(walk, pmdp, ptep, addr, hmm_pfns, &unmapped);
> +		if (r == -EAGAIN) {
> +			HMM_ASSERT_UNLOCKED(hmm_vma_walk);
> +			if (unmapped) {
> +				flush_tlb_range(walk->vma, start, addr);
> +				unmapped = false;
> +			}
> +			goto again;
> +		}
> +		if (r) {
> +			/*
> +			 * Splitting the folio failed; migrate_vma_split_folio()
> +			 * already dropped the PTE lock. Don't migrate the rest
> +			 * of the pmd range; addr keeps its valid pfn.
> +			 */
> +			HMM_ASSERT_UNLOCKED(hmm_vma_walk);
> +			if (unmapped)
> +				flush_tlb_range(walk->vma, start, addr);
> +			return 0;
> +		}

[Severity: Medium]
As reported by Sashiko on v16 [1], when hmm_vma_handle_migrate_prepare()
returns a non-zero error other than -EAGAIN in hmm_vma_walk_pmd() (such as
when migrate_vma_split_folio() fails to split a PTE-mapped large folio in
the subsequent patch "mm/hmm: migrate collection in HMM pagewalk - pte
level"), does returning 0 here leave the remaining entries in
[addr + PAGE_SIZE, end) completely unprocessed?

At the start of a migrating walk, hmm_range_fault_locked() clears
range->hmm_pfns[]:

mm/hmm.c:hmm_range_fault_locked() {
	...
	if (hmm_select_migrate(range)) {
		WARN_ON_ONCE(range->pfn_flags_mask);
		WARN_ON_ONCE(range->dev_private_owner !=
			     range->migrate->pgmap_owner);
		memset(range->hmm_pfns, 0,
		       ((range->end - range->start) >> PAGE_SHIFT) *
		       sizeof(*range->hmm_pfns));
	}
	...
}

While the current address addr already ran hmm_vma_handle_pte() and keeps its
valid PFN, returning 0 directly from the PTE loop terminates processing for
the rest of the PMD range [addr + PAGE_SIZE, end) while reporting success to
walk_page_range().

Consequently, for any addresses in [addr + PAGE_SIZE, end), neither
hmm_vma_handle_pte() nor hmm_vma_handle_migrate_prepare() is called.
Independent migratable pages in that remainder are not collected, unmapped or
read-only PTEs are not faulted in even when HMM_PFN_REQ_FAULT is requested,
and hmm_range_fault() can return 0 with those range->hmm_pfns[] entries still
zeroed and lacking HMM_PFN_VALID.

[1] https://lore.kernel.org/r/20261021060506.2377545-5-mpenttil@redhat.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103061629.972669-1-mpenttil@redhat.com?part=4