Hi,

This is a part of luf(lazy unmap flush) patchset and also referred
several times by e.g. Shivank Garg and Zi Yan who are currently working
on page migration optimization.  Why don't we take this first so that
such a migration optimization tries can use it.

	Byungchul

--->8---
From a65a6e4975962707bf87171e317f005c6276887e Mon Sep 17 00:00:00 2001
From: Byungchul Park <byungchul@sk.com>
Date: Thu, 8 Aug 2024 15:53:58 +0900
Subject: [PATCH] mm: separate move/undo parts from migrate_pages_batch()

Functionally, no change.  This is a preparation for migration
optimization tries that require to use separated folio lists for its own
handling during migration.  Refactored migrate_pages_batch() so as to
separate move/undo parts from migrate_pages_batch().

Signed-off-by: Byungchul Park <byungchul@sk.com>
---
 mm/migrate.c | 134 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 83 insertions(+), 51 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index dfb5eba3c5223..c8f25434b9973 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1695,6 +1695,81 @@ static int migrate_hugetlbs(struct list_head *from, new_folio_t get_new_folio,
 	return nr_failed;
 }
 
+static void migrate_folios_move(struct list_head *src_folios,
+		struct list_head *dst_folios,
+		free_folio_t put_new_folio, unsigned long private,
+		enum migrate_mode mode, int reason,
+		struct list_head *ret_folios,
+		struct migrate_pages_stats *stats,
+		int *retry, int *thp_retry, int *nr_failed,
+		int *nr_retry_pages)
+{
+	struct folio *folio, *folio2, *dst, *dst2;
+	bool is_thp;
+	int nr_pages;
+	int rc;
+
+	dst = list_first_entry(dst_folios, struct folio, lru);
+	dst2 = list_next_entry(dst, lru);
+	list_for_each_entry_safe(folio, folio2, src_folios, lru) {
+		is_thp = folio_test_large(folio) && folio_test_pmd_mappable(folio);
+		nr_pages = folio_nr_pages(folio);
+
+		cond_resched();
+
+		rc = migrate_folio_move(put_new_folio, private,
+				folio, dst, mode,
+				reason, ret_folios);
+		/*
+		 * The rules are:
+		 *	Success: folio will be freed
+		 *	-EAGAIN: stay on the unmap_folios list
+		 *	Other errno: put on ret_folios list
+		 */
+		switch(rc) {
+		case -EAGAIN:
+			*retry += 1;
+			*thp_retry += is_thp;
+			*nr_retry_pages += nr_pages;
+			break;
+		case MIGRATEPAGE_SUCCESS:
+			stats->nr_succeeded += nr_pages;
+			stats->nr_thp_succeeded += is_thp;
+			break;
+		default:
+			*nr_failed += 1;
+			stats->nr_thp_failed += is_thp;
+			stats->nr_failed_pages += nr_pages;
+			break;
+		}
+		dst = dst2;
+		dst2 = list_next_entry(dst, lru);
+	}
+}
+
+static void migrate_folios_undo(struct list_head *src_folios,
+		struct list_head *dst_folios,
+		free_folio_t put_new_folio, unsigned long private,
+		struct list_head *ret_folios)
+{
+	struct folio *folio, *folio2, *dst, *dst2;
+
+	dst = list_first_entry(dst_folios, struct folio, lru);
+	dst2 = list_next_entry(dst, lru);
+	list_for_each_entry_safe(folio, folio2, src_folios, lru) {
+		int old_page_state = 0;
+		struct anon_vma *anon_vma = NULL;
+
+		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
+		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
+				anon_vma, true, ret_folios);
+		list_del(&dst->lru);
+		migrate_folio_undo_dst(dst, true, put_new_folio, private);
+		dst = dst2;
+		dst2 = list_next_entry(dst, lru);
+	}
+}
+
 /*
  * migrate_pages_batch() first unmaps folios in the from list as many as
  * possible, then move the unmapped folios.
@@ -1717,7 +1792,7 @@ static int migrate_pages_batch(struct list_head *from,
 	int pass = 0;
 	bool is_thp = false;
 	bool is_large = false;
-	struct folio *folio, *folio2, *dst = NULL, *dst2;
+	struct folio *folio, *folio2, *dst = NULL;
 	int rc, rc_saved = 0, nr_pages;
 	LIST_HEAD(unmap_folios);
 	LIST_HEAD(dst_folios);
@@ -1888,42 +1963,11 @@ static int migrate_pages_batch(struct list_head *from,
 		thp_retry = 0;
 		nr_retry_pages = 0;
 
-		dst = list_first_entry(&dst_folios, struct folio, lru);
-		dst2 = list_next_entry(dst, lru);
-		list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
-			is_thp = folio_test_large(folio) && folio_test_pmd_mappable(folio);
-			nr_pages = folio_nr_pages(folio);
-
-			cond_resched();
-
-			rc = migrate_folio_move(put_new_folio, private,
-						folio, dst, mode,
-						reason, ret_folios);
-			/*
-			 * The rules are:
-			 *	Success: folio will be freed
-			 *	-EAGAIN: stay on the unmap_folios list
-			 *	Other errno: put on ret_folios list
-			 */
-			switch(rc) {
-			case -EAGAIN:
-				retry++;
-				thp_retry += is_thp;
-				nr_retry_pages += nr_pages;
-				break;
-			case MIGRATEPAGE_SUCCESS:
-				stats->nr_succeeded += nr_pages;
-				stats->nr_thp_succeeded += is_thp;
-				break;
-			default:
-				nr_failed++;
-				stats->nr_thp_failed += is_thp;
-				stats->nr_failed_pages += nr_pages;
-				break;
-			}
-			dst = dst2;
-			dst2 = list_next_entry(dst, lru);
-		}
+		/* Move the unmapped folios */
+		migrate_folios_move(&unmap_folios, &dst_folios,
+				put_new_folio, private, mode, reason,
+				ret_folios, stats, &retry, &thp_retry,
+				&nr_failed, &nr_retry_pages);
 	}
 	nr_failed += retry;
 	stats->nr_thp_failed += thp_retry;
@@ -1932,20 +1976,8 @@ static int migrate_pages_batch(struct list_head *from,
 	rc = rc_saved ? : nr_failed;
 out:
 	/* Cleanup remaining folios */
-	dst = list_first_entry(&dst_folios, struct folio, lru);
-	dst2 = list_next_entry(dst, lru);
-	list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
-		int old_page_state = 0;
-		struct anon_vma *anon_vma = NULL;
-
-		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
-		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
-				       anon_vma, true, ret_folios);
-		list_del(&dst->lru);
-		migrate_folio_undo_dst(dst, true, put_new_folio, private);
-		dst = dst2;
-		dst2 = list_next_entry(dst, lru);
-	}
+	migrate_folios_undo(&unmap_folios, &dst_folios,
+			put_new_folio, private, ret_folios);
 
 	return rc;
 }
-- 
2.17.1

Hi Byungchul,

On 1/15/2025 3:23 PM, Byungchul Park wrote:
> Hi,
> 
> This is a part of luf(lazy unmap flush) patchset and also referred
> several times by e.g. Shivank Garg and Zi Yan who are currently working
> on page migration optimization.  Why don't we take this first so that
> such a migration optimization tries can use it.
> 
> 	Byungchul
> 
> --->8---
> From a65a6e4975962707bf87171e317f005c6276887e Mon Sep 17 00:00:00 2001
> From: Byungchul Park <byungchul@sk.com>
> Date: Thu, 8 Aug 2024 15:53:58 +0900
> Subject: [PATCH] mm: separate move/undo parts from migrate_pages_batch()
> 
> Functionally, no change.  This is a preparation for migration
> optimization tries that require to use separated folio lists for its own
> handling during migration.  Refactored migrate_pages_batch() so as to
> separate move/undo parts from migrate_pages_batch().
> 
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  mm/migrate.c | 134 +++++++++++++++++++++++++++++++--------------------
>  1 file changed, 83 insertions(+), 51 deletions(-)

[...]

> +		 */
> +		switch(rc) {
		     ^^^
minor nit here - space after 'switch' before the parentheses.
This code style issue was present in the original code and was carried
over during refactoring.

With this fix, please add-
Reviewed-by: Shivank Garg <shivankg@amd.com>
 
> +		case -EAGAIN:
> +			*retry += 1;
> +			*thp_retry += is_thp;
> +			*nr_retry_pages += nr_pages;
> +			break;
> +		case MIGRATEPAGE_SUCCESS:
> +			stats->nr_succeeded += nr_pages;
> +			stats->nr_thp_succeeded += is_thp;
> +			break;
> +		default:
> +			*nr_failed += 1;
> +			stats->nr_thp_failed += is_thp;
> +			stats->nr_failed_pages += nr_pages;
> +			break;
> +		}
> +		dst = dst2;
> +		dst2 = list_next_entry(dst, lru);
> +	}
> +}
> +
> +static void migrate_folios_undo(struct list_head *src_folios,
> +		struct list_head *dst_folios,
> +		free_folio_t put_new_folio, unsigned long private,
> +		struct list_head *ret_folios)
> +{
> +	struct folio *folio, *folio2, *dst, *dst2;
> +
> +	dst = list_first_entry(dst_folios, struct folio, lru);
> +	dst2 = list_next_entry(dst, lru);
> +	list_for_each_entry_safe(folio, folio2, src_folios, lru) {
> +		int old_page_state = 0;
> +		struct anon_vma *anon_vma = NULL;
> +
> +		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
> +		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
> +				anon_vma, true, ret_folios);
> +		list_del(&dst->lru);
> +		migrate_folio_undo_dst(dst, true, put_new_folio, private);
> +		dst = dst2;
> +		dst2 = list_next_entry(dst, lru);
> +	}
> +}
> +
>  /*
>   * migrate_pages_batch() first unmaps folios in the from list as many as
>   * possible, then move the unmapped folios.
> @@ -1717,7 +1792,7 @@ static int migrate_pages_batch(struct list_head *from,
>  	int pass = 0;
>  	bool is_thp = false;
>  	bool is_large = false;
> -	struct folio *folio, *folio2, *dst = NULL, *dst2;
> +	struct folio *folio, *folio2, *dst = NULL;
>  	int rc, rc_saved = 0, nr_pages;
>  	LIST_HEAD(unmap_folios);
>  	LIST_HEAD(dst_folios);
> @@ -1888,42 +1963,11 @@ static int migrate_pages_batch(struct list_head *from,
>  		thp_retry = 0;
>  		nr_retry_pages = 0;
>  
> -		dst = list_first_entry(&dst_folios, struct folio, lru);
> -		dst2 = list_next_entry(dst, lru);
> -		list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
> -			is_thp = folio_test_large(folio) && folio_test_pmd_mappable(folio);
> -			nr_pages = folio_nr_pages(folio);
> -
> -			cond_resched();
> -
> -			rc = migrate_folio_move(put_new_folio, private,
> -						folio, dst, mode,
> -						reason, ret_folios);
> -			/*
> -			 * The rules are:
> -			 *	Success: folio will be freed
> -			 *	-EAGAIN: stay on the unmap_folios list
> -			 *	Other errno: put on ret_folios list
> -			 */
> -			switch(rc) {
> -			case -EAGAIN:
> -				retry++;
> -				thp_retry += is_thp;
> -				nr_retry_pages += nr_pages;
> -				break;
> -			case MIGRATEPAGE_SUCCESS:
> -				stats->nr_succeeded += nr_pages;
> -				stats->nr_thp_succeeded += is_thp;
> -				break;
> -			default:
> -				nr_failed++;
> -				stats->nr_thp_failed += is_thp;
> -				stats->nr_failed_pages += nr_pages;
> -				break;
> -			}
> -			dst = dst2;
> -			dst2 = list_next_entry(dst, lru);
> -		}
> +		/* Move the unmapped folios */
> +		migrate_folios_move(&unmap_folios, &dst_folios,
> +				put_new_folio, private, mode, reason,
> +				ret_folios, stats, &retry, &thp_retry,
> +				&nr_failed, &nr_retry_pages);
>  	}
>  	nr_failed += retry;
>  	stats->nr_thp_failed += thp_retry;
> @@ -1932,20 +1976,8 @@ static int migrate_pages_batch(struct list_head *from,
>  	rc = rc_saved ? : nr_failed;
>  out:
>  	/* Cleanup remaining folios */
> -	dst = list_first_entry(&dst_folios, struct folio, lru);
> -	dst2 = list_next_entry(dst, lru);
> -	list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
> -		int old_page_state = 0;
> -		struct anon_vma *anon_vma = NULL;
> -
> -		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
> -		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
> -				       anon_vma, true, ret_folios);
> -		list_del(&dst->lru);
> -		migrate_folio_undo_dst(dst, true, put_new_folio, private);
> -		dst = dst2;
> -		dst2 = list_next_entry(dst, lru);
> -	}
> +	migrate_folios_undo(&unmap_folios, &dst_folios,
> +			put_new_folio, private, ret_folios);
>  
>  	return rc;
>  }

On Wed, Jan 15, 2025 at 03:37:15PM +0530, Shivank Garg wrote:
> Hi Byungchul,
> 
> On 1/15/2025 3:23 PM, Byungchul Park wrote:
> > Hi,
> > 
> > This is a part of luf(lazy unmap flush) patchset and also referred
> > several times by e.g. Shivank Garg and Zi Yan who are currently working
> > on page migration optimization.  Why don't we take this first so that
> > such a migration optimization tries can use it.
> > 
> > 	Byungchul
> > 
> > --->8---
> > From a65a6e4975962707bf87171e317f005c6276887e Mon Sep 17 00:00:00 2001
> > From: Byungchul Park <byungchul@sk.com>
> > Date: Thu, 8 Aug 2024 15:53:58 +0900
> > Subject: [PATCH] mm: separate move/undo parts from migrate_pages_batch()
> > 
> > Functionally, no change.  This is a preparation for migration
> > optimization tries that require to use separated folio lists for its own
> > handling during migration.  Refactored migrate_pages_batch() so as to
> > separate move/undo parts from migrate_pages_batch().
> > 
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> >  mm/migrate.c | 134 +++++++++++++++++++++++++++++++--------------------
> >  1 file changed, 83 insertions(+), 51 deletions(-)
> 
> [...]
> 
> > +		 */
> > +		switch(rc) {
> 		     ^^^
> minor nit here - space after 'switch' before the parentheses.
> This code style issue was present in the original code and was carried
> over during refactoring.

I should've taken more care of it.  Thank you.

	Byungchul

> With this fix, please add-
> Reviewed-by: Shivank Garg <shivankg@amd.com>
>  
> > +		case -EAGAIN:
> > +			*retry += 1;
> > +			*thp_retry += is_thp;
> > +			*nr_retry_pages += nr_pages;
> > +			break;
> > +		case MIGRATEPAGE_SUCCESS:
> > +			stats->nr_succeeded += nr_pages;
> > +			stats->nr_thp_succeeded += is_thp;
> > +			break;
> > +		default:
> > +			*nr_failed += 1;
> > +			stats->nr_thp_failed += is_thp;
> > +			stats->nr_failed_pages += nr_pages;
> > +			break;
> > +		}
> > +		dst = dst2;
> > +		dst2 = list_next_entry(dst, lru);
> > +	}
> > +}
> > +
> > +static void migrate_folios_undo(struct list_head *src_folios,
> > +		struct list_head *dst_folios,
> > +		free_folio_t put_new_folio, unsigned long private,
> > +		struct list_head *ret_folios)
> > +{
> > +	struct folio *folio, *folio2, *dst, *dst2;
> > +
> > +	dst = list_first_entry(dst_folios, struct folio, lru);
> > +	dst2 = list_next_entry(dst, lru);
> > +	list_for_each_entry_safe(folio, folio2, src_folios, lru) {
> > +		int old_page_state = 0;
> > +		struct anon_vma *anon_vma = NULL;
> > +
> > +		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
> > +		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
> > +				anon_vma, true, ret_folios);
> > +		list_del(&dst->lru);
> > +		migrate_folio_undo_dst(dst, true, put_new_folio, private);
> > +		dst = dst2;
> > +		dst2 = list_next_entry(dst, lru);
> > +	}
> > +}
> > +
> >  /*
> >   * migrate_pages_batch() first unmaps folios in the from list as many as
> >   * possible, then move the unmapped folios.
> > @@ -1717,7 +1792,7 @@ static int migrate_pages_batch(struct list_head *from,
> >  	int pass = 0;
> >  	bool is_thp = false;
> >  	bool is_large = false;
> > -	struct folio *folio, *folio2, *dst = NULL, *dst2;
> > +	struct folio *folio, *folio2, *dst = NULL;
> >  	int rc, rc_saved = 0, nr_pages;
> >  	LIST_HEAD(unmap_folios);
> >  	LIST_HEAD(dst_folios);
> > @@ -1888,42 +1963,11 @@ static int migrate_pages_batch(struct list_head *from,
> >  		thp_retry = 0;
> >  		nr_retry_pages = 0;
> >  
> > -		dst = list_first_entry(&dst_folios, struct folio, lru);
> > -		dst2 = list_next_entry(dst, lru);
> > -		list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
> > -			is_thp = folio_test_large(folio) && folio_test_pmd_mappable(folio);
> > -			nr_pages = folio_nr_pages(folio);
> > -
> > -			cond_resched();
> > -
> > -			rc = migrate_folio_move(put_new_folio, private,
> > -						folio, dst, mode,
> > -						reason, ret_folios);
> > -			/*
> > -			 * The rules are:
> > -			 *	Success: folio will be freed
> > -			 *	-EAGAIN: stay on the unmap_folios list
> > -			 *	Other errno: put on ret_folios list
> > -			 */
> > -			switch(rc) {
> > -			case -EAGAIN:
> > -				retry++;
> > -				thp_retry += is_thp;
> > -				nr_retry_pages += nr_pages;
> > -				break;
> > -			case MIGRATEPAGE_SUCCESS:
> > -				stats->nr_succeeded += nr_pages;
> > -				stats->nr_thp_succeeded += is_thp;
> > -				break;
> > -			default:
> > -				nr_failed++;
> > -				stats->nr_thp_failed += is_thp;
> > -				stats->nr_failed_pages += nr_pages;
> > -				break;
> > -			}
> > -			dst = dst2;
> > -			dst2 = list_next_entry(dst, lru);
> > -		}
> > +		/* Move the unmapped folios */
> > +		migrate_folios_move(&unmap_folios, &dst_folios,
> > +				put_new_folio, private, mode, reason,
> > +				ret_folios, stats, &retry, &thp_retry,
> > +				&nr_failed, &nr_retry_pages);
> >  	}
> >  	nr_failed += retry;
> >  	stats->nr_thp_failed += thp_retry;
> > @@ -1932,20 +1976,8 @@ static int migrate_pages_batch(struct list_head *from,
> >  	rc = rc_saved ? : nr_failed;
> >  out:
> >  	/* Cleanup remaining folios */
> > -	dst = list_first_entry(&dst_folios, struct folio, lru);
> > -	dst2 = list_next_entry(dst, lru);
> > -	list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
> > -		int old_page_state = 0;
> > -		struct anon_vma *anon_vma = NULL;
> > -
> > -		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
> > -		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
> > -				       anon_vma, true, ret_folios);
> > -		list_del(&dst->lru);
> > -		migrate_folio_undo_dst(dst, true, put_new_folio, private);
> > -		dst = dst2;
> > -		dst2 = list_next_entry(dst, lru);
> > -	}
> > +	migrate_folios_undo(&unmap_folios, &dst_folios,
> > +			put_new_folio, private, ret_folios);
> >  
> >  	return rc;
> >  }

Changes from v1
	- Fix a wrong coding style
	
--->8---
From a65a6e4975962707bf87171e317f005c6276887e Mon Sep 17 00:00:00 2001
From: Byungchul Park <byungchul@sk.com>
Date: Thu, 8 Aug 2024 15:53:58 +0900
Subject: [PATCH v2] mm: separate move/undo parts from migrate_pages_batch()

Functionally, no change.  This is a preparation for luf mechanism that
requires to use separated folio lists for its own handling during
migration.  Refactored migrate_pages_batch() so as to separate move/undo
parts from migrate_pages_batch().

Signed-off-by: Byungchul Park <byungchul@sk.com>
Reviewed-by: Shivank Garg <shivankg@amd.com>
---
 mm/migrate.c | 134 +++++++++++++++++++++++++++++++--------------------
 1 file changed, 83 insertions(+), 51 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index dfb5eba3c5223..c8f25434b9973 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1695,6 +1695,81 @@ static int migrate_hugetlbs(struct list_head *from, new_folio_t get_new_folio,
 	return nr_failed;
 }
 
+static void migrate_folios_move(struct list_head *src_folios,
+		struct list_head *dst_folios,
+		free_folio_t put_new_folio, unsigned long private,
+		enum migrate_mode mode, int reason,
+		struct list_head *ret_folios,
+		struct migrate_pages_stats *stats,
+		int *retry, int *thp_retry, int *nr_failed,
+		int *nr_retry_pages)
+{
+	struct folio *folio, *folio2, *dst, *dst2;
+	bool is_thp;
+	int nr_pages;
+	int rc;
+
+	dst = list_first_entry(dst_folios, struct folio, lru);
+	dst2 = list_next_entry(dst, lru);
+	list_for_each_entry_safe(folio, folio2, src_folios, lru) {
+		is_thp = folio_test_large(folio) && folio_test_pmd_mappable(folio);
+		nr_pages = folio_nr_pages(folio);
+
+		cond_resched();
+
+		rc = migrate_folio_move(put_new_folio, private,
+				folio, dst, mode,
+				reason, ret_folios);
+		/*
+		 * The rules are:
+		 *	Success: folio will be freed
+		 *	-EAGAIN: stay on the unmap_folios list
+		 *	Other errno: put on ret_folios list
+		 */
+		switch (rc) {
+		case -EAGAIN:
+			*retry += 1;
+			*thp_retry += is_thp;
+			*nr_retry_pages += nr_pages;
+			break;
+		case MIGRATEPAGE_SUCCESS:
+			stats->nr_succeeded += nr_pages;
+			stats->nr_thp_succeeded += is_thp;
+			break;
+		default:
+			*nr_failed += 1;
+			stats->nr_thp_failed += is_thp;
+			stats->nr_failed_pages += nr_pages;
+			break;
+		}
+		dst = dst2;
+		dst2 = list_next_entry(dst, lru);
+	}
+}
+
+static void migrate_folios_undo(struct list_head *src_folios,
+		struct list_head *dst_folios,
+		free_folio_t put_new_folio, unsigned long private,
+		struct list_head *ret_folios)
+{
+	struct folio *folio, *folio2, *dst, *dst2;
+
+	dst = list_first_entry(dst_folios, struct folio, lru);
+	dst2 = list_next_entry(dst, lru);
+	list_for_each_entry_safe(folio, folio2, src_folios, lru) {
+		int old_page_state = 0;
+		struct anon_vma *anon_vma = NULL;
+
+		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
+		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
+				anon_vma, true, ret_folios);
+		list_del(&dst->lru);
+		migrate_folio_undo_dst(dst, true, put_new_folio, private);
+		dst = dst2;
+		dst2 = list_next_entry(dst, lru);
+	}
+}
+
 /*
  * migrate_pages_batch() first unmaps folios in the from list as many as
  * possible, then move the unmapped folios.
@@ -1717,7 +1792,7 @@ static int migrate_pages_batch(struct list_head *from,
 	int pass = 0;
 	bool is_thp = false;
 	bool is_large = false;
-	struct folio *folio, *folio2, *dst = NULL, *dst2;
+	struct folio *folio, *folio2, *dst = NULL;
 	int rc, rc_saved = 0, nr_pages;
 	LIST_HEAD(unmap_folios);
 	LIST_HEAD(dst_folios);
@@ -1888,42 +1963,11 @@ static int migrate_pages_batch(struct list_head *from,
 		thp_retry = 0;
 		nr_retry_pages = 0;
 
-		dst = list_first_entry(&dst_folios, struct folio, lru);
-		dst2 = list_next_entry(dst, lru);
-		list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
-			is_thp = folio_test_large(folio) && folio_test_pmd_mappable(folio);
-			nr_pages = folio_nr_pages(folio);
-
-			cond_resched();
-
-			rc = migrate_folio_move(put_new_folio, private,
-						folio, dst, mode,
-						reason, ret_folios);
-			/*
-			 * The rules are:
-			 *	Success: folio will be freed
-			 *	-EAGAIN: stay on the unmap_folios list
-			 *	Other errno: put on ret_folios list
-			 */
-			switch(rc) {
-			case -EAGAIN:
-				retry++;
-				thp_retry += is_thp;
-				nr_retry_pages += nr_pages;
-				break;
-			case MIGRATEPAGE_SUCCESS:
-				stats->nr_succeeded += nr_pages;
-				stats->nr_thp_succeeded += is_thp;
-				break;
-			default:
-				nr_failed++;
-				stats->nr_thp_failed += is_thp;
-				stats->nr_failed_pages += nr_pages;
-				break;
-			}
-			dst = dst2;
-			dst2 = list_next_entry(dst, lru);
-		}
+		/* Move the unmapped folios */
+		migrate_folios_move(&unmap_folios, &dst_folios,
+				put_new_folio, private, mode, reason,
+				ret_folios, stats, &retry, &thp_retry,
+				&nr_failed, &nr_retry_pages);
 	}
 	nr_failed += retry;
 	stats->nr_thp_failed += thp_retry;
@@ -1932,20 +1976,8 @@ static int migrate_pages_batch(struct list_head *from,
 	rc = rc_saved ? : nr_failed;
 out:
 	/* Cleanup remaining folios */
-	dst = list_first_entry(&dst_folios, struct folio, lru);
-	dst2 = list_next_entry(dst, lru);
-	list_for_each_entry_safe(folio, folio2, &unmap_folios, lru) {
-		int old_page_state = 0;
-		struct anon_vma *anon_vma = NULL;
-
-		__migrate_folio_extract(dst, &old_page_state, &anon_vma);
-		migrate_folio_undo_src(folio, old_page_state & PAGE_WAS_MAPPED,
-				       anon_vma, true, ret_folios);
-		list_del(&dst->lru);
-		migrate_folio_undo_dst(dst, true, put_new_folio, private);
-		dst = dst2;
-		dst2 = list_next_entry(dst, lru);
-	}
+	migrate_folios_undo(&unmap_folios, &dst_folios,
+			put_new_folio, private, ret_folios);
 
 	return rc;
 }
-- 
2.17.1

On 15 Jan 2025, at 5:34, Byungchul Park wrote:

> Changes from v1
> 	- Fix a wrong coding style
> 	
> --->8---
> From a65a6e4975962707bf87171e317f005c6276887e Mon Sep 17 00:00:00 2001
> From: Byungchul Park <byungchul@sk.com>
> Date: Thu, 8 Aug 2024 15:53:58 +0900
> Subject: [PATCH v2] mm: separate move/undo parts from migrate_pages_batch()
>
> Functionally, no change.  This is a preparation for luf mechanism that
> requires to use separated folio lists for its own handling during
> migration.  Refactored migrate_pages_batch() so as to separate move/undo
> parts from migrate_pages_batch().
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> Reviewed-by: Shivank Garg <shivankg@amd.com>
> ---
>  mm/migrate.c | 134 +++++++++++++++++++++++++++++++--------------------
>  1 file changed, 83 insertions(+), 51 deletions(-)
>
LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>

I am planning to send the batching page copy part in my page migration
acceleration patchset separately for upstream first, so that we can
focus on the page copy acceleration, using multithreads and DMA
afterwards.

Best Regards,
Yan, Zi