Re: [Patch v3] mm/huge_memory: simplify page tracking in remap_page() during folio split

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

On 12/22/25 19:14, Andrew Morton wrote:
> On Mon, 22 Dec 2025 13:45:31 +0000 Wei Yang <richard.weiyang@gmail.com> wrote:
> 
>> After splitting a large folio, the resulting anonymous folios must be
>> remapped.
>>
>> Currently, the code tracks progress by recording the number of processed
>> pages in an index variable (e.g., @i) and comparing it against the
>> total. This commit simplifies the logic by directly subtracting the
>> processed pages from the remaining count. This approach is more
>> straightforward and reduces the number of local variables.
>>
>> Additionally, this commit renames the variable nr to nr_pages to improve
>> code readability and self-documentation.
>>
>> ...
>>
>> --- 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;
>>   		folio = folio_next(folio);
>>   	}
> 
> This changes behavior when nr_pages was too large.  It would be best to
> mention this and to describe why this is OK.  Or to check for
> ((signed)nr_pages <= 0).
> 
> Also, isn't it a bit nasty to alter an incoming arg?  I think it's best
> to treat all those as const - part of the function's calling
> environment.  Yes, C permits this hack but that was a mistake!

For small functions I think it's quite acceptable.

But yeah, for bigger functions it can happen too easily that one ends up 
reusing a now-modified value by mistake.

-- 
Cheers

David