Re: [RFC PATCH v4 2/6] mm/migrate: skip data copy for already-copied folios
From: Huang, Ying
Date: Tue Apr 07 2026 - 02:55:44 EST
"Garg, Shivank" <shivankg@xxxxxxx> writes:
> On 3/24/2026 1:52 PM, Huang, Ying wrote:
>> Shivank Garg <shivankg@xxxxxxx> 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@xxxxxxxxxx
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