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
