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