Re: [PATCH 4/7] mm/migrate: add batch-copy path in migrate_pages_batch

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

On 5/11/2026 9:10 PM, David Hildenbrand (Arm) wrote:
> On 4/28/26 17:50, Shivank Garg wrote:
>> Add folios_mc_copy() which walks list of src and dst folios in lockstep,
>> and copies folio content via folio_mc_copy(). folios_cnt parameter is
>> unused here, but is part of the offload_copy callback signature used by
>> later patches in the series.
>>
>> Split unmapped folios into batch-eligible (unmap_batch/dst_batch) and
>> standard (unmap_single/dst_single) lists, gated by the
>> migrate_offload_enabled which is off by default. So, when no offload
>> driver is active, the branch is never taken and everything goes
>> through the standard path.
>>
>> After TLB flush, batch copy the eligible folios via folios_mc_copy()
>> and pass already_copied=true into migrate_folios_move() so
>> __migrate_folio() skips the per-folio copy.
>>
>> On batch copy failure, already_copied flag stays false and each folio
>> fall back to individual copy.
>>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>>  include/linux/mm.h |  2 ++
>>  mm/migrate.c       | 61 +++++++++++++++++++++++++++++++++++-----------
>>  mm/util.c          | 30 +++++++++++++++++++++++
>>  3 files changed, 79 insertions(+), 14 deletions(-)
> 
> [...]
> 
>>  
>> +DEFINE_STATIC_KEY_FALSE(migrate_offload_enabled);
>> +
>>  static const struct movable_operations *offline_movable_ops;
>>  static const struct movable_operations *zsmalloc_movable_ops;
>>  
>> @@ -1724,6 +1727,12 @@ static int migrate_hugetlbs(struct list_head *from, new_folio_t get_new_folio,
>>  	return nr_failed;
>>  }
>>  
>> +/* movable_ops folios have their own migrate path */
>> +static bool folio_supports_batch_copy(struct folio *folio)
>> +{
>> +	return likely(!page_has_movable_ops(&folio->page));
>> +}
> 
> As these things are not actually folios (and callers will have to be taught to
> distinguish them way, way earlier), I guess you should make this
> 
> /* movable_ops pages have a separate migration path */
> static bool page_supports_batch_copy(struct page *page)
> ...

I'll change this.

> 
>> +
>>  static void migrate_folios_move(struct list_head *src_folios,
>>  		struct list_head *dst_folios,
>>  		free_folio_t put_new_folio, unsigned long private,
>> @@ -1752,7 +1761,7 @@ static void migrate_folios_move(struct list_head *src_folios,
>>  		/*
>>  		 * The rules are:
>>  		 *	0: folio will be freed
>> -		 *	-EAGAIN: stay on the unmap_folios list
>> +		 *	-EAGAIN: stay on the src_folios list
>>  		 *	Other errno: put on ret_folios list
>>  		 */
>>  		switch (rc) {
> 
> 
> [...]
> 
>> --- a/mm/util.c
>> +++ b/mm/util.c
>> @@ -778,6 +778,36 @@ int folio_mc_copy(struct folio *dst, struct folio *src)
>>  }
>>  EXPORT_SYMBOL(folio_mc_copy);
>>  
>> +/**
>> + * folios_mc_copy - Copy the contents of list of folios.
>> + * @dst_list: destination folio list.
>> + * @src_list: source folio list.
>> + * @folios_cnt: unused here, present for callback signature compatibility.
>> + *
>> + * Walks list of src and dst folios in lockstep and copies folio
>> + * content via folio_mc_copy(). The caller must ensure both lists have
>> + * the same number of entries. This may sleep.
> 
> This *function*
> 

Will add function.

>> + *
>> + * Return: 0 on success, negative errno on failure.
>> + */
>> +int folios_mc_copy(struct list_head *dst_list, struct list_head *src_list,
>> +		unsigned int __always_unused folios_cnt)
>> +{
>> +	struct folio *src, *dst;
>> +	int ret;
>> +
>> +	dst = list_first_entry(dst_list, struct folio, lru);
>> +	list_for_each_entry(src, src_list, lru) {
>> +		ret = folio_mc_copy(dst, src);
>> +		if (ret)
>> +			return ret;
>> +		dst = list_next_entry(dst, lru);
>> +	}
> 
> Wouldn't it be cleaner to remember "already copied" immediately after we ...
> performed the copy? (succeeded with folio_mc_copy)

IIUC, you mean something like:
	list_for_each_entry(src, src_list, lru) {
		ret = folio_mc_copy(dst, src);
		if (ret)
			return ret;
		dst->migrate_info |= FOLIO_CONTENT_COPIED;
		dst = list_next_entry(dst, lru);
	}

This will be cleaner. However, it will make the folios_mc_copy() specific for
page migration and can't be reused by other potential callers.
And offload driver (like DCBM) must set the already copied marker
themselves for successful copies.
This will have an upside that on partial-success (even a single failed copy),
it will no longer forces re-copy of the whole batch.
If that's the intention, I'll move this to migrate.c itself.

Thanks,
Shivank