1. Patchew
  2. linux
  3. [PATCH v10 00/13] khugepaged: mTHP support
  4. View patch

[PATCH v10 07/13] khugepaged: skip collapsing mTHP to smaller orders

Nico Pache posted 13 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v10 07/13] khugepaged: skip collapsing mTHP to smaller orders
Posted by Nico Pache 1 month, 2 weeks ago 
khugepaged may try to collapse a mTHP to a smaller mTHP, resulting in
some pages being unmapped. Skip these cases until we have a way to check
if its ok to collapse to a smaller mTHP size (like in the case of a
partially mapped folio).

This patch is inspired by Dev Jain's work on khugepaged mTHP support [1].

[1] https://lore.kernel.org/lkml/20241216165105.56185-11-dev.jain@arm.com/

Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
Co-developed-by: Dev Jain <dev.jain@arm.com>
Signed-off-by: Dev Jain <dev.jain@arm.com>
Signed-off-by: Nico Pache <npache@redhat.com>
---
 mm/khugepaged.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 1ad7e00d3fd6..6a4cf7e4a7cc 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -611,6 +611,15 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
 		folio = page_folio(page);
 		VM_BUG_ON_FOLIO(!folio_test_anon(folio), folio);
 
+		/*
+		 * TODO: In some cases of partially-mapped folios, we'd actually
+		 * want to collapse.
+		 */
+		if (order != HPAGE_PMD_ORDER && folio_order(folio) >= order) {
+			result = SCAN_PTE_MAPPED_HUGEPAGE;
+			goto out;
+		}
+
 		/* See collapse_scan_pmd(). */
 		if (folio_maybe_mapped_shared(folio)) {
 			++shared;
-- 
2.50.1
Re: [PATCH v10 07/13] khugepaged: skip collapsing mTHP to smaller orders
Posted by Lorenzo Stoakes 1 month, 1 week ago 
On Tue, Aug 19, 2025 at 07:41:59AM -0600, Nico Pache wrote:
> khugepaged may try to collapse a mTHP to a smaller mTHP, resulting in
> some pages being unmapped. Skip these cases until we have a way to check
> if its ok to collapse to a smaller mTHP size (like in the case of a
> partially mapped folio).
>
> This patch is inspired by Dev Jain's work on khugepaged mTHP support [1].
>
> [1] https://lore.kernel.org/lkml/20241216165105.56185-11-dev.jain@arm.com/
>
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> Co-developed-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> Signed-off-by: Nico Pache <npache@redhat.com>

Other than comment below, LGTM so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

> ---
>  mm/khugepaged.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 1ad7e00d3fd6..6a4cf7e4a7cc 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -611,6 +611,15 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>  		folio = page_folio(page);
>  		VM_BUG_ON_FOLIO(!folio_test_anon(folio), folio);
>
> +		/*
> +		 * TODO: In some cases of partially-mapped folios, we'd actually
> +		 * want to collapse.
> +		 */

Not a fan of adding todo's in code, they have a habit of being left forever. I'd
maybe put a more written  out comment something similar to the commit message.

> +		if (order != HPAGE_PMD_ORDER && folio_order(folio) >= order) {
> +			result = SCAN_PTE_MAPPED_HUGEPAGE;
> +			goto out;
> +		}
> +
>  		/* See collapse_scan_pmd(). */
>  		if (folio_maybe_mapped_shared(folio)) {
>  			++shared;
> --
> 2.50.1
>
Re: [PATCH v10 07/13] khugepaged: skip collapsing mTHP to smaller orders
Posted by Steven Rostedt 1 month, 1 week ago 
On Thu, 21 Aug 2025 13:05:42 +0100
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:

> > +		/*
> > +		 * TODO: In some cases of partially-mapped folios, we'd actually
> > +		 * want to collapse.
> > +		 */  
> 
> Not a fan of adding todo's in code, they have a habit of being left forever.

It's a way to make the developer more depressed by reminding them that they
will never be able to complete their TODO list :-p

Personally, I enjoy the torture of these comments in the code ;-)

-- Steve
Re: [PATCH v10 07/13] khugepaged: skip collapsing mTHP to smaller orders
Posted by Lorenzo Stoakes 1 month, 1 week ago 
On Thu, Aug 21, 2025 at 12:54:45PM -0400, Steven Rostedt wrote:
> On Thu, 21 Aug 2025 13:05:42 +0100
> Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
>
> > > +		/*
> > > +		 * TODO: In some cases of partially-mapped folios, we'd actually
> > > +		 * want to collapse.
> > > +		 */
> >
> > Not a fan of adding todo's in code, they have a habit of being left forever.
>
> It's a way to make the developer more depressed by reminding them that they
> will never be able to complete their TODO list :-p
>
> Personally, I enjoy the torture of these comments in the code ;-)

Well a person must at least _somewhat_ enjoy torture if they decide to do kernel
development (let alone maintainership :P) so this very much figures ;)
Re: [PATCH v10 07/13] khugepaged: skip collapsing mTHP to smaller orders
Posted by Dev Jain 1 month, 1 week ago 
On 21/08/25 5:35 pm, Lorenzo Stoakes wrote:
>
>
>> ---
>>   mm/khugepaged.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 1ad7e00d3fd6..6a4cf7e4a7cc 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -611,6 +611,15 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
>>   		folio = page_folio(page);
>>   		VM_BUG_ON_FOLIO(!folio_test_anon(folio), folio);
>>
>> +		/*
>> +		 * TODO: In some cases of partially-mapped folios, we'd actually
>> +		 * want to collapse.
>> +		 */
> Not a fan of adding todo's in code, they have a habit of being left forever. I'd
> maybe put a more written  out comment something similar to the commit message.

I had suggested to add in https://lore.kernel.org/all/20250211111326.14295-10-dev.jain@arm.com/
from the get go, but then we decided to leave it for later. So rest assured this TODO won't
be left forever : )

>
>> +		if (order != HPAGE_PMD_ORDER && folio_order(folio) >= order) {
>> +			result = SCAN_PTE_MAPPED_HUGEPAGE;
>> +			goto out;
>> +		}
>> +
>>   		/* See collapse_scan_pmd(). */
>>   		if (folio_maybe_mapped_shared(folio)) {
>>   			++shared;
>> --
>> 2.50.1
>>
Re: [PATCH v10 07/13] khugepaged: skip collapsing mTHP to smaller orders
Posted by Lorenzo Stoakes 1 month, 1 week ago 
On Thu, Aug 21, 2025 at 06:03:52PM +0530, Dev Jain wrote:
>
> On 21/08/25 5:35 pm, Lorenzo Stoakes wrote:
> >
> >
> > > ---
> > >   mm/khugepaged.c | 9 +++++++++
> > >   1 file changed, 9 insertions(+)
> > >
> > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > > index 1ad7e00d3fd6..6a4cf7e4a7cc 100644
> > > --- a/mm/khugepaged.c
> > > +++ b/mm/khugepaged.c
> > > @@ -611,6 +611,15 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> > >   		folio = page_folio(page);
> > >   		VM_BUG_ON_FOLIO(!folio_test_anon(folio), folio);
> > >
> > > +		/*
> > > +		 * TODO: In some cases of partially-mapped folios, we'd actually
> > > +		 * want to collapse.
> > > +		 */
> > Not a fan of adding todo's in code, they have a habit of being left forever. I'd
> > maybe put a more written  out comment something similar to the commit message.
>
> I had suggested to add in https://lore.kernel.org/all/20250211111326.14295-10-dev.jain@arm.com/
> from the get go, but then we decided to leave it for later. So rest assured this TODO won't
> be left forever : )

:)

I think it's better, despite Steven's sage words :P, to put something a little
more meaningful here like 'currently we don't blah blah because blah blah'
without the TODO ;)

But obviously this isn't a substantially pressing issue... :>)

Cheers, Lorenzo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.