Re: [RFC PATCH v4 2/6] mm/migrate: skip data copy for already-copied folios

["Huang, Ying" <ying.huang@linux.alibaba.com>]

"Garg, Shivank" <shivankg@amd.com> writes:

> On 3/24/2026 1:52 PM, Huang, Ying wrote:
>> Shivank Garg <shivankg@amd.com> writes:
>> 
>
>>>  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;
>>> @@ -1096,6 +1114,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->private = (void *)(unsigned long)PAGE_ALREADY_COPIED;
>>> +
>> 
>> IMHO, this appears to be an unusual way to pass arguments to a function.
>> Why not adjust the parameters of migrate_folio()?  How about turning enum
>> migrate_mode into a bitmask (migrate_flags)?
>>
>
> Using folio->private, keeps the change self-contained in migrate.c
>
> David suggested adding a dedicated unsigned long migrate_info field in the
> folio union. I'll switch to that, as this is cleaner and avoid hacky use of ->private.
>
> https://lore.kernel.org/linux-mm/27b1b602-129f-4bc5-a553-386e8d1f5d90@kernel.org

That's good for the original usage of folio->private.  That is, to
record migration related information for a list of folios.

>
> Changing the migrate_folio() a_ops signature would touch nearly every
> filesystem for something that only core migration cares about, and does not look
> practical. We can't add to migrate_mode enum either as currently those values are mutually
> exclusive and ordered levels of increasing synchrony (ASYNC < SYNC_LIGHT < SYNC)
> And there are checks like this (cc->mode < MIGRATE_SYNC) or  mode != MIGRATE_SYNC
> This could break it.

IMHO, code readability is more important than limiting the scope of
changes. The migrate_folio callback of most file systems shares a few
common implementations in migrate.c. So, I think it is doable.

---
Best Regards,
Huang, Ying