Re: [PATCH v14 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 2 potential issue(s) to consider:
- [Critical] mm: missing TLB flush on error path in hmm_vma_walk_pmd()
- [High] mm: unbalanced mmu_notifier_invalidate_range_start() on -EINTR
--

commit 7a6e1d53570e6b253761aafa0e2f9a00e3b80665
Author: Mika Penttilä <mpenttil@redhat.com>
Subject: mm/hmm: do the plumbing for HMM to participate in migration

This commit prepares hmm_range_fault() and pagewalk callbacks for migration
by adding hmm_vma_handle_migrate_prepare_pmd() and managing PMD and PTE
locks across the pagewalk.

diff --git a/mm/hmm.c b/mm/hmm.c
index be39d2e5403ab..4805c88347e67 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -398,15 +480,72 @@ static int hmm_vma_handle_absent_pmd(struct mm_walk *walk, unsigned long start,
[ ... ]
> +static int hmm_vma_capture_migrate_range(unsigned long start,
> +					 unsigned long end,
> +					 struct mm_walk *walk)
> +{
> +	struct hmm_vma_walk *hmm_vma_walk = walk->private;
> +	struct hmm_range *range = hmm_vma_walk->range;
[ ... ]
> +	if (!hmm_vma_walk->mmu_range.owner) {
> +		mmu_notifier_range_init_owner(&hmm_vma_walk->mmu_range, MMU_NOTIFY_MIGRATE, 0,
> +					      walk->vma->vm_mm, start, end,
> +					      range->dev_private_owner);
> +		mmu_notifier_invalidate_range_start(&hmm_vma_walk->mmu_range);
> +	}
> +
> +	return 0;
> +}

[Severity: High]
Does this mmu_notifier_invalidate_range_start() become unbalanced if the
fault path is interrupted?

When hmm_vma_capture_migrate_range() sets up the active range, the matching
mmu_notifier_invalidate_range_end() happens in hmm_vma_post_range_fault() at
the end of hmm_range_fault_locked().

However, hmm_range_fault_locked() has an early return for fatal signals:

mm/hmm.c:hmm_range_fault_locked() {
    ...
		if (ret == HMM_FAULT_PENDING) {
			ret = hmm_do_fault(mm, &hmm_vma_walk);
			if (ret == HMM_FAULT_UNLOCKED) {
				if (fatal_signal_pending(current))
					return -EINTR;
    ...

If we take this -EINTR return, does the MMU notifier range remain active
permanently, potentially hanging other invalidation threads?

@@ -465,46 +604,128 @@ static int hmm_vma_walk_pmd(pmd_t *pmdp,
[ ... ]
>  	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 */
>  			return r;
>  		}

[Severity: Critical]
Can this early return bypass the TLB flush for pages unmapped in previous
iterations?

If hmm_vma_handle_migrate_prepare() sets unmapped = true in an earlier loop
iteration, and then hmm_vma_handle_pte() returns an error on a subsequent
iteration, we immediately return r here.

Does this leave stale TLB entries in place for the previously unmapped pages,
since we skip the flush_tlb_range(walk->vma, start, addr) at the end of the
function?

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