Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] test_hmm: device page table desync on concurrent MMU invalidation --
commit fba7f5abd6e7b21ba16a0224adb82cc24650eb6e Author: Mika Penttilä <[email protected]> lib/test_hmm: add a new testcase for the migrate on fault This commit enhances the hmm test driver (lib/test_hmm) by adding a new testcase for the migrate on fault scenario. > diff --git a/lib/test_hmm.c b/lib/test_hmm.c > index cd88e8177d0ab..31608b6998bb2 100644 > --- a/lib/test_hmm.c > +++ b/lib/test_hmm.c [ ... ] > @@ -1377,6 +1398,127 @@ static int dmirror_migrate_to_device(struct dmirror > *dmirror, [ ... ] > +static int do_fault_and_migrate(struct dmirror *dmirror, struct hmm_range > *range) > +{ [ ... ] > + dmirror_migrate_alloc_and_copy(migrate, dmirror); > + migrate_vma_pages(migrate); > + > + mutex_lock(&dmirror->mutex); > + if (mmu_interval_read_retry(&dmirror->notifier, > + range->notifier_seq)) > + ret = -EBUSY; > + else > + ret = __dmirror_migrate_map_locked(migrate, dmirror); > + mutex_unlock(&dmirror->mutex); > + > + migrate_vma_finalize(migrate); [Severity: Medium] Will skipping the device page table update here create an inconsistent state that migrate_vma_finalize() permanently commits? In do_fault_and_migrate() in lib/test_hmm.c, if a concurrent page unmap (like page reclaim) invalidates the MMU notifier interval between mmu_interval_read_begin() and mmu_interval_read_retry(), the retry check fails and ret is set to -EBUSY. However, migrate_vma_pages() has already executed and populated the CPU page tables. By skipping __dmirror_migrate_map_locked() on retry, the device page table is left desynchronized. When migrate_vma_finalize() is subsequently called, the CPU page tables irreversibly commit the migration to device private memory without the new device mappings. Could this cause a wasteful ping-pong migration on the subsequent retry loop, where hmm_range_fault() triggers a fault on these device-private pages, calls the device's migrate_to_ram handler to bounce the pages back to system RAM, only to migrate them to the device again? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
