On Thu, Aug 15, 2019 at 01:22:22PM -0400, Jerome Glisse wrote:
> On Tue, Aug 06, 2019 at 04:00:10PM +0800, Pingfan Liu wrote:
> > MIGRATE_PFN_MIGRATE marks a valid pfn, further more, suitable to migrate.
> > As for hole, there is no valid pfn, not to mention migration.
> >
> > Before this patch, hole has already relied on the following code to be
> > filtered out. Hence it is more reasonable to see hole as invalid source
> > page.
> > migrate_vma_prepare()
> > {
> > struct page *page = migrate_pfn_to_page(migrate->src[i]);
> >
> > if (!page || (migrate->src[i] & MIGRATE_PFN_MIGRATE))
> > \_ this condition
> > }
>
> NAK you break the API, MIGRATE_PFN_MIGRATE is use for 2 things,
> first it allow the collection code to mark entry that can be
> migrated, then it use by driver to allow driver to skip migration
> for some entry (for whatever reason the driver might have), we
> still need to keep the entry and not clear it so that we can
> cleanup thing (ie remove migration pte entry).
Thanks for your kindly review.
I read the code again. Maybe I miss something. But as my understanding,
for hole, there is no pte.
As the current code migrate_vma_collect_pmd()
{
if (pmd_none(*pmdp))
return migrate_vma_collect_hole(start, end, walk);
...
make_migration_entry()
}
We do not install migration entry for hole, then no need to remove
migration pte entry.
And on the driver side, there is way to migrate a hole. The driver just
skip it by
drivers/gpu/drm/nouveau/nouveau_dmem.c: if (!spage || !(src_pfns[i] & MIGRATE_PFN_MIGRATE))
^^^^
Finally, in migrate_vma_finalize(), for a hole,
if (!page) {
if (newpage) {
unlock_page(newpage);
put_page(newpage);
}
continue;
}
And we do not rely on remove_migration_ptes(page, newpage, false); to
restore the orignal pte (and it is impossible).
Thanks,
Pingfan
