[PATCH v3 0/4] Some cleanup for page migration

[PATCH v3 0/4] Some cleanup for page migration

[Baolin Wang]

Hi,

This patch set did some cleanup and improvements for the page migration,
please help to review. Thanks a lot.

Changes from v2:
 - Simplify the page refcount validation suggested by Matthew.

Changes from v1:
 - Add reviewed-by tags.
 - Add more comments for patch 1.
 - Drop unnecessary patch 5 from this patch set.

Baolin Wang (4):
  mm: migrate: Simplify the file-backed pages validation when migrating
    its mapping
  mm: migrate: Introduce a local variable to get the number of pages
  mm: migrate: Fix the incorrect function name in comments
  mm: migrate: Change to use bool type for 'page_was_mapped'

 mm/migrate.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

-- 
1.8.3.1

[PATCH v3 1/4] mm: migrate: Simplify the file-backed pages validation when migrating its mapping

[Baolin Wang]

There is no need to validate the file-backed page's refcount before
trying to freeze the page's expected refcount, instead we can rely on
the folio_ref_freeze() to validate if the page has the expected refcount
before migrating its mapping.

Moreover we are always under the page lock when migrating the page
mapping, which means nowhere else can remove it from the page cache,
so we can remove the xas_load() validation under the i_pages lock.

Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/migrate.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index c6eb2a8..4c93d98 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -404,12 +404,6 @@ int folio_migrate_mapping(struct address_space *mapping,
 	newzone = folio_zone(newfolio);
 
 	xas_lock_irq(&xas);
-	if (folio_ref_count(folio) != expected_count ||
-	    xas_load(&xas) != folio) {
-		xas_unlock_irq(&xas);
-		return -EAGAIN;
-	}
-
 	if (!folio_ref_freeze(folio, expected_count)) {
 		xas_unlock_irq(&xas);
 		return -EAGAIN;
-- 
1.8.3.1

[PATCH v3 2/4] mm: migrate: Introduce a local variable to get the number of pages

[Baolin Wang]

Use thp_nr_pages() instead of compound_nr() to get the number of pages
for THP page, meanwhile introducing a local variable 'nr_pages' to
avoid getting the number of pages repeatedly.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
Reviewed-by: Yang Shi <shy828301@gmail.com>
---
 mm/migrate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 4c93d98..9520d2f 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2102,6 +2102,7 @@ static struct page *alloc_misplaced_dst_page_thp(struct page *page,
 static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
 {
 	int page_lru;
+	int nr_pages = thp_nr_pages(page);
 
 	VM_BUG_ON_PAGE(compound_order(page) && !PageTransHuge(page), page);
 
@@ -2110,7 +2111,7 @@ static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
 		return 0;
 
 	/* Avoid migrating to a node that is nearly full */
-	if (!migrate_balanced_pgdat(pgdat, compound_nr(page)))
+	if (!migrate_balanced_pgdat(pgdat, nr_pages))
 		return 0;
 
 	if (isolate_lru_page(page))
@@ -2118,7 +2119,7 @@ static int numamigrate_isolate_page(pg_data_t *pgdat, struct page *page)
 
 	page_lru = page_is_file_lru(page);
 	mod_node_page_state(page_pgdat(page), NR_ISOLATED_ANON + page_lru,
-				thp_nr_pages(page));
+			    nr_pages);
 
 	/*
 	 * Isolating the page has taken another reference, so the
-- 
1.8.3.1

[PATCH v3 3/4] mm: migrate: Fix the incorrect function name in comments

[Baolin Wang]

since commit a98a2f0c8ce1 ("mm/rmap: split migration into its own function"),
the migration ptes establishment has been split into a separate
try_to_migrate() function, thus update the related comments.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
Reviewed-by: Yang Shi <shy828301@gmail.com>
Reviewed-by: Alistair Popple <apopple@nvidia.com>
---
 mm/migrate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 9520d2f..a9800fa 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1001,7 +1001,7 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 	}
 
 	/*
-	 * By try_to_unmap(), page->mapcount goes down to 0 here. In this case,
+	 * By try_to_migrate(), page->mapcount goes down to 0 here. In this case,
 	 * we cannot notice that anon_vma is freed while we migrates a page.
 	 * This get_anon_vma() delays freeing anon_vma pointer until the end
 	 * of migration. File cache pages are no problem because of page_lock()
-- 
1.8.3.1

[PATCH v3 4/4] mm: migrate: Change to use bool type for 'page_was_mapped'

[Baolin Wang]

Change to use bool type for 'page_was_mapped' variable making it
more readable.

Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
Reviewed-by: Yang Shi <shy828301@gmail.com>
---
 mm/migrate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index a9800fa..04a76fc 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -953,7 +953,7 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 				int force, enum migrate_mode mode)
 {
 	int rc = -EAGAIN;
-	int page_was_mapped = 0;
+	bool page_was_mapped = false;
 	struct anon_vma *anon_vma = NULL;
 	bool is_lru = !__PageMovable(page);
 
@@ -1056,7 +1056,7 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 		VM_BUG_ON_PAGE(PageAnon(page) && !PageKsm(page) && !anon_vma,
 				page);
 		try_to_migrate(page, 0);
-		page_was_mapped = 1;
+		page_was_mapped = true;
 	}
 
 	if (!page_mapped(page))
-- 
1.8.3.1