On 29.06.22 05:54, Alex Sierra wrote: > This case is used to migrate pages from device memory, back to system > memory. Device coherent type memory is cache coherent from device and CPU > point of view. > > Signed-off-by: Alex Sierra <alex.sierra@xxxxxxx> > Acked-by: Felix Kuehling <Felix.Kuehling@xxxxxxx> > Reviewed-by: Alistair Poppple <apopple@xxxxxxxxxx> > Signed-off-by: Christoph Hellwig <hch@xxxxxx> I'm not too familiar with this code, please excuse my naive questions: > @@ -148,15 +148,21 @@ static int migrate_vma_collect_pmd(pmd_t *pmdp, > if (is_writable_device_private_entry(entry)) > mpfn |= MIGRATE_PFN_WRITE; > } else { > - if (!(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) > - goto next; Why not exclude MIGRATE_VMA_SELECT_DEVICE_PRIVATE here? IIRC that would have happened before this change. > pfn = pte_pfn(pte); > - if (is_zero_pfn(pfn)) { > + if (is_zero_pfn(pfn) && > + (migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) { > mpfn = MIGRATE_PFN_MIGRATE; > migrate->cpages++; > goto next; > } > page = vm_normal_page(migrate->vma, addr, pte); > + if (page && !is_zone_device_page(page) && I'm wondering if that check logically belongs into patch #2. > + !(migrate->flags & MIGRATE_VMA_SELECT_SYSTEM)) > + goto next; > + else if (page && is_device_coherent_page(page) && > + (!(migrate->flags & MIGRATE_VMA_SELECT_DEVICE_COHERENT) || > + page->pgmap->owner != migrate->pgmap_owner)) In general LGTM -- Thanks, David / dhildenb