Re: [PATCH 2/4] mm: migrate: convert numamigrate_isolate_page() to numamigrate_isolate_folio()

From: Kefeng Wang
Date: Thu Aug 03 2023 - 03:08:26 EST

On 2023/8/2 20:30, Matthew Wilcox wrote:
On Wed, Aug 02, 2023 at 05:53:44PM +0800, Kefeng Wang wrote:
-static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
+static int numamigrate_isolate_folio(pg_data_t *pgdat, struct folio *folio)
- int nr_pages = thp_nr_pages(page);
- int order = compound_order(page);
+ int nr_pages = folio_nr_pages(folio);
+ int order = folio_order(folio);
- VM_BUG_ON_PAGE(order && !PageTransHuge(page), page);
+ VM_BUG_ON_FOLIO(order && !folio_test_pmd_mappable(folio), folio);

I don't know why we have this assertion. I would be inclined to delete
it as part of generalising the migration code to handle arbitrary sizes
of folio, rather than assert that we only support PMD size folios.

Ok, will drop it.

/* Do not migrate THP mapped by multiple processes */
- if (PageTransHuge(page) && total_mapcount(page) > 1)
+ if (folio_test_pmd_mappable(folio) && folio_estimated_sharers(folio) > 1)
return 0;

I don't know if this is the right logic. We've willing to move folios
mapped by multiple processes, as long as they're smaller than PMD size,
but once they get to PMD size they're magical and can't be moved?

It seems that the logical is introduced by commit 04fa5d6a6547 ("mm:
migrate: check page_count of THP before migrating") and refactor by
340ef3902cf2 ("mm: numa: cleanup flow of transhuge page migration"),

"Hugh Dickins pointed out that migrate_misplaced_transhuge_page() does
not check page_count before migrating like base page migration and
khugepage. He could not see why this was safe and he is right."

For now, there is no migrate_misplaced_transhuge_page() and base/thp
page migrate's path is unified, there is a check(for old/new kernel) in migrate_misplaced_page(),

"Don't migrate file pages that are mapped in multiple processes
with execute permissions as they are probably shared libraries."

We could drop the above check in numamigrate_isolate_page(), but
according to 04fa5d6a6547, maybe disable migrate page shared by
multi-process during numa balance for both base/thp page.