Source-Changes-HG archive

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

[src/trunk]: src/sys Locking a layer vnode is racy as it may become reclaimed...



details:   https://anonhg.NetBSD.org/src/rev/9339c2ce15a1
branches:  trunk
changeset: 822653:9339c2ce15a1
user:      hannken <hannken%NetBSD.org@localhost>
date:      Thu Mar 30 09:16:52 2017 +0000

description:
Locking a layer vnode is racy as it may become reclaimed before
calling the operation on the lower vnode.

Replace vi_lock with a rw_obj and change layered file systems
to share the lock with the lower vnode.

Layered file systems now use genfs_lock()/_unlock/_islocked().

Welcome to 7.99.67

diffstat:

 sys/kern/vfs_vnode.c               |  23 +++++++++++++++---
 sys/miscfs/genfs/genfs_vnops.c     |  32 +++++++++++++-------------
 sys/miscfs/genfs/layer_extern.h    |   6 +++-
 sys/miscfs/genfs/layer_vfsops.c    |   7 ++++-
 sys/miscfs/genfs/layer_vnops.c     |  46 +------------------------------------
 sys/miscfs/nullfs/null_vnops.c     |   6 +++-
 sys/miscfs/overlay/overlay_vnops.c |   8 ++++--
 sys/miscfs/umapfs/umap_vnops.c     |   6 +++-
 sys/sys/param.h                    |   4 +-
 sys/sys/vnode.h                    |   3 +-
 sys/sys/vnode_impl.h               |   4 +-
 11 files changed, 65 insertions(+), 80 deletions(-)

diffs (truncated from 442 to 300 lines):

diff -r 46e3b058b1c4 -r 9339c2ce15a1 sys/kern/vfs_vnode.c
--- a/sys/kern/vfs_vnode.c      Thu Mar 30 09:15:51 2017 +0000
+++ b/sys/kern/vfs_vnode.c      Thu Mar 30 09:16:52 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: vfs_vnode.c,v 1.80 2017/03/30 09:15:51 hannken Exp $   */
+/*     $NetBSD: vfs_vnode.c,v 1.81 2017/03/30 09:16:52 hannken Exp $   */
 
 /*-
  * Copyright (c) 1997-2011 The NetBSD Foundation, Inc.
@@ -156,7 +156,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.80 2017/03/30 09:15:51 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.81 2017/03/30 09:16:52 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/kernel.h>
@@ -416,6 +416,21 @@
 }
 
 /*
+ * Set vnode to share another vnodes lock.
+ */
+void
+vshare_lock(vnode_t *vp, vnode_t *src_vp)
+{
+       vnode_impl_t *vip = VNODE_TO_VIMPL(vp);
+       vnode_impl_t *src_vip = VNODE_TO_VIMPL(src_vp);
+       krwlock_t *oldlock = vip->vi_lock;
+
+       rw_obj_hold(src_vip->vi_lock);
+       vip->vi_lock = src_vip->vi_lock;
+       rw_obj_free(oldlock);
+}
+
+/*
  * Return the lru list this node should be on.
  */
 static vnodelst_t *
@@ -1066,7 +1081,7 @@
        vip = pool_cache_get(vcache_pool, PR_WAITOK);
        memset(vip, 0, sizeof(*vip));
 
-       rw_init(&vip->vi_lock);
+       vip->vi_lock = rw_obj_alloc();
        /* SLIST_INIT(&vip->vi_hash); */
        /* LIST_INIT(&vip->vi_nclist); */
        /* LIST_INIT(&vip->vi_dnclist); */
@@ -1128,7 +1143,7 @@
        if (vp->v_type == VBLK || vp->v_type == VCHR)
                spec_node_destroy(vp);
 
-       rw_destroy(&vip->vi_lock);
+       rw_obj_free(vip->vi_lock);
        uvm_obj_destroy(&vp->v_uobj, true);
        cv_destroy(&vp->v_cv);
        pool_cache_put(vcache_pool, vip);
diff -r 46e3b058b1c4 -r 9339c2ce15a1 sys/miscfs/genfs/genfs_vnops.c
--- a/sys/miscfs/genfs/genfs_vnops.c    Thu Mar 30 09:15:51 2017 +0000
+++ b/sys/miscfs/genfs/genfs_vnops.c    Thu Mar 30 09:16:52 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: genfs_vnops.c,v 1.193 2017/01/11 09:08:59 hannken Exp $        */
+/*     $NetBSD: genfs_vnops.c,v 1.194 2017/03/30 09:16:52 hannken 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.193 2017/01/11 09:08:59 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: genfs_vnops.c,v 1.194 2017/03/30 09:16:52 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -295,7 +295,7 @@
 
        op = (ISSET(flags, LK_EXCLUSIVE) ? RW_WRITER : RW_READER);
        if (ISSET(flags, LK_NOWAIT)) {
-               if (! rw_tryenter(&vip->vi_lock, op))
+               if (! rw_tryenter(vip->vi_lock, op))
                        return EBUSY;
                if (mutex_tryenter(vp->v_interlock)) {
                        error = vdead_check(vp, VDEAD_NOWAIT);
@@ -305,25 +305,25 @@
                } else
                        error = EBUSY;
                if (error)
-                       rw_exit(&vip->vi_lock);
+                       rw_exit(vip->vi_lock);
                return error;
        }
 
-       rw_enter(&vip->vi_lock, op);
+       rw_enter(vip->vi_lock, op);
        mutex_enter(vp->v_interlock);
        error = vdead_check(vp, VDEAD_NOWAIT);
        if (error == EBUSY) {
-               rw_exit(&vip->vi_lock);
+               rw_exit(vip->vi_lock);
                error = vdead_check(vp, 0);
                KASSERT(error == ENOENT);
                mutex_exit(vp->v_interlock);
-               rw_enter(&vip->vi_lock, op);
+               rw_enter(vip->vi_lock, op);
                mutex_enter(vp->v_interlock);
        }
        KASSERT(error == ENOENT);
        mutex_exit(vp->v_interlock);
        if (! ISSET(flags, LK_RETRY)) {
-               rw_exit(&vip->vi_lock);
+               rw_exit(vip->vi_lock);
                return ENOENT;
        }
        return 0;
@@ -341,7 +341,7 @@
        vnode_t *vp = ap->a_vp;
        vnode_impl_t *vip = VNODE_TO_VIMPL(vp);
 
-       rw_exit(&vip->vi_lock);
+       rw_exit(vip->vi_lock);
 
        return 0;
 }
@@ -367,7 +367,7 @@
        if (ISSET(flags, LK_NOWAIT)) {
                if (fstrans_start_nowait(mp, FSTRANS_SHARED))
                        return EBUSY;
-               if (! rw_tryenter(&vip->vi_lock, op)) {
+               if (! rw_tryenter(vip->vi_lock, op)) {
                        fstrans_done(mp);
                        return EBUSY;
                }
@@ -377,18 +377,18 @@
                } else
                        error = EBUSY;
                if (error) {
-                       rw_exit(&vip->vi_lock);
+                       rw_exit(vip->vi_lock);
                        fstrans_done(mp);
                }
                return error;
        }
 
        fstrans_start(mp, FSTRANS_SHARED);
-       rw_enter(&vip->vi_lock, op);
+       rw_enter(vip->vi_lock, op);
        mutex_enter(vp->v_interlock);
        error = vdead_check(vp, VDEAD_NOWAIT);
        if (error) {
-               rw_exit(&vip->vi_lock);
+               rw_exit(vip->vi_lock);
                fstrans_done(mp);
                error = vdead_check(vp, 0);
                KASSERT(error == ENOENT);
@@ -410,7 +410,7 @@
        vnode_impl_t *vip = VNODE_TO_VIMPL(vp);
        struct mount *mp = vp->v_mount;
 
-       rw_exit(&vip->vi_lock);
+       rw_exit(vip->vi_lock);
        fstrans_done(mp);
 
        return 0;
@@ -428,10 +428,10 @@
        vnode_t *vp = ap->a_vp;
        vnode_impl_t *vip = VNODE_TO_VIMPL(vp);
 
-       if (rw_write_held(&vip->vi_lock))
+       if (rw_write_held(vip->vi_lock))
                return LK_EXCLUSIVE;
 
-       if (rw_read_held(&vip->vi_lock))
+       if (rw_read_held(vip->vi_lock))
                return LK_SHARED;
 
        return 0;
diff -r 46e3b058b1c4 -r 9339c2ce15a1 sys/miscfs/genfs/layer_extern.h
--- a/sys/miscfs/genfs/layer_extern.h   Thu Mar 30 09:15:51 2017 +0000
+++ b/sys/miscfs/genfs/layer_extern.h   Thu Mar 30 09:16:52 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: layer_extern.h,v 1.38 2017/02/17 08:31:25 hannken Exp $        */
+/*     $NetBSD: layer_extern.h,v 1.39 2017/03/30 09:16:52 hannken Exp $        */
 
 /*
  * Copyright (c) 1999 National Aeronautics & Space Administration
@@ -99,7 +99,6 @@
 int    layer_getattr(void *);
 int    layer_inactive(void *);
 int    layer_reclaim(void *);
-int    layer_lock(void *);
 int    layer_print(void *);
 int    layer_bmap(void *);
 int    layer_fsync(void *);
@@ -114,5 +113,8 @@
 int    layer_rmdir(void *);
 int    layer_getpages(void *);
 int    layer_putpages(void *);
+#define layer_lock     genfs_lock
+#define layer_unlock   genfs_unlock
+#define layer_islocked genfs_islocked
 
 #endif /* _MISCFS_GENFS_LAYER_EXTERN_H_ */
diff -r 46e3b058b1c4 -r 9339c2ce15a1 sys/miscfs/genfs/layer_vfsops.c
--- a/sys/miscfs/genfs/layer_vfsops.c   Thu Mar 30 09:15:51 2017 +0000
+++ b/sys/miscfs/genfs/layer_vfsops.c   Thu Mar 30 09:16:52 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: layer_vfsops.c,v 1.47 2017/02/17 08:31:25 hannken Exp $        */
+/*     $NetBSD: layer_vfsops.c,v 1.48 2017/03/30 09:16:52 hannken Exp $        */
 
 /*
  * Copyright (c) 1999 National Aeronautics & Space Administration
@@ -74,7 +74,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: layer_vfsops.c,v 1.47 2017/02/17 08:31:25 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: layer_vfsops.c,v 1.48 2017/03/30 09:16:52 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/sysctl.h>
@@ -223,6 +223,9 @@
        mutex_obj_hold(lowervp->v_interlock);
        uvm_obj_setlock(&vp->v_uobj, lowervp->v_interlock);
 
+       /* Share the lock with the lower node. */
+       vshare_lock(vp, lowervp);
+
        vp->v_tag = lmp->layerm_tag;
        vp->v_type = lowervp->v_type;
        vp->v_op = lmp->layerm_vnodeop_p;
diff -r 46e3b058b1c4 -r 9339c2ce15a1 sys/miscfs/genfs/layer_vnops.c
--- a/sys/miscfs/genfs/layer_vnops.c    Thu Mar 30 09:15:51 2017 +0000
+++ b/sys/miscfs/genfs/layer_vnops.c    Thu Mar 30 09:16:52 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: layer_vnops.c,v 1.60 2017/01/27 10:47:13 hannken Exp $ */
+/*     $NetBSD: layer_vnops.c,v 1.61 2017/03/30 09:16:52 hannken Exp $ */
 
 /*
  * Copyright (c) 1999 National Aeronautics & Space Administration
@@ -170,7 +170,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: layer_vnops.c,v 1.60 2017/01/27 10:47:13 hannken Exp $");
+__KERNEL_RCSID(0, "$NetBSD: layer_vnops.c,v 1.61 2017/03/30 09:16:52 hannken Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -743,48 +743,6 @@
        return 0;
 }
 
-int
-layer_lock(void *v)
-{
-       struct vop_lock_args /* {
-               struct vnode *a_vp;
-               int a_flags;
-       } */ *ap = v;
-       struct vnode *vp = ap->a_vp;
-       struct vnode *lowervp = LAYERVPTOLOWERVP(vp);
-       int flags = ap->a_flags;
-       int error;
-
-       if (ISSET(flags, LK_NOWAIT)) {
-               error = VOP_LOCK(lowervp, flags);
-               if (error)
-                       return error;
-               if (mutex_tryenter(vp->v_interlock)) {
-                       error = vdead_check(vp, VDEAD_NOWAIT);
-                       mutex_exit(vp->v_interlock);
-               } else
-                       error = EBUSY;
-               if (error)
-                       VOP_UNLOCK(lowervp);
-               return error;
-       }
-
-       error = VOP_LOCK(lowervp, flags);
-       if (error)
-               return error;
-
-       mutex_enter(vp->v_interlock);
-       error = vdead_check(vp, VDEAD_NOWAIT);
-       if (error) {
-               VOP_UNLOCK(lowervp);
-               error = vdead_check(vp, 0);
-               KASSERT(error == ENOENT);
-       }
-       mutex_exit(vp->v_interlock);
-
-       return error;
-}
-
 /*
  * We just feed the returned vnode up to the caller - there's no need
  * to build a layer node on top of the node on which we're going to do
diff -r 46e3b058b1c4 -r 9339c2ce15a1 sys/miscfs/nullfs/null_vnops.c



Home | Main Index | Thread Index | Old Index