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

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

On 12/18/25 16:28, Wei Yang wrote:
> On Thu, Dec 18, 2025 at 10:58:36AM +0100, David Hildenbrand (Red Hat) wrote:
>> 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
>>
> 
> Sorry for my poor taste :-)
> 
> Is this what you expect?
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 1db9cfb09533..b8ee33318a60 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3423,17 +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, unsigned long nr_pages, int flags)
>   {
> -	int i = 0;
> -
>   	/* If unmap_folio() uses try_to_migrate() on file, remove this check */
>   	if (!folio_test_anon(folio))
>   		return;
>   	for (;;) {
>   		remove_migration_ptes(folio, folio, RMP_LOCKED | flags);
> -		i += folio_nr_pages(folio);
> -		if (i >= nr)
> +		nr_pages -= folio_nr_pages(folio);
> +		if (!nr_pages)
>   			break;

Right, I think we would never underflow nr_pages.

>   		folio = folio_next(folio);
>   	}
> 
>> probably more ... :)
>>
> 
> Don't catch this. Sounds you have more idea on further cleanup?

I was rather wondering whether there could be more cleanups, but I have 
nothing concrete in mind.

-- 
Cheers

David