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

[Barry Song <21cnbao@gmail.com>]

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.

Thanks
Barry