[BUG] mm/hugetlb: possible temporary resv underflow

[BUG] mm/hugetlb: possible temporary resv underflow

[Guillaume Morin]

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.

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;


Reproducer (it only works if HugePages_Rsvd is 0 at the beginning of the test)

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>
#include <ctype.h>
#include <errno.h>
#include <limits.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/wait.h>

static unsigned long num_reserved()
{
	const char* field = "HugePages_Rsvd: ";
	FILE *f = fopen("/proc/meminfo", "r");
	char line[256];
	long v;

	if (!f) {
		perror("Could not open /proc/meminfo");
		exit(1);
	}
	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) {
			printf("could not parse /proc/meminfo line: %s", line);
			exit(1);
		}
		v = strtoul(p, NULL, 10);
		fclose(f);
		return v;
	}
	fclose(f);
	puts("could not parse /proc/meminfo line");
	exit(1);
}

int main(void)
{
	const size_t sz = 2048UL * 1024;
	int pipe_fds[2];
	pid_t pid;
	char *addr;
	long nr;

	nr = num_reserved();
	if (nr != 0) {
		puts("For the test, HugePages_Rsvd must be 0");
		return 1;
	}
	addr = mmap(NULL, sz, PROT_READ | PROT_WRITE,
		MAP_PRIVATE | MAP_ANONYMOUS | MAP_HUGETLB, -1, 0);
	if (addr == MAP_FAILED) {
		perror("Could not allocate a huge page");
		return 1;
	}
	if (num_reserved() != 1) {
		puts("After mmap, HugePages_Rsvd must be 1");
		return 1;
	}

	/* Fault */
	*addr = 0;
	if (num_reserved() != 0) {
		puts("After faulting, HugePages_Rsvd must be 0");
		return 1;
	}

	if (pipe(pipe_fds) != 0) {
		perror("pipe failed");
		return 1;
	}

	pid = fork();
	if (pid < 0) {
		perror("fork failed");
		return 1;
	}

	if (pid == 0) {
		/* Child: Simply wait for the parent */
		char b;
		close(pipe_fds[1]);
		if (read(pipe_fds[0], &b, 1) < 0) {
			puts("Child: read failed");
			return 1;
		}
		return 0;
	}

	/* Parent */
	close(pipe_fds[0]);

	/* First unmap, this will close the vma */
	if (munmap(addr, sz) != 0) {
		perror("munmap");
		kill(pid, SIGKILL);
		return 1;
	}

	nr = num_reserved();
	if (nr == ULONG_MAX) {
		puts("After the munmap, HugePages_Rsvd underflowed!");
	} else if (nr == 0) {
		puts("Underflow not present!");
	} else {
		printf("Unexpected HugePages_Rsvd=%ld after munmap, should be "
			"0 or -1. Repeat the test\n", nr);
	}
	/* Make the child exit, this should restore HugePages_Rsvd to 0 */
	if (write(pipe_fds[1], &nr, 1) < 0) {
		perror("write failed");
		kill(pid, SIGKILL);
		return 1;
	}
	close(pipe_fds[1]);
	if (waitpid(pid, NULL, 0) <= 0) {
		perror("waitpid failed");
		return 1;
	}

	nr = num_reserved();
	if (nr == 0) {
		puts("After the child dies, HugePages_Rsvd is properly "
		     "set to 0");
	} else {
		printf("Unexpected HugePages_Rsvd=%ld after the child "
		       "termination munmap, should be 0 or -1. "
		       "Repeat the test\n", nr);
	}

	return 0;
}

-- 
Guillaume Morin <guillaume@morinfr.org>