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ä <[email protected]> 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/[email protected]?part=4
