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

[Garg, Shivank]

On 6/8/2026 9:06 PM, Zi Yan wrote:
> On 8 Jun 2026, at 11:25, David Hildenbrand (Arm) wrote:
> 
>> On 5/20/26 17:06, Garg, Shivank wrote:
>>>
>>>
>>> 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@xxxxxxx>
>>>>> ---
>>>>>  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.
>>
>> Right, maybe some helper for that would be possible?
>>

I'm adding a migrate.c specific implementation: migrate_folios_mc_copy().

>>> 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.
>>
>> True. I'd probably have to take a look at the end result to see if it looks
>> better that way.
> 
> Maybe do this optimization in a separate patch to make it simpler to review.
> 
> BTW, the whole migration code has become more and more complicated due to
> batching at different levels, I wonder if it is worth the complication to
> optimize for the rare memory error case.


I agree on complication, but I think this change is actually simpler than my previous patch.

Using per-folio marker lets me drop the already_copied arg that was passed through
different function:
migrate_folios_move() -> migrate_folio_move() -> move_to_new_folio().
And there is no separate error path with this.

Whoever performs the batch copy (migrate_folios_mc_copy() or a driver callback)
sets FOLIO_CONTENT_COPIED on each dst it successfully copied, and __migrate_folio()
reads that marker to skip folio_mc_copy().

If copy is unsuccessful (e.g. memory error), FOLIO_CONTENT_COPIED is NOT marked,
and __migrate_folio() will not skip the folio_mc_copy().

Since the per-folio marking is the natural implementation for this, I kept it
in this patch (af44801ee).

Git Tree for reference: https://github.com/AMDESE/linux-mm/commits/shivank/batch-migrate-offload-v6-wip


Thanks,
Shivank