Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: Boundary checks in uvm_pmr_getpages()
To:
mpi@grenadille.net <mpi@grenadille.net>
Cc:
tech@openbsd.org
Date:
Wed, 06 Nov 2024 11:19:55 +0100

Download raw body.

Thread
> From: mpi@grenadille.net <mpi@grenadille.net>
> 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?

ok kettenis@

> 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;
> 
> 
> 
>