Module Name:    src
Committed By:   ad
Date:           Tue May 19 22:22:15 UTC 2020

Modified Files:
        src/sys/fs/tmpfs: tmpfs_vnops.c
        src/sys/uvm: uvm_aobj.c uvm_bio.c uvm_object.c uvm_pager.h uvm_vnode.c

Log Message:
PR kern/32166: pgo_get protocol is ambiguous
Also problems with tmpfs+nfs noted by hannken@.

Don't pass PGO_ALLPAGES to pgo_get, and ignore PGO_DONTCARE in the
!PGO_LOCKED case.  In uao_get() have uvm_pagealloc() take care of page
zeroing and release busy pages on error.


To generate a diff of this commit:
cvs rdiff -u -r1.140 -r1.141 src/sys/fs/tmpfs/tmpfs_vnops.c
cvs rdiff -u -r1.141 -r1.142 src/sys/uvm/uvm_aobj.c
cvs rdiff -u -r1.113 -r1.114 src/sys/uvm/uvm_bio.c
cvs rdiff -u -r1.21 -r1.22 src/sys/uvm/uvm_object.c
cvs rdiff -u -r1.48 -r1.49 src/sys/uvm/uvm_pager.h
cvs rdiff -u -r1.112 -r1.113 src/sys/uvm/uvm_vnode.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/fs/tmpfs/tmpfs_vnops.c
diff -u src/sys/fs/tmpfs/tmpfs_vnops.c:1.140 src/sys/fs/tmpfs/tmpfs_vnops.c:1.141
--- src/sys/fs/tmpfs/tmpfs_vnops.c:1.140	Sun May 17 19:43:31 2020
+++ src/sys/fs/tmpfs/tmpfs_vnops.c	Tue May 19 22:22:15 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: tmpfs_vnops.c,v 1.140 2020/05/17 19:43:31 ad Exp $	*/
+/*	$NetBSD: tmpfs_vnops.c,v 1.141 2020/05/19 22:22:15 ad Exp $	*/
 
 /*
  * Copyright (c) 2005, 2006, 2007, 2020 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.140 2020/05/17 19:43:31 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tmpfs_vnops.c,v 1.141 2020/05/19 22:22:15 ad Exp $");
 
 #include <sys/param.h>
 #include <sys/dirent.h>
@@ -1234,20 +1234,10 @@ tmpfs_getpages(void *v)
 		tmpfs_update_lazily(vp, tflags);
 	}
 
-	/*
-	 * Invoke the pager.
-	 *
-	 * Clean the array of pages before.  XXX: PR/32166
-	 * Note that vnode lock is shared with underlying UVM object.
-	 */
-	if ((flags & PGO_LOCKED) == 0 && pgs) {
-		memset(pgs, 0, sizeof(struct vm_pages *) * npages);
-	}
+	/* Invoke the pager.  The vnode vmobjlock is shared with the UAO. */
 	KASSERT(vp->v_uobj.vmobjlock == uobj->vmobjlock);
-
 	error = (*uobj->pgops->pgo_get)(uobj, offset, pgs, &npages, centeridx,
 	    access_type, advice, flags);
-
 #if defined(DEBUG)
 	if (!error && pgs) {
 		KASSERT(pgs[centeridx] != NULL);

Index: src/sys/uvm/uvm_aobj.c
diff -u src/sys/uvm/uvm_aobj.c:1.141 src/sys/uvm/uvm_aobj.c:1.142
--- src/sys/uvm/uvm_aobj.c:1.141	Sun May 17 19:38:17 2020
+++ src/sys/uvm/uvm_aobj.c	Tue May 19 22:22:15 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: uvm_aobj.c,v 1.141 2020/05/17 19:38:17 ad Exp $	*/
+/*	$NetBSD: uvm_aobj.c,v 1.142 2020/05/19 22:22:15 ad Exp $	*/
 
 /*
  * Copyright (c) 1998 Chuck Silvers, Charles D. Cranor and
@@ -38,7 +38,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_aobj.c,v 1.141 2020/05/17 19:38:17 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_aobj.c,v 1.142 2020/05/19 22:22:15 ad Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_uvmhist.h"
@@ -786,14 +786,12 @@ uao_put(struct uvm_object *uobj, voff_t 
  * 2: page is zero-fill    -> allocate a new page and zero it.
  * 3: page is swapped out  -> fetch the page from swap.
  *
- * cases 1 and 2 can be handled with PGO_LOCKED, case 3 cannot.
- * so, if the "center" page hits case 3 (or any page, with PGO_ALLPAGES),
- * then we will need to return EBUSY.
+ * case 1 can be handled with PGO_LOCKED, cases 2 and 3 cannot.
+ * so, if the "center" page hits case 2/3 then we will need to return EBUSY.
  *
  * => prefer map unlocked (not required)
  * => object must be locked!  we will _unlock_ it before starting any I/O.
- * => flags: PGO_ALLPAGES: get all of the pages
- *           PGO_LOCKED: fault data structures are locked
+ * => flags: PGO_LOCKED: fault data structures are locked
  * => NOTE: offset is the offset of pps[0], _NOT_ pps[centeridx]
  * => NOTE: caller must check for released pages!!
  */
@@ -805,7 +803,6 @@ uao_get(struct uvm_object *uobj, voff_t 
 	voff_t current_offset;
 	struct vm_page *ptmp = NULL;	/* Quell compiler warning */
 	int lcv, gotpages, maxpages, swslot, pageidx;
-	bool done;
 	UVMHIST_FUNC("uao_get"); UVMHIST_CALLED(pdhist);
 
 	UVMHIST_LOG(pdhist, "aobj=%#jx offset=%jd, flags=%jd",
@@ -841,7 +838,6 @@ uao_get(struct uvm_object *uobj, voff_t 
  		 */
 
 		uvm_page_array_init(&a);
-		done = true;	/* be optimistic */
 		gotpages = 0;	/* # of pages we got so far */
 		for (lcv = 0; lcv < maxpages; lcv++) {
 			ptmp = uvm_page_array_fill_and_peek(&a, uobj,
@@ -880,16 +876,9 @@ uao_get(struct uvm_object *uobj, voff_t 
 		 * to unlock and do some waiting or I/O.
  		 */
 
-		if ((flags & PGO_ALLPAGES) != 0) {
-			for (int i = 0; i < maxpages; i++) {
-				done &= (pps[i] != NULL);
-			}
-		} else {
-			done = (pps[centeridx] != NULL);
-		}
 		UVMHIST_LOG(pdhist, "<- done (done=%jd)", done, 0,0,0);
 		*npagesp = gotpages;
-		return done ? 0 : EBUSY;
+		return pps[centeridx] != NULL ? 0 : EBUSY;
 	}
 
 	/*
@@ -905,17 +894,6 @@ uao_get(struct uvm_object *uobj, voff_t 
 	    lcv++, current_offset += PAGE_SIZE) {
 
 		/*
-		 * - skip over pages we've already gotten or don't want
-		 * - skip over pages we don't _have_ to get
-		 */
-
-		if (pps[lcv] != NULL ||
-		    (lcv != centeridx && (flags & PGO_ALLPAGES) == 0))
-			continue;
-
-		pageidx = current_offset >> PAGE_SHIFT;
-
-		/*
  		 * we have yet to locate the current page (pps[lcv]).   we
 		 * first look for a page that is already at the current offset.
 		 * if we find a page, we check to see if it is busy or
@@ -926,20 +904,23 @@ uao_get(struct uvm_object *uobj, voff_t 
 		 * 'break's the following while loop and indicates we are
 		 * ready to move on to the next page in the "lcv" loop above.
  		 *
- 		 * if we exit the while loop with pps[lcv] still set to NULL,
+ 		 * if we exit the while loop with pps[lcv] set to NULL,
 		 * then it means that we allocated a new busy/fake/clean page
 		 * ptmp in the object and we need to do I/O to fill in the data.
  		 */
 
 		/* top of "pps" while loop */
-		while (pps[lcv] == NULL) {
+		for (;;) {
 			/* look for a resident page */
 			ptmp = uvm_pagelookup(uobj, current_offset);
 
 			/* not resident?   allocate one now (if we can) */
 			if (ptmp == NULL) {
-
-				ptmp = uao_pagealloc(uobj, current_offset, 0);
+				/* get a zeroed page if not in swap */
+				pageidx = current_offset >> PAGE_SHIFT;
+				swslot = uao_find_swslot(uobj, pageidx);
+				ptmp = uao_pagealloc(uobj, current_offset,
+				    swslot == 0 ? UVM_PGA_ZERO : 0);
 
 				/* out of RAM? */
 				if (ptmp == NULL) {
@@ -952,10 +933,11 @@ uao_get(struct uvm_object *uobj, voff_t 
 				}
 
 				/*
-				 * got new page ready for I/O.  break pps while
-				 * loop.  pps[lcv] is still NULL.
+				 * got new page ready for I/O.  break pps for
+				 * loop.
 				 */
 
+				pps[lcv] = NULL;
 				break;
 			}
 
@@ -982,6 +964,7 @@ uao_get(struct uvm_object *uobj, voff_t 
 			ptmp->flags |= PG_BUSY;
 			UVM_PAGE_OWN(ptmp, "uao_get2");
 			pps[lcv] = ptmp;
+			break;
 		}
 
 		/*
@@ -993,24 +976,12 @@ uao_get(struct uvm_object *uobj, voff_t 
 			continue;			/* next lcv */
 
 		/*
- 		 * we have a "fake/busy/clean" page that we just allocated.
- 		 * do the needed "i/o", either reading from swap or zeroing.
+ 		 * if swslot == 0, page hasn't existed before and is zeroed. 
+ 		 * otherwise we have a "fake/busy/clean" page that we just
+ 		 * allocated.  do the needed "i/o", reading from swap.
  		 */
 
-		swslot = uao_find_swslot(uobj, pageidx);
-
-		/*
- 		 * just zero the page if there's nothing in swap.
- 		 */
-
-		if (swslot == 0) {
-
-			/*
-			 * page hasn't existed before, just zero it.
-			 */
-
-			uvm_pagezero(ptmp);
-		} else {
+		if (swslot != 0) {
 #if defined(VMSWAP)
 			int error;
 
@@ -1049,6 +1020,12 @@ uao_get(struct uvm_object *uobj, voff_t 
 
 				uvm_pagefree(ptmp);
 				rw_exit(uobj->vmobjlock);
+				UVMHIST_LOG(pdhist, "<- done (error)",
+				    error,lcv,0,0);
+				if (lcv != 0) {
+					uvm_page_unbusy(pps, lcv);
+				}
+				memset(pps, 0, maxpages * sizeof(pps[0]));
 				return error;
 			}
 #else /* defined(VMSWAP) */

Index: src/sys/uvm/uvm_bio.c
diff -u src/sys/uvm/uvm_bio.c:1.113 src/sys/uvm/uvm_bio.c:1.114
--- src/sys/uvm/uvm_bio.c:1.113	Sun Apr 26 16:16:13 2020
+++ src/sys/uvm/uvm_bio.c	Tue May 19 22:22:15 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: uvm_bio.c,v 1.113 2020/04/26 16:16:13 thorpej Exp $	*/
+/*	$NetBSD: uvm_bio.c,v 1.114 2020/05/19 22:22:15 ad Exp $	*/
 
 /*
  * Copyright (c) 1998 Chuck Silvers.
@@ -34,7 +34,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_bio.c,v 1.113 2020/04/26 16:16:13 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_bio.c,v 1.114 2020/05/19 22:22:15 ad Exp $");
 
 #include "opt_uvmhist.h"
 #include "opt_ubc.h"
@@ -823,7 +823,7 @@ ubc_alloc_direct(struct uvm_object *uobj
 {
 	voff_t pgoff;
 	int error;
-	int gpflags = flags | PGO_NOTIMESTAMP | PGO_SYNCIO | PGO_ALLPAGES;
+	int gpflags = flags | PGO_NOTIMESTAMP | PGO_SYNCIO;
 	int access_type = VM_PROT_READ;
 	UVMHIST_FUNC("ubc_alloc_direct"); UVMHIST_CALLED(ubchist);
 

Index: src/sys/uvm/uvm_object.c
diff -u src/sys/uvm/uvm_object.c:1.21 src/sys/uvm/uvm_object.c:1.22
--- src/sys/uvm/uvm_object.c:1.21	Sun Feb 23 15:46:43 2020
+++ src/sys/uvm/uvm_object.c	Tue May 19 22:22:15 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: uvm_object.c,v 1.21 2020/02/23 15:46:43 ad Exp $	*/
+/*	$NetBSD: uvm_object.c,v 1.22 2020/05/19 22:22:15 ad Exp $	*/
 
 /*
  * Copyright (c) 2006, 2010, 2019 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_object.c,v 1.21 2020/02/23 15:46:43 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_object.c,v 1.22 2020/05/19 22:22:15 ad Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -146,7 +146,7 @@ uvm_obj_wirepages(struct uvm_object *uob
 		memset(pgs, 0, sizeof(pgs));
 		error = (*uobj->pgops->pgo_get)(uobj, offset, pgs, &npages, 0,
 			VM_PROT_READ | VM_PROT_WRITE, UVM_ADV_SEQUENTIAL,
-			PGO_ALLPAGES | PGO_SYNCIO);
+			PGO_SYNCIO);
 
 		if (error)
 			goto error;

Index: src/sys/uvm/uvm_pager.h
diff -u src/sys/uvm/uvm_pager.h:1.48 src/sys/uvm/uvm_pager.h:1.49
--- src/sys/uvm/uvm_pager.h:1.48	Sun May 17 19:38:17 2020
+++ src/sys/uvm/uvm_pager.h	Tue May 19 22:22:15 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: uvm_pager.h,v 1.48 2020/05/17 19:38:17 ad Exp $	*/
+/*	$NetBSD: uvm_pager.h,v 1.49 2020/05/19 22:22:15 ad Exp $	*/
 
 /*
  * Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -148,7 +148,7 @@ struct uvm_pagerops {
 #define PGO_FREE	0x008	/* free flushed pages */
 /* if PGO_FREE is not set then the pages stay where they are. */
 
-#define PGO_ALLPAGES	0x010	/* flush whole object/get all pages */
+#define PGO_ALLPAGES	0x010	/* flush whole object [put] */
 #define PGO_JOURNALLOCKED 0x020	/* journal is already locked [get/put] */
 #define PGO_LOCKED	0x040	/* fault data structures are locked [get] */
 #define PGO_BUSYFAIL	0x080	/* fail if a page is busy [put] */

Index: src/sys/uvm/uvm_vnode.c
diff -u src/sys/uvm/uvm_vnode.c:1.112 src/sys/uvm/uvm_vnode.c:1.113
--- src/sys/uvm/uvm_vnode.c:1.112	Tue May 19 21:45:57 2020
+++ src/sys/uvm/uvm_vnode.c	Tue May 19 22:22:15 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: uvm_vnode.c,v 1.112 2020/05/19 21:45:57 ad Exp $	*/
+/*	$NetBSD: uvm_vnode.c,v 1.113 2020/05/19 22:22:15 ad Exp $	*/
 
 /*
  * Copyright (c) 1997 Charles D. Cranor and Washington University.
@@ -45,7 +45,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: uvm_vnode.c,v 1.112 2020/05/19 21:45:57 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uvm_vnode.c,v 1.113 2020/05/19 22:22:15 ad Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_uvmhist.h"
@@ -161,8 +161,7 @@ uvn_put(struct uvm_object *uobj, voff_t 
  *
  * => prefer map unlocked (not required)
  * => object must be locked!  we will _unlock_ it before starting any I/O.
- * => flags: PGO_ALLPAGES: get all of the pages
- *           PGO_LOCKED: fault data structures are locked
+ * => flags: PGO_LOCKED: fault data structures are locked
  * => NOTE: offset is the offset of pps[0], _NOT_ pps[centeridx]
  * => NOTE: caller must check for released pages!!
  */

Reply via email to