* [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping @ 2025-12-15 0:48 Wei Yang 2025-12-15 2:31 ` Barry Song 2025-12-15 3:19 ` wang lian 0 siblings, 2 replies; 10+ messages in thread From: Wei Yang @ 2025-12-15 0:48 UTC (permalink / raw) To: akpm, david, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, baohua, lance.yang Cc: linux-mm, Wei Yang 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) { - int i = 0; - /* If unmap_folio() uses try_to_migrate() on file, remove this check */ if (!folio_test_anon(folio)) return; - for (;;) { + do { remove_migration_ptes(folio, folio, RMP_LOCKED | flags); - i += folio_nr_pages(folio); - if (i >= nr) - break; folio = folio_next(folio); - } + } while (folio != end_folio); } static void lru_add_split_folio(struct folio *folio, struct folio *new_folio, @@ -4066,7 +4061,7 @@ static int __folio_split(struct folio *folio, unsigned int new_order, if (!ret && is_anon && !folio_is_device_private(folio)) remap_flags = RMP_USE_SHARED_ZEROPAGE; - remap_page(folio, 1 << old_order, remap_flags); + remap_page(folio, end_folio, remap_flags); /* * Unlock all after-split folios except the one containing -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-15 0:48 [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping Wei Yang @ 2025-12-15 2:31 ` Barry Song 2025-12-15 8:43 ` Wei Yang 2025-12-15 3:19 ` wang lian 1 sibling, 1 reply; 10+ messages in thread From: Barry Song @ 2025-12-15 2:31 UTC (permalink / raw) To: Wei Yang Cc: akpm, david, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm 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 :-) Thanks Barry ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-15 2:31 ` Barry Song @ 2025-12-15 8:43 ` Wei Yang 2025-12-15 9:15 ` Barry Song 0 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2025-12-15 8:43 UTC (permalink / raw) To: Barry Song Cc: Wei Yang, akpm, david, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm 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. But yeah, this is my personal preference. If it is not the case, I am fine to keep as it is now. :-) >Thanks >Barry -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-15 8:43 ` Wei Yang @ 2025-12-15 9:15 ` Barry Song 2025-12-15 11:57 ` Wei Yang 2025-12-18 9:58 ` David Hildenbrand (Red Hat) 0 siblings, 2 replies; 10+ messages in thread From: Barry Song @ 2025-12-15 9:15 UTC (permalink / raw) To: Wei Yang Cc: akpm, david, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-15 9:15 ` Barry Song @ 2025-12-15 11:57 ` Wei Yang 2025-12-18 9:58 ` David Hildenbrand (Red Hat) 1 sibling, 0 replies; 10+ messages in thread From: Wei Yang @ 2025-12-15 11:57 UTC (permalink / raw) To: Barry Song Cc: Wei Yang, akpm, david, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm On Mon, Dec 15, 2025 at 05:15:17PM +0800, 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. > Thanks for your feedback. >> >> 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 >Thanks >Barry -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-15 9:15 ` Barry Song 2025-12-15 11:57 ` Wei Yang @ 2025-12-18 9:58 ` David Hildenbrand (Red Hat) 2025-12-18 15:28 ` Wei Yang 1 sibling, 1 reply; 10+ messages in thread From: David Hildenbrand (Red Hat) @ 2025-12-18 9:58 UTC (permalink / raw) To: Barry Song, Wei Yang Cc: akpm, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-18 9:58 ` David Hildenbrand (Red Hat) @ 2025-12-18 15:28 ` Wei Yang 2025-12-19 14:26 ` David Hildenbrand (Red Hat) 0 siblings, 1 reply; 10+ messages in thread From: Wei Yang @ 2025-12-18 15:28 UTC (permalink / raw) To: David Hildenbrand (Red Hat) Cc: Barry Song, Wei Yang, akpm, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm 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; folio = folio_next(folio); } >probably more ... :) > Don't catch this. Sounds you have more idea on further cleanup? Glad to hear your insight. >-- >Cheers > >David -- Wei Yang Help you, Help me ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-18 15:28 ` Wei Yang @ 2025-12-19 14:26 ` David Hildenbrand (Red Hat) 2025-12-20 0:33 ` Wei Yang 0 siblings, 1 reply; 10+ messages in thread From: David Hildenbrand (Red Hat) @ 2025-12-19 14:26 UTC (permalink / raw) To: Wei Yang Cc: Barry Song, akpm, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-19 14:26 ` David Hildenbrand (Red Hat) @ 2025-12-20 0:33 ` Wei Yang 0 siblings, 0 replies; 10+ messages in thread From: Wei Yang @ 2025-12-20 0:33 UTC (permalink / raw) To: David Hildenbrand (Red Hat) Cc: Wei Yang, Barry Song, akpm, lorenzo.stoakes, ziy, baolin.wang, Liam.Howlett, npache, ryan.roberts, dev.jain, lance.yang, linux-mm 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping 2025-12-15 0:48 [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping Wei Yang 2025-12-15 2:31 ` Barry Song @ 2025-12-15 3:19 ` wang lian 1 sibling, 0 replies; 10+ messages in thread From: wang lian @ 2025-12-15 3:19 UTC (permalink / raw) To: richard.weiyang Cc: Liam.Howlett, akpm, baohua, baolin.wang, david, dev.jain, lance.yang, linux-mm, lorenzo.stoakes, npache, ryan.roberts, ziy, wang lian > 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) > { Reviewed v1. The end_folio logic in __folio_split is clear, but I agree with Barry that nr_pages is sufficiently intuitive. :-) -- Best Regards, wang lian ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-12-20 0:33 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-12-15 0:48 [Patch v2] mm/huge_memory: use end_folio to terminate anonymous folio remapping Wei Yang 2025-12-15 2:31 ` Barry Song 2025-12-15 8:43 ` Wei Yang 2025-12-15 9:15 ` Barry Song 2025-12-15 11:57 ` Wei Yang 2025-12-18 9:58 ` David Hildenbrand (Red Hat) 2025-12-18 15:28 ` Wei Yang 2025-12-19 14:26 ` David Hildenbrand (Red Hat) 2025-12-20 0:33 ` Wei Yang 2025-12-15 3:19 ` wang lian
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox