Index | Thread | Search

From:
mpi@grenadille.net <mpi@grenadille.net>
Subject:
Re: Boundary checks in uvm_pmr_getpages()
To:
tech@openbsd.org
Date:
Wed, 06 Nov 2024 10:48:03 +0100

Download raw body.

Thread
On 05/11/24(Tue) 19:53, Martin Pieuchot wrote:
> 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.

I've been pointed off-list that moving the boundary check is incorrect.
So there's a diff that just change the label used after sleeping such
that the reserves are not stolen.

ok?

Index: uvm/uvm_pmemrange.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_pmemrange.c,v
diff -u -p -r1.72 uvm_pmemrange.c
--- uvm/uvm_pmemrange.c	5 Nov 2024 18:35:14 -0000	1.72
+++ uvm/uvm_pmemrange.c	6 Nov 2024 09:05:39 -0000
@@ -955,9 +955,8 @@ uvm_pmr_getpages(psize_t count, paddr_t  	 */
  	desperate = 0;
-again:
  	uvm_lock_fpageq();
-
+retry:		/* Return point after sleeping. */
  	/*
  	 * check to see if we need to generate some free pages waking
  	 * the pagedaemon.
@@ -986,12 +985,12 @@ again:
  	    	uvm_unlock_fpageq();
  		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;