Module Name: src Committed By: christos Date: Sun Dec 1 00:59:34 UTC 2013
Modified Files: src/sys/kern: vfs_vnode.c src/sys/sys: vnode.h Log Message: Revert recent vnode changes per PR/48411, I still have deadlocks with build -j 20 on an 8 cpu machine. To generate a diff of this commit: cvs rdiff -u -r1.27 -r1.28 src/sys/kern/vfs_vnode.c cvs rdiff -u -r1.241 -r1.242 src/sys/sys/vnode.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/kern/vfs_vnode.c diff -u src/sys/kern/vfs_vnode.c:1.27 src/sys/kern/vfs_vnode.c:1.28 --- src/sys/kern/vfs_vnode.c:1.27 Fri Nov 29 09:58:55 2013 +++ src/sys/kern/vfs_vnode.c Sat Nov 30 19:59:34 2013 @@ -1,4 +1,4 @@ -/* $NetBSD: vfs_vnode.c,v 1.27 2013/11/29 14:58:55 hannken Exp $ */ +/* $NetBSD: vfs_vnode.c,v 1.28 2013/12/01 00:59:34 christos Exp $ */ /*- * Copyright (c) 1997-2011 The NetBSD Foundation, Inc. @@ -116,7 +116,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.27 2013/11/29 14:58:55 hannken Exp $"); +__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.28 2013/12/01 00:59:34 christos Exp $"); #define _VFS_VNODE_PRIVATE @@ -145,7 +145,6 @@ __KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c, /* Flags to vrelel. */ #define VRELEL_ASYNC_RELE 0x0001 /* Always defer to vrele thread. */ -#define VRELEL_CHANGING_SET 0x0002 /* VI_CHANGING set by caller. */ u_int numvnodes __cacheline_aligned; @@ -324,10 +323,8 @@ try_nextlist: * before doing this. */ vp->v_usecount = 1; - KASSERT((vp->v_iflag & VI_CHANGING) == 0); - vp->v_iflag |= VI_CHANGING; vclean(vp); - vrelel(vp, VRELEL_CHANGING_SET); + vrelel(vp, 0); fstrans_done(mp); return 0; @@ -479,10 +476,10 @@ vremfree(vnode_t *vp) * * => Should be called with v_interlock held. * - * If VI_CHANGING is set, the vnode may be eliminated in vgone()/vclean(). + * If VI_XLOCK is set, the vnode is being eliminated in vgone()/vclean(). * In that case, we cannot grab the vnode, so the process is awakened when * the transition is completed, and an error returned to indicate that the - * vnode is no longer usable. + * vnode is no longer usable (e.g. changed to a new file system type). */ int vget(vnode_t *vp, int flags) @@ -505,16 +502,31 @@ vget(vnode_t *vp, int flags) } /* - * If the vnode is in the process of changing state we wait - * for the change to complete and take care not to return - * a clean vnode. + * If the vnode is in the process of being cleaned out for + * another use, we wait for the cleaning to finish and then + * return failure. Cleaning is determined by checking if + * the VI_XLOCK flag is set. */ - if ((vp->v_iflag & VI_CHANGING) != 0) { + if ((vp->v_iflag & VI_XLOCK) != 0) { if ((flags & LK_NOWAIT) != 0) { vrelel(vp, 0); return EBUSY; } - vwait(vp, VI_CHANGING); + vwait(vp, VI_XLOCK); + vrelel(vp, 0); + return ENOENT; + } + + if ((vp->v_iflag & VI_INACTNOW) != 0) { + /* + * if it's being desactived, wait for it to complete. + * Make sure to not return a clean vnode. + */ + if ((flags & LK_NOWAIT) != 0) { + vrelel(vp, 0); + return EBUSY; + } + vwait(vp, VI_INACTNOW); if ((vp->v_iflag & VI_CLEAN) != 0) { vrelel(vp, 0); return ENOENT; @@ -593,11 +605,7 @@ vrelel(vnode_t *vp, int flags) * and unlock. */ if (vtryrele(vp)) { - if ((flags & VRELEL_CHANGING_SET) != 0) { - KASSERT((vp->v_iflag & VI_CHANGING) != 0); - vp->v_iflag &= ~VI_CHANGING; - cv_broadcast(&vp->v_cv); - } + vp->v_iflag |= VI_INACTREDO; mutex_exit(vp->v_interlock); return; } @@ -618,8 +626,10 @@ vrelel(vnode_t *vp, int flags) * If not clean, deactivate the vnode, but preserve * our reference across the call to VOP_INACTIVE(). */ +retry: if ((vp->v_iflag & VI_CLEAN) == 0) { recycle = false; + vp->v_iflag |= VI_INACTNOW; /* * XXX This ugly block can be largely eliminated if @@ -634,8 +644,11 @@ vrelel(vnode_t *vp, int flags) defer = true; } else if (curlwp == vrele_lwp) { /* - * We have to try harder. + * We have to try harder. But we can't sleep + * with VI_INACTNOW as vget() may be waiting on it. */ + vp->v_iflag &= ~(VI_INACTREDO|VI_INACTNOW); + cv_broadcast(&vp->v_cv); mutex_exit(vp->v_interlock); error = vn_lock(vp, LK_EXCLUSIVE); if (error != 0) { @@ -650,14 +663,11 @@ vrelel(vnode_t *vp, int flags) */ if (__predict_false(vtryrele(vp))) { VOP_UNLOCK(vp); - if ((flags & VRELEL_CHANGING_SET) != 0) { - KASSERT((vp->v_iflag & VI_CHANGING) != 0); - vp->v_iflag &= ~VI_CHANGING; - cv_broadcast(&vp->v_cv); - } mutex_exit(vp->v_interlock); return; } + vp->v_iflag |= VI_INACTNOW; + mutex_exit(vp->v_interlock); defer = false; } else if ((vp->v_iflag & VI_LAYER) != 0) { /* @@ -668,14 +678,15 @@ vrelel(vnode_t *vp, int flags) defer = true; } else { /* If we can't acquire the lock, then defer. */ + vp->v_iflag &= ~VI_INACTREDO; mutex_exit(vp->v_interlock); error = vn_lock(vp, LK_EXCLUSIVE | LK_NOWAIT); if (error != 0) { defer = true; + mutex_enter(vp->v_interlock); } else { defer = false; } - mutex_enter(vp->v_interlock); } if (defer) { @@ -684,26 +695,17 @@ vrelel(vnode_t *vp, int flags) * clean it here. We donate it our last reference. */ KASSERT(mutex_owned(vp->v_interlock)); - if ((flags & VRELEL_CHANGING_SET) != 0) { - KASSERT((vp->v_iflag & VI_CHANGING) != 0); - vp->v_iflag &= ~VI_CHANGING; - cv_broadcast(&vp->v_cv); - } + vp->v_iflag &= ~VI_INACTNOW; mutex_enter(&vrele_lock); TAILQ_INSERT_TAIL(&vrele_list, vp, v_freelist); if (++vrele_pending > (desiredvnodes >> 8)) cv_signal(&vrele_cv); mutex_exit(&vrele_lock); + cv_broadcast(&vp->v_cv); mutex_exit(vp->v_interlock); return; } - if ((flags & VRELEL_CHANGING_SET) == 0) { - KASSERT((vp->v_iflag & VI_CHANGING) == 0); - vp->v_iflag |= VI_CHANGING; - } - mutex_exit(vp->v_interlock); - /* * The vnode can gain another reference while being * deactivated. If VOP_INACTIVE() indicates that @@ -716,14 +718,21 @@ vrelel(vnode_t *vp, int flags) */ VOP_INACTIVE(vp, &recycle); mutex_enter(vp->v_interlock); + vp->v_iflag &= ~VI_INACTNOW; + cv_broadcast(&vp->v_cv); if (!recycle) { if (vtryrele(vp)) { - KASSERT((vp->v_iflag & VI_CHANGING) != 0); - vp->v_iflag &= ~VI_CHANGING; - cv_broadcast(&vp->v_cv); mutex_exit(vp->v_interlock); return; } + + /* + * If we grew another reference while + * VOP_INACTIVE() was underway, retry. + */ + if ((vp->v_iflag & VI_INACTREDO) != 0) { + goto retry; + } } /* Take care of space accounting. */ @@ -744,18 +753,10 @@ vrelel(vnode_t *vp, int flags) vclean(vp); } KASSERT(vp->v_usecount > 0); - } else { /* vnode was already clean */ - if ((flags & VRELEL_CHANGING_SET) == 0) { - KASSERT((vp->v_iflag & VI_CHANGING) == 0); - vp->v_iflag |= VI_CHANGING; - } } if (atomic_dec_uint_nv(&vp->v_usecount) != 0) { /* Gained another reference while being reclaimed. */ - KASSERT((vp->v_iflag & VI_CHANGING) != 0); - vp->v_iflag &= ~VI_CHANGING; - cv_broadcast(&vp->v_cv); mutex_exit(vp->v_interlock); return; } @@ -787,9 +788,6 @@ vrelel(vnode_t *vp, int flags) } TAILQ_INSERT_TAIL(vp->v_freelisthd, vp, v_freelist); mutex_exit(&vnode_free_list_lock); - KASSERT((vp->v_iflag & VI_CHANGING) != 0); - vp->v_iflag &= ~VI_CHANGING; - cv_broadcast(&vp->v_cv); mutex_exit(vp->v_interlock); } } @@ -800,7 +798,7 @@ vrele(vnode_t *vp) KASSERT((vp->v_iflag & VI_MARKER) == 0); - if (vtryrele(vp)) { + if ((vp->v_iflag & VI_INACTNOW) == 0 && vtryrele(vp)) { return; } mutex_enter(vp->v_interlock); @@ -816,7 +814,7 @@ vrele_async(vnode_t *vp) KASSERT((vp->v_iflag & VI_MARKER) == 0); - if (vtryrele(vp)) { + if ((vp->v_iflag & VI_INACTNOW) == 0 && vtryrele(vp)) { return; } mutex_enter(vp->v_interlock); @@ -1060,10 +1058,8 @@ vrecycle(vnode_t *vp, kmutex_t *inter_lk } vremfree(vp); vp->v_usecount = 1; - KASSERT((vp->v_iflag & VI_CHANGING) == 0); - vp->v_iflag |= VI_CHANGING; vclean(vp); - vrelel(vp, VRELEL_CHANGING_SET); + vrelel(vp, 0); return 1; } @@ -1086,8 +1082,8 @@ vrevoke(vnode_t *vp) return; } else if (vp->v_type != VBLK && vp->v_type != VCHR) { atomic_inc_uint(&vp->v_usecount); - mutex_exit(vp->v_interlock); - vgone(vp); + vclean(vp); + vrelel(vp, 0); return; } else { dev = vp->v_rdev; @@ -1096,7 +1092,9 @@ vrevoke(vnode_t *vp) } while (spec_node_lookup_by_dev(type, dev, &vq) == 0) { - vgone(vq); + mutex_enter(vq->v_interlock); + vclean(vq); + vrelel(vq, 0); } } @@ -1109,11 +1107,8 @@ vgone(vnode_t *vp) { mutex_enter(vp->v_interlock); - if ((vp->v_iflag & VI_CHANGING) != 0) - vwait(vp, VI_CHANGING); - vp->v_iflag |= VI_CHANGING; vclean(vp); - vrelel(vp, VRELEL_CHANGING_SET); + vrelel(vp, 0); } /* Index: src/sys/sys/vnode.h diff -u src/sys/sys/vnode.h:1.241 src/sys/sys/vnode.h:1.242 --- src/sys/sys/vnode.h:1.241 Sat Nov 23 08:46:22 2013 +++ src/sys/sys/vnode.h Sat Nov 30 19:59:34 2013 @@ -1,4 +1,4 @@ -/* $NetBSD: vnode.h,v 1.241 2013/11/23 13:46:22 hannken Exp $ */ +/* $NetBSD: vnode.h,v 1.242 2013/12/01 00:59:34 christos Exp $ */ /*- * Copyright (c) 2008 The NetBSD Foundation, Inc. @@ -206,7 +206,8 @@ typedef struct vnode vnode_t; #define VI_LOCKSHARE 0x00040000 /* v_interlock is shared */ #define VI_CLEAN 0x00080000 /* has been reclaimed */ #ifdef _VFS_VNODE_PRIVATE -#define VI_CHANGING 0x00100000 /* vnode changes state */ +#define VI_INACTREDO 0x00200000 /* need to redo VOP_INACTIVE() */ +#define VI_INACTNOW 0x00800000 /* VOP_INACTIVE() in progress */ #endif /* _VFS_VNODE_PRIVATE */ /* @@ -217,7 +218,8 @@ typedef struct vnode vnode_t; #define VNODE_FLAGBITS \ "\20\1ROOT\2SYSTEM\3ISTTY\4MAPPED\5MPSAFE\6LOCKSWORK\11TEXT\12EXECMAP" \ "\13WRMAP\14WRMAPDIRTY\15XLOCK\17ONWORKLST\20MARKER" \ - "\22LAYER\24CLEAN\25CHANGING\31DIROP" + "\22LAYER\24CLEAN\26INACTREDO" \ + "\30INACTNOW\31DIROP" #define VSIZENOTSET ((voff_t)-1)