Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/uvm PR kern/32166: pgo_get protocol is ambiguous



details:   https://anonhg.NetBSD.org/src/rev/77158a1fc0b0
branches:  trunk
changeset: 933157:77158a1fc0b0
user:      ad <ad%NetBSD.org@localhost>
date:      Tue May 19 22:22:15 2020 +0000

description:
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.

diffstat:

 sys/fs/tmpfs/tmpfs_vnops.c |  16 +-------
 sys/uvm/uvm_aobj.c         |  77 ++++++++++++++++-----------------------------
 sys/uvm/uvm_bio.c          |   6 +-
 sys/uvm/uvm_object.c       |   6 +-
 sys/uvm/uvm_pager.h        |   4 +-
 sys/uvm/uvm_vnode.c        |   7 +--
 6 files changed, 41 insertions(+), 75 deletions(-)

diffs (truncated from 320 to 300 lines):

diff -r a89899f55995 -r 77158a1fc0b0 sys/fs/tmpfs/tmpfs_vnops.c
--- a/sys/fs/tmpfs/tmpfs_vnops.c        Tue May 19 21:57:25 2020 +0000
+++ b/sys/fs/tmpfs/tmpfs_vnops.c        Tue May 19 22:22:15 2020 +0000
@@ -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_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);
diff -r a89899f55995 -r 77158a1fc0b0 sys/uvm/uvm_aobj.c
--- a/sys/uvm/uvm_aobj.c        Tue May 19 21:57:25 2020 +0000
+++ b/sys/uvm/uvm_aobj.c        Tue May 19 22:22:15 2020 +0000
@@ -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 @@
  * 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 @@
        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 @@
                 */
 
                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 @@
                 * 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 @@
            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 @@
                 * '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 @@
                                }
 
                                /*
-                                * 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 @@
                        ptmp->flags |= PG_BUSY;
                        UVM_PAGE_OWN(ptmp, "uao_get2");
                        pps[lcv] = ptmp;
+                       break;
                }
 
                /*
@@ -993,24 +976,12 @@
                        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 @@
 
                                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) */
diff -r a89899f55995 -r 77158a1fc0b0 sys/uvm/uvm_bio.c
--- a/sys/uvm/uvm_bio.c Tue May 19 21:57:25 2020 +0000
+++ b/sys/uvm/uvm_bio.c Tue May 19 22:22:15 2020 +0000
@@ -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 @@
 {
        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);
 
diff -r a89899f55995 -r 77158a1fc0b0 sys/uvm/uvm_object.c
--- a/sys/uvm/uvm_object.c      Tue May 19 21:57:25 2020 +0000
+++ b/sys/uvm/uvm_object.c      Tue May 19 22:22:15 2020 +0000
@@ -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 @@
                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;
diff -r a89899f55995 -r 77158a1fc0b0 sys/uvm/uvm_pager.h
--- a/sys/uvm/uvm_pager.h       Tue May 19 21:57:25 2020 +0000
+++ b/sys/uvm/uvm_pager.h       Tue May 19 22:22:15 2020 +0000
@@ -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 @@
 #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] */
diff -r a89899f55995 -r 77158a1fc0b0 sys/uvm/uvm_vnode.c
--- a/sys/uvm/uvm_vnode.c       Tue May 19 21:57:25 2020 +0000
+++ b/sys/uvm/uvm_vnode.c       Tue May 19 22:22:15 2020 +0000
@@ -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 $       */
 
 /*



Home | Main Index | Thread Index | Old Index