Index | Thread | Search

From:
Martin Pieuchot <mpi@grenadille.net>
Subject:
Sync uao_get() with uvn_get()
To:
tech@openbsd.org
Date:
Tue, 26 Nov 2024 12:11:34 +0100

Download raw body.

Thread
uao_get() and uvn_get() are the two `pgo_get' implementations used by
the lower layer fault handler.  First they are called with PGO_LOCKED
to return resident pages, then if needed, they are called again with
PGO_SYNCIO to fetch content of the page from disk (or cache).

The comment for uao_get() explains the following:

 /*
  * uao_get: fetch me a page     
  *
  * we have three cases:
  * 1: page is resident     -> just return the page.
  * 2: page is zero-fill    -> allocate a new page and zero it.
  * 3: page is swapped out  -> fetch the page from swap.
  *
  * cases 1 can be handled with PGO_LOCKED, cases 2 and 3 cannot.
[...]
 */ 

However case 2 is currently optimistically handled with PGO_LOCKED, if
the NOWAIT allocation succeeds.

Diff below changes that and sync the PGO_LOCKED case with uvn_get() in
preparation for faulting ahead resident pages in parallel.

The explanation for this change is that PGO_LOCKED will be soon a read-
only operation, so we do not want any modification on the underlying UVM
object.

While here reduce some cosmetic differences in sync with NetBSD.

ok?

Index: uvm/uvm_aobj.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v
diff -u -p -r1.110 uvm_aobj.c
--- uvm/uvm_aobj.c	13 Apr 2024 23:44:11 -0000	1.110
+++ uvm/uvm_aobj.c	26 Nov 2024 10:57:01 -0000
@@ -1012,7 +1012,6 @@ uao_get(struct uvm_object *uobj, voff_t 
 		 * this if the data structures are locked (i.e. the first
 		 * time through).
  		 */
-
 		done = TRUE;	/* be optimistic */
 		gotpages = 0;	/* # of pages we got so far */
 
@@ -1022,35 +1021,17 @@ uao_get(struct uvm_object *uobj, voff_t 
 			if (pps[lcv] == PGO_DONTCARE)
 				continue;
 
+			/* lookup page */
 			ptmp = uvm_pagelookup(uobj, current_offset);
 
 			/*
- 			 * if page is new, attempt to allocate the page,
-			 * zero-fill'd.
- 			 */
-			if (ptmp == NULL && uao_find_swslot(uobj,
-			    current_offset >> PAGE_SHIFT) == 0) {
-				ptmp = uvm_pagealloc(uobj, current_offset,
-				    NULL, UVM_PGA_ZERO);
-				if (ptmp) {
-					/* new page */
-					atomic_clearbits_int(&ptmp->pg_flags,
-					    PG_BUSY|PG_FAKE);
-					atomic_setbits_int(&ptmp->pg_flags,
-					    PQ_AOBJ);
-					UVM_PAGE_OWN(ptmp, NULL);
-				}
-			}
-
-			/*
 			 * to be useful must get a non-busy page
 			 */
-			if (ptmp == NULL ||
-			    (ptmp->pg_flags & PG_BUSY) != 0) {
+			if (ptmp == NULL || (ptmp->pg_flags & PG_BUSY) != 0) {
 				if (lcv == centeridx ||
 				    (flags & PGO_ALLPAGES) != 0)
 					/* need to do a wait or I/O! */
-					done = FALSE;	
+					done = FALSE;
 				continue;
 			}
 
@@ -1061,7 +1042,6 @@ uao_get(struct uvm_object *uobj, voff_t 
 			UVM_PAGE_OWN(ptmp, "uao_get1");
 			pps[lcv] = ptmp;
 			gotpages++;
-
 		}
 
 		/*
@@ -1069,12 +1049,7 @@ uao_get(struct uvm_object *uobj, voff_t 
 		 * to unlock and do some waiting or I/O.
  		 */
 		*npagesp = gotpages;
-		if (done)
-			/* bingo! */
-			return VM_PAGER_OK;	
-		else
-			/* EEK!   Need to unlock and I/O */
-			return VM_PAGER_UNLOCK;
+		return done ? VM_PAGER_OK : VM_PAGER_UNLOCK;
 	}
 
 	/*
Index: uvm/uvm_vnode.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v
diff -u -p -r1.134 uvm_vnode.c
--- uvm/uvm_vnode.c	23 Oct 2024 07:18:44 -0000	1.134
+++ uvm/uvm_vnode.c	26 Nov 2024 10:57:01 -0000
@@ -970,7 +970,6 @@ uvn_get(struct uvm_object *uobj, voff_t 
 
 		for (lcv = 0, current_offset = offset ; lcv < *npagesp ;
 		    lcv++, current_offset += PAGE_SIZE) {
-
 			/* do we care about this page?  if not, skip it */
 			if (pps[lcv] == PGO_DONTCARE)
 				continue;
@@ -978,12 +977,14 @@ uvn_get(struct uvm_object *uobj, voff_t 
 			/* lookup page */
 			ptmp = uvm_pagelookup(uobj, current_offset);
 
-			/* to be useful must get a non-busy, non-released pg */
-			if (ptmp == NULL ||
-			    (ptmp->pg_flags & PG_BUSY) != 0) {
-				if (lcv == centeridx || (flags & PGO_ALLPAGES)
-				    != 0)
-					done = FALSE;	/* need to do a wait or I/O! */
+			/*
+			 * to be useful must get a non-busy page
+			 */
+			if (ptmp == NULL || (ptmp->pg_flags & PG_BUSY) != 0) {
+				if (lcv == centeridx ||
+				    (flags & PGO_ALLPAGES) != 0)
+					/* need to do a wait or I/O! */
+					done = FALSE;
 				continue;
 			}
 
@@ -1014,12 +1015,8 @@ uvn_get(struct uvm_object *uobj, voff_t 
 		 * step 1c: now we've either done everything needed or we to
 		 * unlock and do some waiting or I/O.
 		 */
-
 		*npagesp = gotpages;		/* let caller know */
-		if (done)
-			return VM_PAGER_OK;		/* bingo! */
-		else
-			return VM_PAGER_UNLOCK;
+		return done ? VM_PAGER_OK : VM_PAGER_UNLOCK;
 	}
 
 	/*