linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] mm/hugetlb: A fix and some cleanups
@ 2026-01-15 18:14 Joshua Hahn
  2026-01-15 18:14 ` [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool Joshua Hahn
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 18:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Muchun Song, Oscar Salvador, linux-kernel,
	linux-mm, kernel-team

As the subject suggests, a fix and some cleanup patches.

The first patch fixes a hugetlb accounting error that would leave
hugetlb subpools with an elevated used_hpages count, if a reservation
attempt passes the subpool limit check but fails to acquire pages from
the global pool. It was introduced in a833a693a490: ("mm: hugetlb: fix
incorrect fallback for subpool"), which itself was a fix for a hugetlb
reservation accounting error.

The last two patches are some cleanup patches that I've been holding
onto for a while that I didn't feel was big enough to send on their own.
No functional changes intended for the last two.

Joshua Hahn (3):
  mm/hugetlb: Restore failed global reservations to subpool
  mm/hugetlb: Remove unnecessary if condition
  mm/hugetlb: Enforce brace style

 mm/hugetlb.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)


base-commit: d70f9612414bd3ed6bb709ccbeb4206d1a1927a5
-- 
2.47.3


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool
  2026-01-15 18:14 [PATCH 0/3] mm/hugetlb: A fix and some cleanups Joshua Hahn
@ 2026-01-15 18:14 ` Joshua Hahn
  2026-01-15 19:19   ` Andrew Morton
  2026-01-15 18:14 ` [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition Joshua Hahn
  2026-01-15 18:14 ` [PATCH 3/3] mm/hugetlb: Enforce brace style Joshua Hahn
  2 siblings, 1 reply; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 18:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Muchun Song, Oscar Salvador, Wupeng Ma,
	linux-kernel, linux-mm, kernel-team, stable

Commit a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
fixed an underflow error for hstate->resv_huge_pages caused by
incorrectly attributing globally requested pages to the subpool's
reservation.

Unfortunately, this fix also introduced the opposite problem, which would
leave spool->used_hpages elevated if the globally requested pages could
not be acquired. This is because while a subpool's reserve pages only
accounts for what is requested and allocated from the subpool, its
"used" counter keeps track of what is consumed in total, both from the
subpool and globally. Thus, we need to adjust spool->used_hpages in the
other direction, and make sure that globally requested pages are
uncharged from the subpool's used counter.

Each failed allocation attempt increments the used_hpages counter by
how many pages were requested from the global pool. Ultimately, this
renders the subpool unusable, as used_hpages approaches the max limit.

The issue can be reproduced as follows:
1. Allocate 4 hugetlb pages
2. Create a hugetlb mount with max=4, min=2
3. Consume 2 pages globally
4. Request 3 pages from the subpool (2 from subpool + 1 from global)
	4.1 hugepage_subpool_get_pages(spool, 3) succeeds.
		used_hpages += 3
	4.2 hugetlb_acct_memory(h, 1) fails: no global pages left
		used_hpages -= 2
5. Subpool now has used_hpages = 1, despite not being able to
   successfully allocate any hugepages. It believes it can now only
   allocate 3 more hugepages, not 4.

Repeating this process will ultimately render the subpool unable to
allocate any hugepages, since it believes that it is using the maximum
number of hugepages that the subpool has been allotted.

The underflow issue that commit a833a693a490 fixes still remains fixed
as well.

Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
Cc: stable@vger.kernel.org
---
 mm/hugetlb.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 2e296d30a8d7..88b9e997c9da 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -6560,6 +6560,7 @@ long hugetlb_reserve_pages(struct inode *inode,
 	struct resv_map *resv_map;
 	struct hugetlb_cgroup *h_cg = NULL;
 	long gbl_reserve, regions_needed = 0;
+	unsigned long flags;
 	int err;
 
 	/* This should never happen */
@@ -6704,6 +6705,13 @@ long hugetlb_reserve_pages(struct inode *inode,
 		 */
 		hugetlb_acct_memory(h, -gbl_resv);
 	}
+	/* Restore used_hpages for pages that failed global reservation */
+	if (gbl_reserve && spool) {
+		spin_lock_irqsave(&spool->lock, flags);
+		if (spool->max_hpages != -1)
+			spool->used_hpages -= gbl_reserve;
+		unlock_or_release_subpool(spool, flags);
+	}
 out_uncharge_cgroup:
 	hugetlb_cgroup_uncharge_cgroup_rsvd(hstate_index(h),
 					    chg * pages_per_huge_page(h), h_cg);
-- 
2.47.3


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition
  2026-01-15 18:14 [PATCH 0/3] mm/hugetlb: A fix and some cleanups Joshua Hahn
  2026-01-15 18:14 ` [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool Joshua Hahn
@ 2026-01-15 18:14 ` Joshua Hahn
  2026-01-15 20:14   ` David Hildenbrand (Red Hat)
  2026-01-16  1:39   ` SeongJae Park
  2026-01-15 18:14 ` [PATCH 3/3] mm/hugetlb: Enforce brace style Joshua Hahn
  2 siblings, 2 replies; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 18:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Muchun Song, Oscar Salvador, linux-kernel, linux-mm

if (map_chg) is always true, since it is nested in another if statement
which checks for it already. Remove the check and un-indent for readability.

if (unlikely(map_chg == MAP_CHG_NEEDED && retval == 0)) {
	...

	if (map_chg) {
		...
	}
}

No functional change intended.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 mm/hugetlb.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 88b9e997c9da..432a5054ca1d 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3019,13 +3019,11 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
 
 			rsv_adjust = hugepage_subpool_put_pages(spool, 1);
 			hugetlb_acct_memory(h, -rsv_adjust);
-			if (map_chg) {
-				spin_lock_irq(&hugetlb_lock);
-				hugetlb_cgroup_uncharge_folio_rsvd(
-				    hstate_index(h), pages_per_huge_page(h),
-				    folio);
-				spin_unlock_irq(&hugetlb_lock);
-			}
+			spin_lock_irq(&hugetlb_lock);
+			hugetlb_cgroup_uncharge_folio_rsvd(
+			    hstate_index(h), pages_per_huge_page(h),
+			    folio);
+			spin_unlock_irq(&hugetlb_lock);
 		}
 	}
 
-- 
2.47.3


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH 3/3] mm/hugetlb: Enforce brace style
  2026-01-15 18:14 [PATCH 0/3] mm/hugetlb: A fix and some cleanups Joshua Hahn
  2026-01-15 18:14 ` [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool Joshua Hahn
  2026-01-15 18:14 ` [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition Joshua Hahn
@ 2026-01-15 18:14 ` Joshua Hahn
  2026-01-15 20:10   ` David Hildenbrand (Red Hat)
  2 siblings, 1 reply; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 18:14 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Muchun Song, Oscar Salvador, linux-kernel, linux-mm

Documentation/process/coding-style.rst explicitly notes that if only one
branch of a conditional statement is a single statement, braces should
be used in both branches. Enforce this in mm/hugetlb.c.

No functional change intended.

Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
---
 mm/hugetlb.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 432a5054ca1d..75f002f18365 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -578,8 +578,9 @@ hugetlb_resv_map_add(struct resv_map *map, struct list_head *rg, long from,
 		record_hugetlb_cgroup_uncharge_info(cg, h, map, nrg);
 		list_add(&nrg->link, rg);
 		coalesce_file_region(map, nrg);
-	} else
+	} else {
 		*regions_needed += 1;
+	}
 
 	return to - from;
 }
@@ -1247,8 +1248,9 @@ void hugetlb_dup_vma_private(struct vm_area_struct *vma)
 
 		if (vma_lock && vma_lock->vma != vma)
 			vma->vm_private_data = NULL;
-	} else
+	} else {
 		vma->vm_private_data = NULL;
+	}
 }
 
 /*
@@ -2076,8 +2078,9 @@ int dissolve_free_hugetlb_folio(struct folio *folio)
 				h->max_huge_pages++;
 				goto out;
 			}
-		} else
+		} else {
 			rc = 0;
+		}
 
 		update_and_free_hugetlb_folio(h, folio, false);
 		return rc;
@@ -2672,11 +2675,13 @@ void restore_reserve_on_error(struct hstate *h, struct vm_area_struct *vma,
 				 * be consumed on a subsequent allocation.
 				 */
 				folio_set_hugetlb_restore_reserve(folio);
-		} else
+		} else {
 			/*
 			 * No reservation present, do nothing
 			 */
 			 vma_end_reservation(h, vma, address);
+
+		}
 	}
 }
 
@@ -4703,10 +4708,12 @@ static void hugetlb_vm_op_open(struct vm_area_struct *vma)
 			if (vma_lock->vma != vma) {
 				vma->vm_private_data = NULL;
 				hugetlb_vma_lock_alloc(vma);
-			} else
+			} else {
 				pr_warn("HugeTLB: vma_lock already exists in %s.\n", __func__);
-		} else
+			}
+		} else {
 			hugetlb_vma_lock_alloc(vma);
+		}
 	}
 }
 
-- 
2.47.3


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool
  2026-01-15 18:14 ` [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool Joshua Hahn
@ 2026-01-15 19:19   ` Andrew Morton
  2026-01-15 19:45     ` Joshua Hahn
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2026-01-15 19:19 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: David Hildenbrand, Muchun Song, Oscar Salvador, Wupeng Ma,
	linux-kernel, linux-mm, kernel-team, stable

On Thu, 15 Jan 2026 13:14:35 -0500 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> Commit a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> fixed an underflow error for hstate->resv_huge_pages caused by
> incorrectly attributing globally requested pages to the subpool's
> reservation.
> 
> Unfortunately, this fix also introduced the opposite problem, which would
> leave spool->used_hpages elevated if the globally requested pages could
> not be acquired. This is because while a subpool's reserve pages only
> accounts for what is requested and allocated from the subpool, its
> "used" counter keeps track of what is consumed in total, both from the
> subpool and globally. Thus, we need to adjust spool->used_hpages in the
> other direction, and make sure that globally requested pages are
> uncharged from the subpool's used counter.
> 
> Each failed allocation attempt increments the used_hpages counter by
> how many pages were requested from the global pool. Ultimately, this
> renders the subpool unusable, as used_hpages approaches the max limit.
> 
> The issue can be reproduced as follows:
> 1. Allocate 4 hugetlb pages
> 2. Create a hugetlb mount with max=4, min=2
> 3. Consume 2 pages globally
> 4. Request 3 pages from the subpool (2 from subpool + 1 from global)
> 	4.1 hugepage_subpool_get_pages(spool, 3) succeeds.
> 		used_hpages += 3
> 	4.2 hugetlb_acct_memory(h, 1) fails: no global pages left
> 		used_hpages -= 2
> 5. Subpool now has used_hpages = 1, despite not being able to
>    successfully allocate any hugepages. It believes it can now only
>    allocate 3 more hugepages, not 4.
> 
> Repeating this process will ultimately render the subpool unable to
> allocate any hugepages, since it believes that it is using the maximum
> number of hugepages that the subpool has been allotted.
> 
> The underflow issue that commit a833a693a490 fixes still remains fixed
> as well.

Thanks, I submitted the above to the Changelog Of The Year judging
committee.

> Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> Cc: stable@vger.kernel.org

I'll add this to mm.git's mm-hotfixes queue, for testing and review
input.

> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -6560,6 +6560,7 @@ long hugetlb_reserve_pages(struct inode *inode,
>  	struct resv_map *resv_map;
>  	struct hugetlb_cgroup *h_cg = NULL;
>  	long gbl_reserve, regions_needed = 0;
> +	unsigned long flags;

This could have been local to the {block} which uses it, which would be
nicer, no?

>  	int err;
>  
>  	/* This should never happen */
> @@ -6704,6 +6705,13 @@ long hugetlb_reserve_pages(struct inode *inode,
>  		 */
>  		hugetlb_acct_memory(h, -gbl_resv);
>  	}
> +	/* Restore used_hpages for pages that failed global reservation */
> +	if (gbl_reserve && spool) {
> +		spin_lock_irqsave(&spool->lock, flags);
> +		if (spool->max_hpages != -1)
> +			spool->used_hpages -= gbl_reserve;
> +		unlock_or_release_subpool(spool, flags);
> +	}

I'll add [2/3] and [3/3] to the mm-new queue while discarding your
perfectly good [0/N] :(

Please, let's try not to mix backportable patches with the
non-backportable ones?



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool
  2026-01-15 19:19   ` Andrew Morton
@ 2026-01-15 19:45     ` Joshua Hahn
  0 siblings, 0 replies; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 19:45 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand, Muchun Song, Oscar Salvador, Wupeng Ma,
	linux-kernel, linux-mm, kernel-team, stable

On Thu, 15 Jan 2026 11:19:46 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 15 Jan 2026 13:14:35 -0500 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

Hello Andrew, I hope you are doing well. Thank you for your help as always!

> > Commit a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> > fixed an underflow error for hstate->resv_huge_pages caused by
> > incorrectly attributing globally requested pages to the subpool's
> > reservation.
> > 
> > Unfortunately, this fix also introduced the opposite problem, which would
> > leave spool->used_hpages elevated if the globally requested pages could
> > not be acquired. This is because while a subpool's reserve pages only
> > accounts for what is requested and allocated from the subpool, its
> > "used" counter keeps track of what is consumed in total, both from the
> > subpool and globally. Thus, we need to adjust spool->used_hpages in the
> > other direction, and make sure that globally requested pages are
> > uncharged from the subpool's used counter.
> > 
> > Each failed allocation attempt increments the used_hpages counter by
> > how many pages were requested from the global pool. Ultimately, this
> > renders the subpool unusable, as used_hpages approaches the max limit.
> > 
> > The issue can be reproduced as follows:
> > 1. Allocate 4 hugetlb pages
> > 2. Create a hugetlb mount with max=4, min=2
> > 3. Consume 2 pages globally
> > 4. Request 3 pages from the subpool (2 from subpool + 1 from global)
> > 	4.1 hugepage_subpool_get_pages(spool, 3) succeeds.
> > 		used_hpages += 3
> > 	4.2 hugetlb_acct_memory(h, 1) fails: no global pages left
> > 		used_hpages -= 2
> > 5. Subpool now has used_hpages = 1, despite not being able to
> >    successfully allocate any hugepages. It believes it can now only
> >    allocate 3 more hugepages, not 4.
> > 
> > Repeating this process will ultimately render the subpool unable to
> > allocate any hugepages, since it believes that it is using the maximum
> > number of hugepages that the subpool has been allotted.
> > 
> > The underflow issue that commit a833a693a490 fixes still remains fixed
> > as well.
> 
> Thanks, I submitted the above to the Changelog Of The Year judging
> committee.

: -) Thank you for the kind words!

> > Fixes: a833a693a490 ("mm: hugetlb: fix incorrect fallback for subpool")
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> > Cc: stable@vger.kernel.org
> 
> I'll add this to mm.git's mm-hotfixes queue, for testing and review
> input.

Sounds good to me! I'll wait a bit in case others have different concerns,
but I'll send out a new version which addresses your comments below (and
any future comments) in a day or two.

> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -6560,6 +6560,7 @@ long hugetlb_reserve_pages(struct inode *inode,
> >  	struct resv_map *resv_map;
> >  	struct hugetlb_cgroup *h_cg = NULL;
> >  	long gbl_reserve, regions_needed = 0;
> > +	unsigned long flags;
> 
> This could have been local to the {block} which uses it, which would be
> nicer, no?

Definiely, I'll address this in v2!

> >  	int err;
> >  
> >  	/* This should never happen */
> > @@ -6704,6 +6705,13 @@ long hugetlb_reserve_pages(struct inode *inode,
> >  		 */
> >  		hugetlb_acct_memory(h, -gbl_resv);
> >  	}
> > +	/* Restore used_hpages for pages that failed global reservation */
> > +	if (gbl_reserve && spool) {
> > +		spin_lock_irqsave(&spool->lock, flags);
> > +		if (spool->max_hpages != -1)
> > +			spool->used_hpages -= gbl_reserve;
> > +		unlock_or_release_subpool(spool, flags);
> > +	}
> 
> I'll add [2/3] and [3/3] to the mm-new queue while discarding your
> perfectly good [0/N] :(
> 
> Please, let's try not to mix backportable patches with the
> non-backportable ones?

Oh no! Sorry, this is my first time Cc-ing stable so I wasn't aware of the
implications. In v2, I'll send the two out as separate patches, so that it's
easier to backport. I was just eager to send out 2/3 and 3/3 because I've
been waiting for a functional hugetlb patch to smoosh these cleanups into.

I'll be more mindful in the future.

Thank you again, I hope you have a great day!!
Joshua


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mm/hugetlb: Enforce brace style
  2026-01-15 18:14 ` [PATCH 3/3] mm/hugetlb: Enforce brace style Joshua Hahn
@ 2026-01-15 20:10   ` David Hildenbrand (Red Hat)
  2026-01-15 21:14     ` Joshua Hahn
  0 siblings, 1 reply; 17+ messages in thread
From: David Hildenbrand (Red Hat) @ 2026-01-15 20:10 UTC (permalink / raw)
  To: Joshua Hahn, Andrew Morton
  Cc: Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On 1/15/26 19:14, Joshua Hahn wrote:
> Documentation/process/coding-style.rst explicitly notes that if only one
> branch of a conditional statement is a single statement, braces should
> be used in both branches. Enforce this in mm/hugetlb.c.
> 
> No functional change intended.
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> ---

[...]

>   		update_and_free_hugetlb_folio(h, folio, false);
>   		return rc;
> @@ -2672,11 +2675,13 @@ void restore_reserve_on_error(struct hstate *h, struct vm_area_struct *vma,
>   				 * be consumed on a subsequent allocation.
>   				 */
>   				folio_set_hugetlb_restore_reserve(folio);
> -		} else
> +		} else {
>   			/*
>   			 * No reservation present, do nothing
>   			 */
>   			 vma_end_reservation(h, vma, address);
> +

But why the empty line? :)

> +		}
>   	}
>   }



-- 
Cheers

David


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition
  2026-01-15 18:14 ` [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition Joshua Hahn
@ 2026-01-15 20:14   ` David Hildenbrand (Red Hat)
  2026-01-15 21:10     ` Joshua Hahn
  2026-01-16  1:39   ` SeongJae Park
  1 sibling, 1 reply; 17+ messages in thread
From: David Hildenbrand (Red Hat) @ 2026-01-15 20:14 UTC (permalink / raw)
  To: Joshua Hahn, Andrew Morton
  Cc: Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On 1/15/26 19:14, Joshua Hahn wrote:
> if (map_chg) is always true, since it is nested in another if statement
> which checks for it already. Remove the check and un-indent for readability.
> 
> if (unlikely(map_chg == MAP_CHG_NEEDED && retval == 0)) {
> 	...
> 
> 	if (map_chg) {
> 		...
> 	}
> }
> 
> No functional change intended.
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> ---
>   mm/hugetlb.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 88b9e997c9da..432a5054ca1d 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3019,13 +3019,11 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>   
>   			rsv_adjust = hugepage_subpool_put_pages(spool, 1);
>   			hugetlb_acct_memory(h, -rsv_adjust);
> -			if (map_chg) {
> -				spin_lock_irq(&hugetlb_lock);
> -				hugetlb_cgroup_uncharge_folio_rsvd(
> -				    hstate_index(h), pages_per_huge_page(h),
> -				    folio);
> -				spin_unlock_irq(&hugetlb_lock);
> -			}
> +			spin_lock_irq(&hugetlb_lock);
> +			hugetlb_cgroup_uncharge_folio_rsvd(
> +			    hstate_index(h), pages_per_huge_page(h),
> +			    folio);
> +			spin_unlock_irq(&hugetlb_lock);
>   		}
>   	}
>   

MAP_CHG_NEEDED = 1

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition
  2026-01-15 20:14   ` David Hildenbrand (Red Hat)
@ 2026-01-15 21:10     ` Joshua Hahn
  2026-01-15 21:59       ` Andrew Morton
  0 siblings, 1 reply; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 21:10 UTC (permalink / raw)
  To: David Hildenbrand (Red Hat)
  Cc: Andrew Morton, Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On Thu, 15 Jan 2026 21:14:15 +0100 "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:

> On 1/15/26 19:14, Joshua Hahn wrote:
> > if (map_chg) is always true, since it is nested in another if statement
> > which checks for it already. Remove the check and un-indent for readability.
> > 
> > if (unlikely(map_chg == MAP_CHG_NEEDED && retval == 0)) {
> > 	...
> > 
> > 	if (map_chg) {
> > 		...
> > 	}
> > }
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> > ---
> >   mm/hugetlb.c | 12 +++++-------
> >   1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 88b9e997c9da..432a5054ca1d 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -3019,13 +3019,11 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> >   
> >   			rsv_adjust = hugepage_subpool_put_pages(spool, 1);
> >   			hugetlb_acct_memory(h, -rsv_adjust);
> > -			if (map_chg) {
> > -				spin_lock_irq(&hugetlb_lock);
> > -				hugetlb_cgroup_uncharge_folio_rsvd(
> > -				    hstate_index(h), pages_per_huge_page(h),
> > -				    folio);
> > -				spin_unlock_irq(&hugetlb_lock);
> > -			}
> > +			spin_lock_irq(&hugetlb_lock);
> > +			hugetlb_cgroup_uncharge_folio_rsvd(
> > +			    hstate_index(h), pages_per_huge_page(h),
> > +			    folio);
> > +			spin_unlock_irq(&hugetlb_lock);
> >   		}
> >   	}
> >   
> 
> MAP_CHG_NEEDED = 1
> 
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

Thank you David, I agree it's helpful to include MAP_CHG_NEEDED != 0.
Will add in the v2! Have a great day : -)

Joshua


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mm/hugetlb: Enforce brace style
  2026-01-15 20:10   ` David Hildenbrand (Red Hat)
@ 2026-01-15 21:14     ` Joshua Hahn
  2026-01-15 21:16       ` David Hildenbrand (Red Hat)
  2026-01-15 21:57       ` Andrew Morton
  0 siblings, 2 replies; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 21:14 UTC (permalink / raw)
  To: David Hildenbrand (Red Hat)
  Cc: Andrew Morton, Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On Thu, 15 Jan 2026 21:10:41 +0100 "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:

> On 1/15/26 19:14, Joshua Hahn wrote:
> > Documentation/process/coding-style.rst explicitly notes that if only one
> > branch of a conditional statement is a single statement, braces should
> > be used in both branches. Enforce this in mm/hugetlb.c.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> > ---
> 
> [...]
> 
> >   		update_and_free_hugetlb_folio(h, folio, false);
> >   		return rc;
> > @@ -2672,11 +2675,13 @@ void restore_reserve_on_error(struct hstate *h, struct vm_area_struct *vma,
> >   				 * be consumed on a subsequent allocation.
> >   				 */
> >   				folio_set_hugetlb_restore_reserve(folio);
> > -		} else
> > +		} else {
> >   			/*
> >   			 * No reservation present, do nothing
> >   			 */
> >   			 vma_end_reservation(h, vma, address);
> > +
> 
> But why the empty line? :)

Hello David,

Whoops, I totally forgot to run checkpatch on this one. I think Andrew
also noticed this, he sent a fixlet for it. Sorry for this! I'll fix it up
in v2. 

Thanks again, have a great day David!
Joshua


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mm/hugetlb: Enforce brace style
  2026-01-15 21:14     ` Joshua Hahn
@ 2026-01-15 21:16       ` David Hildenbrand (Red Hat)
  2026-01-16  1:54         ` SeongJae Park
  2026-01-15 21:57       ` Andrew Morton
  1 sibling, 1 reply; 17+ messages in thread
From: David Hildenbrand (Red Hat) @ 2026-01-15 21:16 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: Andrew Morton, Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On 1/15/26 22:14, Joshua Hahn wrote:
> On Thu, 15 Jan 2026 21:10:41 +0100 "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
> 
>> On 1/15/26 19:14, Joshua Hahn wrote:
>>> Documentation/process/coding-style.rst explicitly notes that if only one
>>> branch of a conditional statement is a single statement, braces should
>>> be used in both branches. Enforce this in mm/hugetlb.c.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
>>> ---
>>
>> [...]
>>
>>>    		update_and_free_hugetlb_folio(h, folio, false);
>>>    		return rc;
>>> @@ -2672,11 +2675,13 @@ void restore_reserve_on_error(struct hstate *h, struct vm_area_struct *vma,
>>>    				 * be consumed on a subsequent allocation.
>>>    				 */
>>>    				folio_set_hugetlb_restore_reserve(folio);
>>> -		} else
>>> +		} else {
>>>    			/*
>>>    			 * No reservation present, do nothing
>>>    			 */
>>>    			 vma_end_reservation(h, vma, address);
>>> +
>>
>> But why the empty line? :)
> 
> Hello David,
> 
> Whoops, I totally forgot to run checkpatch on this one. I think Andrew
> also noticed this, he sent a fixlet for it. Sorry for this! I'll fix it up
> in v2.

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

with that ;)

-- 
Cheers

David


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mm/hugetlb: Enforce brace style
  2026-01-15 21:14     ` Joshua Hahn
  2026-01-15 21:16       ` David Hildenbrand (Red Hat)
@ 2026-01-15 21:57       ` Andrew Morton
  1 sibling, 0 replies; 17+ messages in thread
From: Andrew Morton @ 2026-01-15 21:57 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: David Hildenbrand (Red Hat),
	Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On Thu, 15 Jan 2026 16:14:46 -0500 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> On Thu, 15 Jan 2026 21:10:41 +0100 "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
> 
> > On 1/15/26 19:14, Joshua Hahn wrote:
> > > Documentation/process/coding-style.rst explicitly notes that if only one
> > > branch of a conditional statement is a single statement, braces should
> > > be used in both branches. Enforce this in mm/hugetlb.c.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> > > ---
> > 
> > [...]
> > 
> > >   		update_and_free_hugetlb_folio(h, folio, false);
> > >   		return rc;
> > > @@ -2672,11 +2675,13 @@ void restore_reserve_on_error(struct hstate *h, struct vm_area_struct *vma,
> > >   				 * be consumed on a subsequent allocation.
> > >   				 */
> > >   				folio_set_hugetlb_restore_reserve(folio);
> > > -		} else
> > > +		} else {
> > >   			/*
> > >   			 * No reservation present, do nothing
> > >   			 */
> > >   			 vma_end_reservation(h, vma, address);
> > > +
> > 
> > But why the empty line? :)
> 
> Hello David,
> 
> Whoops, I totally forgot to run checkpatch on this one. I think Andrew
> also noticed this, he sent a fixlet for it.

I did.

> Sorry for this! I'll fix it up in v2. 

I'm not actually seeing a need for a v2, unless something else changed?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition
  2026-01-15 21:10     ` Joshua Hahn
@ 2026-01-15 21:59       ` Andrew Morton
  2026-01-15 22:04         ` Joshua Hahn
  0 siblings, 1 reply; 17+ messages in thread
From: Andrew Morton @ 2026-01-15 21:59 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: David Hildenbrand (Red Hat),
	Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On Thu, 15 Jan 2026 16:10:51 -0500 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> > > -				spin_unlock_irq(&hugetlb_lock);
> > > -			}
> > > +			spin_lock_irq(&hugetlb_lock);
> > > +			hugetlb_cgroup_uncharge_folio_rsvd(
> > > +			    hstate_index(h), pages_per_huge_page(h),
> > > +			    folio);
> > > +			spin_unlock_irq(&hugetlb_lock);
> > >   		}
> > >   	}
> > >   
> > 
> > MAP_CHG_NEEDED = 1
> > 
> > Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> 
> Thank you David, I agree it's helpful to include MAP_CHG_NEEDED != 0.

Oh.

> Will add in the v2! Have a great day : -)

While in there, this:

> > > +			hugetlb_cgroup_uncharge_folio_rsvd(
> > > +			    hstate_index(h), pages_per_huge_page(h),
> > > +			    folio);

contains now-less-needed 80-column party tricks.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition
  2026-01-15 21:59       ` Andrew Morton
@ 2026-01-15 22:04         ` Joshua Hahn
  0 siblings, 0 replies; 17+ messages in thread
From: Joshua Hahn @ 2026-01-15 22:04 UTC (permalink / raw)
  To: Andrew Morton
  Cc: David Hildenbrand (Red Hat),
	Muchun Song, Oscar Salvador, linux-kernel, linux-mm

On Thu, 15 Jan 2026 13:59:42 -0800 Andrew Morton <akpm@linux-foundation.org> wrote:

> On Thu, 15 Jan 2026 16:10:51 -0500 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> > > > -				spin_unlock_irq(&hugetlb_lock);
> > > > -			}
> > > > +			spin_lock_irq(&hugetlb_lock);
> > > > +			hugetlb_cgroup_uncharge_folio_rsvd(
> > > > +			    hstate_index(h), pages_per_huge_page(h),
> > > > +			    folio);
> > > > +			spin_unlock_irq(&hugetlb_lock);
> > > >   		}
> > > >   	}
> > > >   
> > > 
> > > MAP_CHG_NEEDED = 1
> > > 
> > > Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> > 
> > Thank you David, I agree it's helpful to include MAP_CHG_NEEDED != 0.
> 
> Oh.
> 
> > Will add in the v2! Have a great day : -)
> 
> While in there, this:
> 
> > > > +			hugetlb_cgroup_uncharge_folio_rsvd(
> > > > +			    hstate_index(h), pages_per_huge_page(h),
> > > > +			    folio);
> 
> contains now-less-needed 80-column party tricks.

Ah, that's a nice catch as well. I'll be sure to include it as well!

My bigger motivation for sending a v2 was mostly to separate out the
fix from the cleanup patches. Since they are separate now, I'll send out a
v2 for 2/3 and 3/3, while I wait for reviews for 1/3 and send out a v2 for
that one at a later point. For what it's worth, I don't think the fix is
super urgent : -)

Thanks again, Andrew!
Joshua


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition
  2026-01-15 18:14 ` [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition Joshua Hahn
  2026-01-15 20:14   ` David Hildenbrand (Red Hat)
@ 2026-01-16  1:39   ` SeongJae Park
  2026-01-16 16:45     ` Joshua Hahn
  1 sibling, 1 reply; 17+ messages in thread
From: SeongJae Park @ 2026-01-16  1:39 UTC (permalink / raw)
  To: Joshua Hahn
  Cc: SeongJae Park, Andrew Morton, David Hildenbrand, Muchun Song,
	Oscar Salvador, linux-kernel, linux-mm

On Thu, 15 Jan 2026 13:14:36 -0500 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:

> if (map_chg) is always true, since it is nested in another if statement
> which checks for it already. Remove the check and un-indent for readability.
> 
> if (unlikely(map_chg == MAP_CHG_NEEDED && retval == 0)) {
> 	...
> 
> 	if (map_chg) {
> 		...
> 	}
> }
> 
> No functional change intended.
> 
> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>

Reviewed-by: SeongJae Park <sj@kernel.org>

> ---
>  mm/hugetlb.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 88b9e997c9da..432a5054ca1d 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3019,13 +3019,11 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
>  
>  			rsv_adjust = hugepage_subpool_put_pages(spool, 1);
>  			hugetlb_acct_memory(h, -rsv_adjust);
> -			if (map_chg) {
> -				spin_lock_irq(&hugetlb_lock);
> -				hugetlb_cgroup_uncharge_folio_rsvd(
> -				    hstate_index(h), pages_per_huge_page(h),
> -				    folio);
> -				spin_unlock_irq(&hugetlb_lock);
> -			}
> +			spin_lock_irq(&hugetlb_lock);
> +			hugetlb_cgroup_uncharge_folio_rsvd(
> +			    hstate_index(h), pages_per_huge_page(h),
> +			    folio);

Nit.  Good chance to reduce one more line by putting 'folio' on the upper line?

> +			spin_unlock_irq(&hugetlb_lock);
>  		}
>  	}
>  
> -- 
> 2.47.3


Thanks,
SJ


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 3/3] mm/hugetlb: Enforce brace style
  2026-01-15 21:16       ` David Hildenbrand (Red Hat)
@ 2026-01-16  1:54         ` SeongJae Park
  0 siblings, 0 replies; 17+ messages in thread
From: SeongJae Park @ 2026-01-16  1:54 UTC (permalink / raw)
  To: David Hildenbrand (Red Hat)
  Cc: SeongJae Park, Joshua Hahn, Andrew Morton, Muchun Song,
	Oscar Salvador, linux-kernel, linux-mm

On Thu, 15 Jan 2026 22:16:12 +0100 "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:

> On 1/15/26 22:14, Joshua Hahn wrote:
> > On Thu, 15 Jan 2026 21:10:41 +0100 "David Hildenbrand (Red Hat)" <david@kernel.org> wrote:
> > 
> >> On 1/15/26 19:14, Joshua Hahn wrote:
> >>> Documentation/process/coding-style.rst explicitly notes that if only one
> >>> branch of a conditional statement is a single statement, braces should
> >>> be used in both branches. Enforce this in mm/hugetlb.c.
> >>>
> >>> No functional change intended.
> >>>
> >>> Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> >>> ---
> >>
> >> [...]
> >>
> >>>    		update_and_free_hugetlb_folio(h, folio, false);
> >>>    		return rc;
> >>> @@ -2672,11 +2675,13 @@ void restore_reserve_on_error(struct hstate *h, struct vm_area_struct *vma,
> >>>    				 * be consumed on a subsequent allocation.
> >>>    				 */
> >>>    				folio_set_hugetlb_restore_reserve(folio);
> >>> -		} else
> >>> +		} else {
> >>>    			/*
> >>>    			 * No reservation present, do nothing
> >>>    			 */
> >>>    			 vma_end_reservation(h, vma, address);

                                ^ The space here seems unnecessary?

> >>> +
> >>
> >> But why the empty line? :)
> > 
> > Hello David,
> > 
> > Whoops, I totally forgot to run checkpatch on this one. I think Andrew
> > also noticed this, he sent a fixlet for it. Sorry for this! I'll fix it up
> > in v2.
> 
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>
> 
> with that ;)

Regardless of my finding, but assuming you will fix David's finding ;) please
feel free to add below to the v2.

Reviewed-by: SeongJae PArk <sj@kernel.org>


Thanks,
SJ

[...]


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition
  2026-01-16  1:39   ` SeongJae Park
@ 2026-01-16 16:45     ` Joshua Hahn
  0 siblings, 0 replies; 17+ messages in thread
From: Joshua Hahn @ 2026-01-16 16:45 UTC (permalink / raw)
  To: SeongJae Park
  Cc: Andrew Morton, David Hildenbrand, Muchun Song, Oscar Salvador,
	linux-kernel, linux-mm

On Thu, 15 Jan 2026 17:39:39 -0800 SeongJae Park <sj@kernel.org> wrote:

> On Thu, 15 Jan 2026 13:14:36 -0500 Joshua Hahn <joshua.hahnjy@gmail.com> wrote:
> 
> > if (map_chg) is always true, since it is nested in another if statement
> > which checks for it already. Remove the check and un-indent for readability.
> > 
> > if (unlikely(map_chg == MAP_CHG_NEEDED && retval == 0)) {
> > 	...
> > 
> > 	if (map_chg) {
> > 		...
> > 	}
> > }
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> 
> Reviewed-by: SeongJae Park <sj@kernel.org>

Thank you, SJ!

> > ---
> >  mm/hugetlb.c | 12 +++++-------
> >  1 file changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 88b9e997c9da..432a5054ca1d 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -3019,13 +3019,11 @@ struct folio *alloc_hugetlb_folio(struct vm_area_struct *vma,
> >  
> >  			rsv_adjust = hugepage_subpool_put_pages(spool, 1);
> >  			hugetlb_acct_memory(h, -rsv_adjust);
> > -			if (map_chg) {
> > -				spin_lock_irq(&hugetlb_lock);
> > -				hugetlb_cgroup_uncharge_folio_rsvd(
> > -				    hstate_index(h), pages_per_huge_page(h),
> > -				    folio);
> > -				spin_unlock_irq(&hugetlb_lock);
> > -			}
> > +			spin_lock_irq(&hugetlb_lock);
> > +			hugetlb_cgroup_uncharge_folio_rsvd(
> > +			    hstate_index(h), pages_per_huge_page(h),
> > +			    folio);
> 
> Nit.  Good chance to reduce one more line by putting 'folio' on the upper line?

Ack, will include in the next version of this : -) I hope you have a great day!
Joshua


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2026-01-16 16:45 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-01-15 18:14 [PATCH 0/3] mm/hugetlb: A fix and some cleanups Joshua Hahn
2026-01-15 18:14 ` [PATCH 1/3] mm/hugetlb: Restore failed global reservations to subpool Joshua Hahn
2026-01-15 19:19   ` Andrew Morton
2026-01-15 19:45     ` Joshua Hahn
2026-01-15 18:14 ` [PATCH 2/3] mm/hugetlb: Remove unnecessary if condition Joshua Hahn
2026-01-15 20:14   ` David Hildenbrand (Red Hat)
2026-01-15 21:10     ` Joshua Hahn
2026-01-15 21:59       ` Andrew Morton
2026-01-15 22:04         ` Joshua Hahn
2026-01-16  1:39   ` SeongJae Park
2026-01-16 16:45     ` Joshua Hahn
2026-01-15 18:14 ` [PATCH 3/3] mm/hugetlb: Enforce brace style Joshua Hahn
2026-01-15 20:10   ` David Hildenbrand (Red Hat)
2026-01-15 21:14     ` Joshua Hahn
2026-01-15 21:16       ` David Hildenbrand (Red Hat)
2026-01-16  1:54         ` SeongJae Park
2026-01-15 21:57       ` Andrew Morton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox