Re: [BUG] mm/hugetlb: possible temporary resv underflow

[Guillaume Morin <gemorin@debian.org>]

Hello Breno,

On 13 Jul  2:43, Breno Leitao wrote:
> On Fri, Jul 10, 2026 at 05:02:43PM +0200, Guillaume Morin wrote:
> > Hi,
> > 
> > We've encountered a temporary underflow in the reserved count after a fork when
> > a parent unmaps a faulted hugetlb page before a child process. 
> > 
> > After a reservation is consumed (i.e the page is faulted), if there is a fork()
> > and the parent munmaps the page before the child, the reserved count
> > underflows. The count is restored when the child unmaps the page/exits. I have
> > reproduced the issue in 6.12.95 and 6.18.38. I believe it's still present in
> > 7.2-rc2 but didn't try to reproduce.
> 
> Thanks for the detailed report and the reproducer.
> 
> I reproduced the underflow on current linux-next (7.2.0-rc2-next-20260710)
> with your program — it fires on every run. I also confirmed your
> folio_mapcount() == 0 check fixes it: with it applied the underflow is gone
> across many runs, and the existing hugetlb reservation selftests still pass.
> 
> > The patch that seems to have introduced the issue is df7a6d1f6405, "mm/hugetlb:
> > restore the reservation if needed"
> > 
> > I am not sure what the best fix is but checking folio_mapcount() == 0 
> > in __unmap_hugepage_range() seems to do the trick
> > 
> >                 adjust_reservation = false;
> >  
> >                 spin_lock_irq(&hugetlb_lock);
> >                 if (!h->surplus_huge_pages && __vma_private_lock(vma) &&
> > -                   folio_test_anon(folio)) {
> > +                   folio_test_anon(folio) &&
> > +                   folio_mapcount(folio) == 0) {
> >                         folio_set_hugetlb_restore_reserve(page_folio(page));
> >                         /* Reservation to be adjusted after the spin lock */
> >                         adjust_reservation = true;
> 
> The fix looks correct to me.

Excellent! Will you be submitting a patch or do you need me to do
something there?

> > Reproducer (it only works if HugePages_Rsvd is 0 at the beginning of the test)
> 
> Would you be up for turning your reproducer into an expansion of the selftest
> I've added a while ago for the fix I've added a while ago:
> 
>     tools/testing/selftests/mm/hugetlb_madv_vs_map.c
> 
> Your case is a nice complement to it, so it'd be great to have it there.

I've never used the selftest API but I gave it a shot. Hopefully it's a
decent first try

diff --git a/tools/testing/selftests/mm/hugepage_settings.c b/tools/testing/selftests/mm/hugepage_settings.c
index 2eab2110ac6a..02323389df39 100644
--- a/tools/testing/selftests/mm/hugepage_settings.c
+++ b/tools/testing/selftests/mm/hugepage_settings.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <ctype.h>
 #include <dirent.h>
 #include <fcntl.h>
 #include <limits.h>
@@ -449,6 +450,34 @@ unsigned long hugetlb_free_pages(unsigned long size)
 	return read_num(path);
 }
 
+bool hugetlb_nr_resv_pages(unsigned long *num)
+{
+	const char* field = "HugePages_Rsvd: ";
+	FILE *f = fopen("/proc/meminfo", "r");
+	char line[256];
+
+	if (!f) {
+		ksft_perror("Could not open /proc/meminfo");
+		return false;
+	}
+	while (fgets(line, sizeof(line), f)) {
+		if (strncmp(line, field, strlen(field)))
+			continue;
+		const char *p = line + strlen(field);
+		while (*p && isspace(*p))
+			++p;
+		if (!*p) {
+			break;
+		}
+		*num = strtoul(p, NULL, 10);
+		fclose(f);
+		return true;
+	}
+	fclose(f);
+	ksft_perror("could not parse /proc/meminfo");
+	return false;
+}
+
 static bool __hugetlb_setup(unsigned long size, unsigned long nr)
 {
 	unsigned long free = hugetlb_free_pages(size);
diff --git a/tools/testing/selftests/mm/hugepage_settings.h b/tools/testing/selftests/mm/hugepage_settings.h
index 726c73c43c05..a73146c8e4a4 100644
--- a/tools/testing/selftests/mm/hugepage_settings.h
+++ b/tools/testing/selftests/mm/hugepage_settings.h
@@ -98,6 +98,7 @@ unsigned long default_huge_page_size(void);
 unsigned long hugetlb_nr_pages(unsigned long size);
 void hugetlb_set_nr_pages(unsigned long size, unsigned long nr);
 unsigned long hugetlb_free_pages(unsigned long size);
+bool hugetlb_nr_resv_pages(unsigned long *num);
 
 static inline void hugetlb_save_settings(void)
 {
diff --git a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
index f94549efcc6f..f3ee3aa49322 100644
--- a/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
+++ b/tools/testing/selftests/mm/hugetlb_madv_vs_map.c
@@ -15,13 +15,20 @@
  *
  *  Touching the first page after thread3's allocation will raise a SIGBUS
  *
+ *  We setup a 2nd test where we create a child process, then unmap the page in
+ *  the parent while the child waits and verify that there is no underflow
+ *  of the reserved count.
+ *
  *  Author: Breno Leitao <leitao@debian.org>
  */
+#include <limits.h>
 #include <pthread.h>
+#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/mman.h>
 #include <sys/types.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
 #include "vm_util.h"
@@ -86,7 +93,7 @@ int main(void)
 	int max = 10;
 
 	ksft_print_header();
-	ksft_set_plan(1);
+	ksft_set_plan(3);
 
 	if (!hugetlb_setup_default_exact(1))
 		ksft_exit_skip("This test needs one and only one page to execute. Got %lu\n",
@@ -120,5 +127,88 @@ int main(void)
 	}
 
 	ksft_test_result_pass("No unexpected huge page allocations\n");
+	huge_ptr = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE,
+			MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
+
+	if ((unsigned long)huge_ptr == -1)
+		ksft_exit_fail_msg("Failed to allocate huge page\n");
+
+	{
+		pid_t pid;
+		int pipe_fds[2];
+		unsigned long nr_reserved = 0;
+		if (!hugetlb_nr_resv_pages(&nr_reserved))
+			ksft_exit_fail_msg("Cannot read number of reserved pages\n");
+		else if (nr_reserved != 1)
+			ksft_exit_fail_msg("Unexpected number of reserved pages: %lu, expected 1\n",
+					   nr_reserved);
+
+		/* Force the fault to ensure the reservation is consumed */
+		*huge_ptr = 0;
+		if (!hugetlb_nr_resv_pages(&nr_reserved))
+			ksft_exit_fail_msg("Cannot read number of reserved pages\n");
+		else if (nr_reserved != 0)
+			ksft_exit_fail_msg("Unexpected number of reserved pages: %lu, expected 0\n",
+					   nr_reserved);
+
+		if (pipe(pipe_fds) != 0) {
+			ksft_exit_fail_msg("pipe failed");
+		}
+
+		pid = fork();
+		if (pid < 0) {
+			ksft_exit_fail_msg("fork failed");
+		}
+
+		if (pid == 0) {
+			/* Child: Simply wait for the parent */
+			char b;
+			close(pipe_fds[1]);
+			if (read(pipe_fds[0], &b, 1) < 0) {
+				ksft_exit_fail_msg("child read failed");
+			}
+			return 0;
+		}
+
+		/* Parent */
+		close(pipe_fds[0]);
+
+		/* First unmap, this will close the vma */
+		if (munmap(huge_ptr, mmap_size) != 0) {
+			kill(pid, SIGKILL);
+			ksft_exit_fail_msg("munmap failed");
+		}
+
+		if (!hugetlb_nr_resv_pages(&nr_reserved))
+			ksft_exit_fail_msg("Cannot read number of reserved pages\n");
+		if (nr_reserved == ULONG_MAX) {
+			ksft_test_result_fail("After the munmap, HugePages_Rsvd underflowed!\n");
+		} else if (nr_reserved == 0) {
+			ksft_test_result_pass("Underflow not present!\n");
+		} else {
+			ksft_exit_fail_msg("Unexpected HugePages_Rsvd=%ld after munmap, should "
+					   "be 0 or -1. Repeat the test\n",
+					   nr_reserved);
+		}
+		/* Make the child exit, this should restore HugePages_Rsvd to 0 */
+		if (write(pipe_fds[1], &nr_reserved, 1) < 0) {
+			kill(pid, SIGKILL);
+			ksft_exit_fail_msg("write failed");
+		}
+		close(pipe_fds[1]);
+		if (waitpid(pid, NULL, 0) <= 0) {
+			ksft_exit_fail_msg("write failed");
+		}
+
+		if (!hugetlb_nr_resv_pages(&nr_reserved))
+			ksft_exit_fail_msg("Cannot read number of reserved pages\n");
+		if (nr_reserved == 0) {
+			ksft_test_result_pass("After the child dies, HugePages_Rsvd is properly set to 0\n");
+		} else {
+			ksft_exit_fail_msg("Unexpected HugePages_Rsvd=%ld after "
+					   "the child termination munmap, should be 0 or -1. "
+					   "Repeat the test\n", nr_reserved);
+		}
+	}
 	ksft_finished();
 }

-- 
Guillaume Morin <guillaume@morinfr.org>