Re: [PATCH 3/7] mm/migrate: skip data copy for already-copied folios

["Garg, Shivank" <shivankg@amd.com>]

On 5/11/2026 9:05 PM, David Hildenbrand (Arm) wrote:
> On 4/28/26 17:50, Shivank Garg wrote:
>> Add a FOLIO_ALREADY_COPIED flag to the dst->migrate_info migration
>> state. When set, __migrate_folio() skips folio_mc_copy() and
>> performs metadata-only migration. All callers currently pass
>> already_copied=false. The batch-copy path enables it later in a
>> subsequent patch.
>>
>> Move the dst->migrate_info state enum earlier in the file so
>> __migrate_folio() and move_to_new_folio() can see FOLIO_ALREADY_COPIED.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  mm/migrate.c | 53 +++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 32 insertions(+), 21 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 03c2a6f7e5e4..c493e67e359d 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -850,6 +850,19 @@ void folio_migrate_flags(struct folio *newfolio, struct folio *folio)
>>  }
>>  EXPORT_SYMBOL(folio_migrate_flags);
>>  
>> +/*
>> + * To record some information during migration, we use the migrate_info
>> + * field of struct folio of the newly allocated destination folio.
>> + * This is safe because nobody is using it except us.
>> + */
>> +enum {
>> +	FOLIO_WAS_MAPPED = BIT(0),
>> +	FOLIO_WAS_MLOCKED = BIT(1),
>> +	FOLIO_ALREADY_COPIED = BIT(2),
> 
> I wonder whether we want to talk about "folio content copied", to not confuse it
> with folio flags copied etc.
> 
> FOLIO_CONTENT_COPIED.
> 
> Thoughts?

Good point, this is more precise.

>> +	FOLIO_OLD_STATES = FOLIO_WAS_MAPPED | FOLIO_WAS_MLOCKED |
>> +			   FOLIO_ALREADY_COPIED,
>> +};
>> +
>>  /************************************************************
>>   *                    Migration functions
>>   ***********************************************************/
>> @@ -859,14 +872,20 @@ static int __migrate_folio(struct address_space *mapping, struct folio *dst,
>>  			   enum migrate_mode mode)
>>  {
>>  	int rc, expected_count = folio_expected_ref_count(src) + 1;
>> +	bool already_copied = (dst->migrate_info & FOLIO_ALREADY_COPIED);
> 
> const, and no need for ().

Will fix.

>> +
>> +	if (already_copied)
>> +		dst->migrate_info = 0;
> 
> Hm, why is that required? Might deserve a comment.
> 
> Likely you want to clear the "already copied" marker?
> 
> 	dst->migrate_info &= ~FOLIO_ALREADY_COPIED;
> 
> ?

Yes,
In current code, migrate_info only carry FOLIO_ALREADY_COPIED.
So, =0 and &=~ are equivalent and similar for |= case. I mirrored the
how anon_vma/old_folio_state get handled __migrate_folio_extract().

But with your other suggestion of marking already_copied immediately
after folio_mc_copy(), then bitwise clear and set will be required.
I'll do this.



> But I wonder if this really belongs exactly here.
>

I did not understand this.

 
>>  
>>  	/* Check whether src does not have extra refs before we do more work */
>>  	if (folio_ref_count(src) != expected_count)
>>  		return -EAGAIN;
>>  
>> -	rc = folio_mc_copy(dst, src);
>> -	if (unlikely(rc))
>> -		return rc;
>> +	if (!already_copied) {
>> +		rc = folio_mc_copy(dst, src);
>> +		if (unlikely(rc))
>> +			return rc;
>> +	}
>>  
>>  	rc = __folio_migrate_mapping(mapping, dst, src, expected_count);
>>  	if (rc)
>> @@ -1090,7 +1109,7 @@ static int fallback_migrate_folio(struct address_space *mapping,
>>   *     0 - success
>>   */
>>  static int move_to_new_folio(struct folio *dst, struct folio *src,
>> -				enum migrate_mode mode)
>> +		enum migrate_mode mode, bool already_copied)
>>  {
>>  	struct address_space *mapping = folio_mapping(src);
>>  	int rc = -EAGAIN;
>> @@ -1098,6 +1117,9 @@ static int move_to_new_folio(struct folio *dst, struct folio *src,
>>  	VM_BUG_ON_FOLIO(!folio_test_locked(src), src);
>>  	VM_BUG_ON_FOLIO(!folio_test_locked(dst), dst);
>>  
>> +	if (already_copied)
>> +		dst->migrate_info = FOLIO_ALREADY_COPIED;
> 
> |= ?
>

Will do.
 
>> +
>>  	if (!mapping)
>>  		rc = migrate_folio(mapping, dst, src, mode);
>>  	else if (mapping_inaccessible(mapping))
>> @@ -1129,17 +1151,6 @@ static int move_to_new_folio(struct folio *dst, struct folio *src,
>>  	return rc;
>>  }
>>  
>> -/*
>> - * To record some information during migration, we use the migrate_info
>> - * field of struct folio of the newly allocated destination folio.
>> - * This is safe because nobody is using it except us.
>> - */
>> -enum {
>> -	FOLIO_WAS_MAPPED = BIT(0),
>> -	FOLIO_WAS_MLOCKED = BIT(1),
>> -	FOLIO_OLD_STATES = FOLIO_WAS_MAPPED | FOLIO_WAS_MLOCKED,
>> -};
>> -
>>  static void __migrate_folio_record(struct folio *dst,
>>  		int old_folio_state, struct anon_vma *anon_vma)
>>  {
>> @@ -1353,7 +1364,7 @@ static int migrate_folio_unmap(new_folio_t get_new_folio,
>>  static int migrate_folio_move(free_folio_t put_new_folio, unsigned long private,
>>  			      struct folio *src, struct folio *dst,
>>  			      enum migrate_mode mode, enum migrate_reason reason,
>> -			      struct list_head *ret)
>> +			      struct list_head *ret, bool already_copied)
>>  {
>>  	int rc;
>>  	int old_folio_state = 0;
>> @@ -1379,7 +1390,7 @@ static int migrate_folio_move(free_folio_t put_new_folio, unsigned long private,
>>  		src_partially_mapped = folio_test_partially_mapped(src);
>>  	}
>>  
>> -	rc = move_to_new_folio(dst, src, mode);
>> +	rc = move_to_new_folio(dst, src, mode, already_copied);
>>  	if (rc)
>>  		goto out;
>>  
>> @@ -1536,7 +1547,7 @@ static int unmap_and_move_huge_page(new_folio_t get_new_folio,
>>  	}
>>  
>>  	if (!folio_mapped(src))
>> -		rc = move_to_new_folio(dst, src, mode);
>> +		rc = move_to_new_folio(dst, src, mode, false);
> 
> ... mode, /* already_copied = */ false
> 

will fix

>>  
>>  	if (page_was_mapped)
>>  		remove_migration_ptes(src, !rc ? dst : src, ttu);
>> @@ -1720,7 +1731,7 @@ static void migrate_folios_move(struct list_head *src_folios,
>>  		struct list_head *ret_folios,
>>  		struct migrate_pages_stats *stats,
>>  		int *retry, int *thp_retry, int *nr_failed,
>> -		int *nr_retry_pages)
>> +		int *nr_retry_pages, bool already_copied)
>>  {
>>  	struct folio *folio, *folio2, *dst, *dst2;
>>  	bool is_thp;
>> @@ -1737,7 +1748,7 @@ static void migrate_folios_move(struct list_head *src_folios,
>>  
>>  		rc = migrate_folio_move(put_new_folio, private,
>>  				folio, dst, mode,
>> -				reason, ret_folios);
>> +				reason, ret_folios, already_copied);
>>  		/*
>>  		 * The rules are:
>>  		 *	0: folio will be freed
>> @@ -1994,7 +2005,7 @@ static int migrate_pages_batch(struct list_head *from,
>>  		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, &nr_retry_pages, false);
>>  	}
> 
> dito.
>

will fix.

Thanks,
Shivank