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