Re: [PATCH 2/3] mm/migrate: see hole as invalid source page

[Pingfan Liu]

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