From: Martin Pieuchot <mpi@grenadille.net>
Subject: Boundary checks in uvm_pmr_getpages()
To: tech@openbsd.org
Date: Tue, 5 Nov 2024 19:53:42 +0100

Diff below includes two changes related to OOM situations:

As soon as an allocation succeeds we know if we need to wakeup the page
daemon.  So move the check just after updating `uvmexp.free' which also
helps reasoning about locking.

Move the "retry" label, used when a caller wakes up after having slept
for physical pages, above the checks for reserved pages to ensure they
are not stolen.

ok?

Index: uvm/uvm_pmemrange.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_pmemrange.c,v
diff -u -p -r1.70 uvm_pmemrange.c
--- uvm/uvm_pmemrange.c	5 Nov 2024 17:28:32 -0000	1.70
+++ uvm/uvm_pmemrange.c	5 Nov 2024 18:15:10 -0000
@@ -955,18 +955,8 @@ uvm_pmr_getpages(psize_t count, paddr_t 
 	 */
 	desperate = 0;
 
-again:
 	uvm_lock_fpageq();
-
-	/*
-	 * check to see if we need to generate some free pages waking
-	 * the pagedaemon.
-	 */
-	if ((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freemin ||
-	    ((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg &&
-	    (uvmexp.inactive + BUFPAGES_INACT) < uvmexp.inactarg))
-		wakeup(&uvm.pagedaemon);
-
+retry:		/* Return point after sleeping. */
 	/*
 	 * fail if any of these conditions is true:
 	 * [1]  there really are no free pages, or
@@ -985,12 +975,12 @@ again:
 	    !in_pagedaemon(1)) {
 		if (flags & UVM_PLA_WAITOK) {
 			uvm_wait("uvm_pmr_getpages");
-			goto again;
+			uvm_lock_fpageq();
+			goto retry;
 		}
 		return ENOMEM;
 	}
 
-retry:		/* Return point after sleeping. */
 	fcount = 0;
 	fnsegs = 0;
 
@@ -1203,6 +1193,15 @@ fail:
 out:
 	/* Allocation successful. */
 	uvmexp.free -= fcount;
+
+	/*
+	 * check to see if we need to generate some free pages waking
+	 * the pagedaemon.
+	 */
+	if ((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freemin ||
+	    ((uvmexp.free - BUFPAGES_DEFICIT) < uvmexp.freetarg &&
+	    (uvmexp.inactive + BUFPAGES_INACT) < uvmexp.inactarg))
+		wakeup(&uvm.pagedaemon);
 
 	uvm_unlock_fpageq();