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

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Dec 19, 2025 at 03:26:37PM +0100, David Hildenbrand (Red Hat) wrote:
>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.
>

Thanks. Well prepare v3 with this.

>>   		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

-- 
Wei Yang
Help you, Help me