Re: [PATCH v3 1/2] mm/huge_memory: move unrelated code out of __split_unmapped_folio()

[Zi Yan <ziy@nvidia.com>]

On 17 Jul 2025, at 13:44, Lorenzo Stoakes wrote:

> On Thu, Jul 17, 2025 at 11:41:01AM -0400, Zi Yan wrote:
>> On 17 Jul 2025, at 10:07, Lorenzo Stoakes wrote:
>>
>>> On Mon, Jul 14, 2025 at 01:18:22PM -0400, Zi Yan wrote:
>>>> remap(), folio_ref_unfreeze(), lru_add_split_folio() are not relevant to
>>>> splitting unmapped folio operations. Move them out to the caller so that
>>>> __split_unmapped_folio() only handles unmapped folio splits. This makes
>>>> __split_unmapped_folio() reusable.
>>>
>>> Nit but maybe worth mentioning the various renames etc.
>>
>> You mean release -> new_folio, origin_folio is replaced by folio?
>> Sure, I can do that.
>
> Yeah that kind of thing, just basically briefly mention the other stuff you
> did.
>
> Thanks!
>
>>
>>>
>>>>
>>>> Convert VM_BUG_ON(mapping) to use VM_WARN_ON_ONCE_FOLIO().
>>>>
>>>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>>>> Acked-by: Balbir Singh <balbirs@nvidia.com>
>>>
>>> After a lot of staring, 2 difftastic's at once and exactly 0 coverity
>>> instances, I've convinced myself this looks right.
>>>
>>> I think you really should have split this up into smaller patches, as this
>>> is moving stuff around and changing stuff all at once with a lot of
>>> complexity and moving parts.
>>>
>>> However not going to make you do that, since you got acks and I don't want
>>> to hold this up.
>>>
>>> I have a few nits + queries below that need addressing however, see below.
>>
>> Since I need to address these nits, I might just split this up.
>> How about:
>>
>> 1.
>
> Missing some text? :P

Yeah, I meant to fill this up after going through your comments below.
The plan is:

1. Just move code from __split_unmapped_folio() to __folio_split().
2. one patch to remove after_split label
3. one patch to move fail label and related code
4. one patch to refactor remap_page() flag
5. one patch to convert VM_BUG* to VM_WARM*, three instances.
6. use folio_expected_ref_count() patch
7. mm/huge_memory: refactor after-split (page) cache code.

Maybe 2, 3, 4 can be squashed into a single refactor patch?

Best Regards,
Yan, Zi