Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping

["David Hildenbrand (Red Hat)" <david@kernel.org>]

On 12/15/25 10:15, Barry Song wrote:
> On Mon, Dec 15, 2025 at 4:43 PM Wei Yang <richard.weiyang@gmail.com> wrote:
>>
>> On Mon, Dec 15, 2025 at 10:31:15AM +0800, Barry Song wrote:
>>> On Mon, Dec 15, 2025 at 8:49 AM Wei Yang <richard.weiyang@gmail.com> wrote:
>>>>
>>>> After splitting a large folio, it is necessary to remap the resulting
>>>> anonymous folios.
>>>>
>>>> The current implementation determines the end of the remapping process
>>>> by counting the number of pages that have been processed.
>>>>
>>>> Since the final folio in the sequence, end_folio, is already known and
>>>> tracked, this commit refactors the remapping loop to leverage end_folio
>>>> as the termination marker.
>>>>
>>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>>> Cc: Zi Yan <ziy@nvidia.com>
>>>>
>>>> ---
>>>> v2: move folio assignment in loop
>>>> ---
>>>>   mm/huge_memory.c | 13 ++++---------
>>>>   1 file changed, 4 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>>> index 40cf59301c21..fe812d9c7807 100644
>>>> --- a/mm/huge_memory.c
>>>> +++ b/mm/huge_memory.c
>>>> @@ -3423,20 +3423,15 @@ bool unmap_huge_pmd_locked(struct vm_area_struct *vma, unsigned long addr,
>>>>          return __discard_anon_folio_pmd_locked(vma, addr, pmdp, folio);
>>>>   }
>>>>
>>>> -static void remap_page(struct folio *folio, unsigned long nr, int flags)
>>>> +static void remap_page(struct folio *folio, struct folio *end_folio, int flags)
>>>>   {
>>>
>>> Do we actually see an improvement in readability or performance?
>>> The existing code feels a bit more readable to me; nr is clearly
>>> the number of pages :-)
>>>
>>
>> Hi, Barry,
>>
>> Thanks for your review.
>>
>> Currently end_folio has been used in __folio_split() and
>> __folio_freeze_and_split_unmapped() as a termination marker. So continue to
>> use end_folio here as a termination marker looks consistent and improves
>> readability to me.
> 
> In both cases the variables are temporary, and they look fine within their
> own self-documented contexts.
> 
> Now that we’re crossing two functions in the context of remap_page(), this
> doesn’t seem to be the case, though.
> 
>>
>> But yeah, this is my personal preference. If it is not the case, I am fine to
>> keep as it is now. :-)
> 
> I personally find the existing code clearer, but I’m also okay if
> others like your new approach more.

Me too. Well, the existing code could be cleaned up
* nr -> nr_pages
* remove i and subtract from nr_pages instead

probably more ... :)

-- 
Cheers

David