Source-Changes-HG archive

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

[src/trunk]: src/sys replace the earlier workaround for PR 40389 with a bette...



details:   https://anonhg.NetBSD.org/src/rev/cb9e21e09469
branches:  trunk
changeset: 757506:cb9e21e09469
user:      chs <chs%NetBSD.org@localhost>
date:      Wed Sep 01 16:56:19 2010 +0000

description:
replace the earlier workaround for PR 40389 with a better fix.
the earlier change caused data corruption by freeing pages
without invaliding their mappings.  instead of the trylock/retry,
just take the genfs-node lock before calling VOP_GETPAGES()
and pass a new flag to tell it that we're already holding this lock.

diffstat:

 sys/miscfs/genfs/genfs_io.c    |  46 +++++++++++++++++++++++++++++------------
 sys/miscfs/genfs/genfs_node.h  |   3 +-
 sys/miscfs/genfs/genfs_vnops.c |  12 +++++++++-
 sys/ufs/ufs/ufs_inode.c        |  29 ++++---------------------
 sys/uvm/uvm_pager.h            |   3 +-
 5 files changed, 51 insertions(+), 42 deletions(-)

diffs (258 lines):

diff -r bf6dd8cb3d7d -r cb9e21e09469 sys/miscfs/genfs/genfs_io.c
--- a/sys/miscfs/genfs/genfs_io.c       Wed Sep 01 16:48:00 2010 +0000
+++ b/sys/miscfs/genfs/genfs_io.c       Wed Sep 01 16:56:19 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: genfs_io.c,v 1.39 2010/08/19 02:10:02 pooka Exp $      */
+/*     $NetBSD: genfs_io.c,v 1.40 2010/09/01 16:56:19 chs Exp $        */
 
 /*
  * Copyright (c) 1982, 1986, 1989, 1993
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: genfs_io.c,v 1.39 2010/08/19 02:10:02 pooka Exp $");
+__KERNEL_RCSID(0, "$NetBSD: genfs_io.c,v 1.40 2010/09/01 16:56:19 chs Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -125,7 +125,6 @@
        int i, error, npages;
        const int flags = ap->a_flags;
        struct vnode * const vp = ap->a_vp;
-       struct genfs_node * const gp = VTOG(vp);
        struct uvm_object * const uobj = &vp->v_uobj;
        kauth_cred_t const cred = curlwp->l_cred;               /* XXXUBC curlwp */
        const bool async = (flags & PGO_SYNCIO) == 0;
@@ -133,6 +132,7 @@
        bool has_trans = false;
        const bool overwrite = (flags & PGO_OVERWRITE) != 0;
        const bool blockalloc = memwrite && (flags & PGO_NOBLOCKALLOC) == 0;
+       const bool glocked = (flags & PGO_GLOCKHELD) != 0;
        UVMHIST_FUNC("genfs_getpages"); UVMHIST_CALLED(ubchist);
 
        UVMHIST_LOG(ubchist, "vp %p off 0x%x/%x count %d",
@@ -211,6 +211,7 @@
                int nfound;
                struct vm_page *pg;
 
+               KASSERT(!glocked);
                npages = *ap->a_count;
 #if defined(DEBUG)
                for (i = 0; i < npages; i++) {
@@ -303,14 +304,19 @@
         * check if our idea of v_size is still valid.
         */
 
-       if (blockalloc) {
-               rw_enter(&gp->g_glock, RW_WRITER);
-       } else {
-               rw_enter(&gp->g_glock, RW_READER);
+       KASSERT(!glocked || genfs_node_wrlocked(vp));
+       if (!glocked) {
+               if (blockalloc) {
+                       genfs_node_wrlock(vp);
+               } else {
+                       genfs_node_rdlock(vp);
+               }
        }
        mutex_enter(&uobj->vmobjlock);
        if (vp->v_size < origvsize) {
-               genfs_node_unlock(vp);
+               if (!glocked) {
+                       genfs_node_unlock(vp);
+               }
                if (pgs != pgs_onstack)
                        kmem_free(pgs, pgs_size);
                goto startover;
@@ -318,7 +324,9 @@
 
        if (uvn_findpages(uobj, origoffset, &npages, &pgs[ridx],
            async ? UFP_NOWAIT : UFP_ALL) != orignmempages) {
-               genfs_node_unlock(vp);
+               if (!glocked) {
+                       genfs_node_unlock(vp);
+               }
                KASSERT(async != 0);
                genfs_rel_pages(&pgs[ridx], orignmempages);
                mutex_exit(&uobj->vmobjlock);
@@ -339,7 +347,9 @@
                }
        }
        if (i == npages) {
-               genfs_node_unlock(vp);
+               if (!glocked) {
+                       genfs_node_unlock(vp);
+               }
                UVMHIST_LOG(ubchist, "returning cached pages", 0,0,0,0);
                npages += ridx;
                goto out;
@@ -350,7 +360,9 @@
         */
 
        if (overwrite) {
-               genfs_node_unlock(vp);
+               if (!glocked) {
+                       genfs_node_unlock(vp);
+               }
                UVMHIST_LOG(ubchist, "PGO_OVERWRITE",0,0,0,0);
 
                for (i = 0; i < npages; i++) {
@@ -386,7 +398,9 @@
                npgs = npages;
                if (uvn_findpages(uobj, startoffset, &npgs, pgs,
                    async ? UFP_NOWAIT : UFP_ALL) != npages) {
-                       genfs_node_unlock(vp);
+                       if (!glocked) {
+                               genfs_node_unlock(vp);
+                       }
                        KASSERT(async != 0);
                        genfs_rel_pages(pgs, npages);
                        mutex_exit(&uobj->vmobjlock);
@@ -584,7 +598,9 @@
        nestiobuf_done(mbp, skipbytes, error);
        if (async) {
                UVMHIST_LOG(ubchist, "returning 0 (async)",0,0,0,0);
-               genfs_node_unlock(vp);
+               if (!glocked) {
+                       genfs_node_unlock(vp);
+               }
                error = 0;
                goto out_err_free;
        }
@@ -635,7 +651,9 @@
                        }
                }
        }
-       genfs_node_unlock(vp);
+       if (!glocked) {
+               genfs_node_unlock(vp);
+       }
 
        putiobuf(mbp);
     }
diff -r bf6dd8cb3d7d -r cb9e21e09469 sys/miscfs/genfs/genfs_node.h
--- a/sys/miscfs/genfs/genfs_node.h     Wed Sep 01 16:48:00 2010 +0000
+++ b/sys/miscfs/genfs/genfs_node.h     Wed Sep 01 16:56:19 2010 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: genfs_node.h,v 1.19 2010/01/27 15:52:31 uebayasi Exp $ */
+/* $NetBSD: genfs_node.h,v 1.20 2010/09/01 16:56:19 chs Exp $ */
 
 /*
  * Copyright (c) 2001 Chuck Silvers.
@@ -93,5 +93,6 @@
 void   genfs_node_rdlock(struct vnode *);
 int    genfs_node_rdtrylock(struct vnode *);
 void   genfs_node_unlock(struct vnode *);
+int    genfs_node_wrlocked(struct vnode *);
 
 #endif /* _MISCFS_GENFS_GENFS_NODE_H_ */
diff -r bf6dd8cb3d7d -r cb9e21e09469 sys/miscfs/genfs/genfs_vnops.c
--- a/sys/miscfs/genfs/genfs_vnops.c    Wed Sep 01 16:48:00 2010 +0000
+++ b/sys/miscfs/genfs/genfs_vnops.c    Wed Sep 01 16:56:19 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: genfs_vnops.c,v 1.182 2010/07/01 13:00:56 hannken Exp $        */
+/*     $NetBSD: genfs_vnops.c,v 1.183 2010/09/01 16:56:19 chs Exp $    */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -57,7 +57,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: genfs_vnops.c,v 1.182 2010/07/01 13:00:56 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: genfs_vnops.c,v 1.183 2010/09/01 16:56:19 chs Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -551,6 +551,14 @@
        rw_exit(&gp->g_glock);
 }
 
+int
+genfs_node_wrlocked(struct vnode *vp)
+{
+       struct genfs_node *gp = VTOG(vp);
+
+       return rw_write_held(&gp->g_glock);
+}
+
 /*
  * Do the usual access checking.
  * file_mode, uid and gid are from the vnode in question,
diff -r bf6dd8cb3d7d -r cb9e21e09469 sys/ufs/ufs/ufs_inode.c
--- a/sys/ufs/ufs/ufs_inode.c   Wed Sep 01 16:48:00 2010 +0000
+++ b/sys/ufs/ufs/ufs_inode.c   Wed Sep 01 16:56:19 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ufs_inode.c,v 1.82 2010/07/28 11:03:48 hannken Exp $   */
+/*     $NetBSD: ufs_inode.c,v 1.83 2010/09/01 16:56:19 chs Exp $       */
 
 /*
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ufs_inode.c,v 1.82 2010/07/28 11:03:48 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ufs_inode.c,v 1.83 2010/09/01 16:56:19 chs Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_ffs.h"
@@ -264,11 +264,11 @@
        off -= delta;
        len += delta;
 
- retry:
+       genfs_node_wrlock(vp);
        mutex_enter(&uobj->vmobjlock);
        error = VOP_GETPAGES(vp, pagestart, pgs, &npages, 0,
-           VM_PROT_WRITE, 0,
-           PGO_SYNCIO|PGO_PASTEOF|PGO_NOBLOCKALLOC|PGO_NOTIMESTAMP);
+           VM_PROT_WRITE, 0, PGO_SYNCIO | PGO_PASTEOF | PGO_NOBLOCKALLOC |
+           PGO_NOTIMESTAMP | PGO_GLOCKHELD);
        if (error) {
                goto out;
        }
@@ -287,25 +287,6 @@
         * now allocate the range.
         */
 
-       /*
-        * XXX: Hack around deadlock with pagebusy and genfs node lock.
-        *      This should be properly fixed.  PR kern/40389
-        */
-       {
-       struct genfs_node *gp = VTOG(vp); /* XXX */
-
-       if (!rw_tryenter(&gp->g_glock, RW_WRITER)) {
-               mutex_enter(&uobj->vmobjlock);
-               for (i = 0; i < npages; i++)
-                       pgs[i]->flags |= PG_RELEASED | PG_CLEAN;
-               mutex_enter(&uvm_pageqlock);
-               uvm_page_unbusy(pgs, npages);
-               mutex_exit(&uvm_pageqlock);
-               mutex_exit(&uobj->vmobjlock);
-               kpause("uballo", false, 1, NULL);
-               goto retry;
-       }}
-
        error = GOP_ALLOC(vp, off, len, flags, cred);
        genfs_node_unlock(vp);
 
diff -r bf6dd8cb3d7d -r cb9e21e09469 sys/uvm/uvm_pager.h
--- a/sys/uvm/uvm_pager.h       Wed Sep 01 16:48:00 2010 +0000
+++ b/sys/uvm/uvm_pager.h       Wed Sep 01 16:56:19 2010 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: uvm_pager.h,v 1.38 2008/08/22 10:48:22 hannken Exp $   */
+/*     $NetBSD: uvm_pager.h,v 1.39 2010/09/01 16:56:19 chs Exp $       */
 
 /*
  *
@@ -164,6 +164,7 @@
 #define PGO_NOBLOCKALLOC 0x800 /* backing block allocation is not needed */
 #define PGO_NOTIMESTAMP 0x1000 /* don't mark object accessed/modified */
 #define PGO_RECLAIM    0x2000  /* object is being reclaimed */
+#define PGO_GLOCKHELD  0x4000  /* genfs_node's lock is already held */
 
 /* page we are not interested in getting */
 #define PGO_DONTCARE ((struct vm_page *) -1L)  /* [get only] */



Home | Main Index | Thread Index | Old Index