Re: [PATCH RFC v6 3/5] mm/migrate: add copy offload registration infrastructure

[Zi Yan <ziy@nvidia.com>]

On 20 Jul 2026, at 11:32, Garg, Shivank wrote:

> On 7/20/2026 8:14 PM, Zi Yan wrote:
>> On 20 Jul 2026, at 7:19, Huang, Ying wrote:
>>
>>> Shivank Garg <shivankg@amd.com> writes:
>>>
>>> [snip]
>>>
>>>> diff --git a/mm/migrate.c b/mm/migrate.c
>>>> index 41b732e78a67..4fed3110ca0a 100644
>>>> --- a/mm/migrate.c
>>>> +++ b/mm/migrate.c
>>>> @@ -43,6 +43,7 @@
>>>>  #include <linux/sched/sysctl.h>
>>>>  #include <linux/memory-tiers.h>
>>>>  #include <linux/pagewalk.h>
>>>> +#include <linux/migrate_copy_offload.h>
>>>>
>>>>  #include <asm/tlbflush.h>
>>>>
>>>> @@ -51,12 +52,6 @@
>>>>  #include "internal.h"
>>>>  #include "swap.h"
>>>>
>>>> -/* For now, never offload. Wired up in later patch. */
>>>> -static bool migrate_should_offload(int reason)
>>>> -{
>>>> -	return false;
>>>> -}
>>>> -
>>>>  static const struct movable_operations *offline_movable_ops;
>>>>  static const struct movable_operations *zsmalloc_movable_ops;
>>>>
>>>> @@ -2050,7 +2045,7 @@ static int migrate_pages_batch(struct list_head *from,
>>>>
>>>>  	/* Batch-copy eligible folios before the move phase */
>>>>  	if (!list_empty(&unmap_batch))
>>>> -		migrate_folios_mc_copy(&dst_batch, &unmap_batch, nr_batch);
>>>> +		migrate_offload_batch_copy(&dst_batch, &unmap_batch, nr_batch);
>>>>
>>>>  	retry = 1;
>>>>  	for (pass = 0; pass < nr_pass && retry; pass++) {
>>>
>>> Found a difference between v5 and v6 here.  In v5, migrate_pages_batch()
>>> checks the return value of migrate_offload_batch_copy() and acts
>>> accordingly. While in v6, it does not.  Why was this changed?
>>>
>>> IIUC, FOLIO_CONTENT_COPIED is now used to indicate a copy failure.  Is
>>> this necessary?  Is it possible that some (but not all) folios fail to
>>> be copied?
>>
>> Based on the discussion[1], FOLIO_CONTENT_COPIED is used to indicate both
>> 1. folio is copied via offloading, and 2. MC copy failure. So the code
>> no longer needs to store the return value of migrate_offload_batch_copy()
>> and can just check dst->migrate_info & FOLIO_CONTENT_COPIED.
>>
>> For 32bit, FOLIO_CONTENT_COPIED is always 0, so all folios are always
>> copied in the serialized path. Hmm, that might be causing double copying,
>> since migrate_folios_mc_copy() copies batched folios and later
>> migrate_folios_move() copies them again since
>> dst->migrate_info & FOLIO_CONTENT_COPIED is always false. Maybe we could
>> make migrate_folios_mc_copy() return immediately in 32bit, otherwise,
>> we will need to find somewhere else to store FOLIO_CONTENT_COPIED.
>>
>> [1] https://lore.kernel.org/all/c2bfdf39-8caa-43bd-b1ad-2285b4cc767a@amd.com/
>>
>>>
> It is not cause double copying.
> migrate_should_offload() /and migrate_offload_enabled
> gate if folio can be added to unmap_batch/dst_batch.

So migrate_should_offload() is actually migrate_should_offload_batched_copy().
32BIT does not get batched copy nor offloaded copy.

>
> migrate_offload_enabled branch is enabled from the offload driver like DCBM (which
> again depends on 64_BIT) will call migrate_offload_register().
>
> I think I can add a defensive 64_BIT check in migrate_should_offload() to prevent
> any potential bugs.

I think a better name would be helpful and sufficient.

BTW, ideally, batched copy and offloaded copy are two separate features
and could be enabled separately. But considering the hassle of finding a
place to store FOLIO_CONTENT_COPIED on 32BIT, it might be fine to
have a combined batched and offloaded copy.

Best Regards,
Yan, Zi