From owner-svn-src-stable-11@freebsd.org Sun Sep 3 09:12:04 2017 Return-Path: Delivered-To: svn-src-stable-11@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 71AC2E015FA; Sun, 3 Sep 2017 09:12:04 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4EAAE63D84; Sun, 3 Sep 2017 09:12:04 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v839C3E4096951; Sun, 3 Sep 2017 09:12:03 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v839C2uX096945; Sun, 3 Sep 2017 09:12:02 GMT (envelope-from kib@FreeBSD.org) Message-Id: <201709030912.v839C2uX096945@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Sun, 3 Sep 2017 09:12:02 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r323142 - stable/11/sys/fs/nfsclient X-SVN-Group: stable-11 X-SVN-Commit-Author: kib X-SVN-Commit-Paths: stable/11/sys/fs/nfsclient X-SVN-Commit-Revision: 323142 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Sep 2017 09:12:04 -0000 Author: kib Date: Sun Sep 3 09:12:02 2017 New Revision: 323142 URL: https://svnweb.freebsd.org/changeset/base/323142 Log: MFC r322722: Do not drop NFS vnode lock when performing consistency checks. Modified: stable/11/sys/fs/nfsclient/nfs_clbio.c stable/11/sys/fs/nfsclient/nfs_clnode.c stable/11/sys/fs/nfsclient/nfs_clport.c stable/11/sys/fs/nfsclient/nfs_clsubs.c stable/11/sys/fs/nfsclient/nfs_clvnops.c stable/11/sys/fs/nfsclient/nfsnode.h Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/fs/nfsclient/nfs_clbio.c ============================================================================== --- stable/11/sys/fs/nfsclient/nfs_clbio.c Sun Sep 3 09:09:28 2017 (r323141) +++ stable/11/sys/fs/nfsclient/nfs_clbio.c Sun Sep 3 09:12:02 2017 (r323142) @@ -365,20 +365,13 @@ nfs_bioread_check_cons(struct vnode *vp, struct thread int error = 0; struct vattr vattr; struct nfsnode *np = VTONFS(vp); - int old_lock; + bool old_lock; /* - * Grab the exclusive lock before checking whether the cache is - * consistent. - * XXX - We can make this cheaper later (by acquiring cheaper locks). - * But for now, this suffices. + * Ensure the exclusove access to the node before checking + * whether the cache is consistent. */ - old_lock = ncl_upgrade_vnlock(vp); - if (vp->v_iflag & VI_DOOMED) { - error = EBADF; - goto out; - } - + old_lock = ncl_excl_start(vp); mtx_lock(&np->n_mtx); if (np->n_flag & NMODIFIED) { mtx_unlock(&np->n_mtx); @@ -386,9 +379,7 @@ nfs_bioread_check_cons(struct vnode *vp, struct thread if (vp->v_type != VDIR) panic("nfs: bioread, not dir"); ncl_invaldir(vp); - error = ncl_vinvalbuf(vp, V_SAVE, td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - error = EBADF; + error = ncl_vinvalbuf(vp, V_SAVE | V_ALLOWCLEAN, td, 1); if (error != 0) goto out; } @@ -404,16 +395,14 @@ nfs_bioread_check_cons(struct vnode *vp, struct thread mtx_unlock(&np->n_mtx); error = VOP_GETATTR(vp, &vattr, cred); if (error) - return (error); + goto out; mtx_lock(&np->n_mtx); if ((np->n_flag & NSIZECHANGED) || (NFS_TIMESPEC_COMPARE(&np->n_mtime, &vattr.va_mtime))) { mtx_unlock(&np->n_mtx); if (vp->v_type == VDIR) ncl_invaldir(vp); - error = ncl_vinvalbuf(vp, V_SAVE, td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - error = EBADF; + error = ncl_vinvalbuf(vp, V_SAVE | V_ALLOWCLEAN, td, 1); if (error != 0) goto out; mtx_lock(&np->n_mtx); @@ -423,7 +412,7 @@ nfs_bioread_check_cons(struct vnode *vp, struct thread mtx_unlock(&np->n_mtx); } out: - ncl_downgrade_vnlock(vp, old_lock); + ncl_excl_finish(vp, old_lock); return (error); } @@ -608,8 +597,6 @@ ncl_bioread(struct vnode *vp, struct uio *uio, int iof while (error == NFSERR_BAD_COOKIE) { ncl_invaldir(vp); error = ncl_vinvalbuf(vp, 0, td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - return (EBADF); /* * Yuck! The directory has been modified on the @@ -933,8 +920,6 @@ ncl_write(struct vop_write_args *ap) KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp); error = ncl_vinvalbuf(vp, V_SAVE | ((ioflag & IO_VMIO) != 0 ? V_VMIO : 0), td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - error = EBADF; if (error != 0) return (error); } else @@ -1016,9 +1001,6 @@ ncl_write(struct vop_write_args *ap) KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp); error = ncl_vinvalbuf(vp, V_SAVE | ((ioflag & IO_VMIO) != 0 ? V_VMIO : 0), td, 1); - if (error == 0 && - (vp->v_iflag & VI_DOOMED) != 0) - error = EBADF; if (error != 0) return (error); wouldcommit = biosize; @@ -1336,7 +1318,7 @@ ncl_vinvalbuf(struct vnode *vp, int flags, struct thre struct nfsnode *np = VTONFS(vp); struct nfsmount *nmp = VFSTONFS(vp->v_mount); int error = 0, slpflag, slptimeo; - int old_lock = 0; + bool old_lock; ASSERT_VOP_LOCKED(vp, "ncl_vinvalbuf"); @@ -1352,16 +1334,9 @@ ncl_vinvalbuf(struct vnode *vp, int flags, struct thre slptimeo = 0; } - old_lock = ncl_upgrade_vnlock(vp); - if (vp->v_iflag & VI_DOOMED) { - /* - * Since vgonel() uses the generic vinvalbuf() to flush - * dirty buffers and it does not call this function, it - * is safe to just return OK when VI_DOOMED is set. - */ - ncl_downgrade_vnlock(vp, old_lock); - return (0); - } + old_lock = ncl_excl_start(vp); + if (old_lock) + flags |= V_ALLOWCLEAN; /* * Now, flush as required. @@ -1400,7 +1375,7 @@ ncl_vinvalbuf(struct vnode *vp, int flags, struct thre np->n_flag &= ~NMODIFIED; mtx_unlock(&np->n_mtx); out: - ncl_downgrade_vnlock(vp, old_lock); + ncl_excl_finish(vp, old_lock); return error; } Modified: stable/11/sys/fs/nfsclient/nfs_clnode.c ============================================================================== --- stable/11/sys/fs/nfsclient/nfs_clnode.c Sun Sep 3 09:09:28 2017 (r323141) +++ stable/11/sys/fs/nfsclient/nfs_clnode.c Sun Sep 3 09:12:02 2017 (r323142) @@ -141,6 +141,9 @@ ncl_nget(struct mount *mntp, u_int8_t *fhp, int fhsize * happened to return an error no special casing is needed). */ mtx_init(&np->n_mtx, "NEWNFSnode lock", NULL, MTX_DEF | MTX_DUPOK); + lockinit(&np->n_excl, PVFS, "nfsupg", VLKTIMEOUT, LK_NOSHARE | + LK_CANRECURSE); + /* * NFS supports recursive and shared locking. */ @@ -167,6 +170,7 @@ ncl_nget(struct mount *mntp, u_int8_t *fhp, int fhsize *npp = NULL; FREE((caddr_t)np->n_fhp, M_NFSFH); mtx_destroy(&np->n_mtx); + lockdestroy(&np->n_excl); uma_zfree(newnfsnode_zone, np); return (error); } @@ -332,6 +336,7 @@ ncl_reclaim(struct vop_reclaim_args *ap) if (np->n_v4 != NULL) FREE((caddr_t)np->n_v4, M_NFSV4NODE); mtx_destroy(&np->n_mtx); + lockdestroy(&np->n_excl); uma_zfree(newnfsnode_zone, vp->v_data); vp->v_data = NULL; return (0); Modified: stable/11/sys/fs/nfsclient/nfs_clport.c ============================================================================== --- stable/11/sys/fs/nfsclient/nfs_clport.c Sun Sep 3 09:09:28 2017 (r323141) +++ stable/11/sys/fs/nfsclient/nfs_clport.c Sun Sep 3 09:12:02 2017 (r323142) @@ -230,6 +230,8 @@ nfscl_nget(struct mount *mntp, struct vnode *dvp, stru * happened to return an error no special casing is needed). */ mtx_init(&np->n_mtx, "NEWNFSnode lock", NULL, MTX_DEF | MTX_DUPOK); + lockinit(&np->n_excl, PVFS, "nfsupg", VLKTIMEOUT, LK_NOSHARE | + LK_CANRECURSE); /* * Are we getting the root? If so, make sure the vnode flags @@ -271,6 +273,7 @@ nfscl_nget(struct mount *mntp, struct vnode *dvp, stru if (error != 0) { *npp = NULL; mtx_destroy(&np->n_mtx); + lockdestroy(&np->n_excl); FREE((caddr_t)nfhp, M_NFSFH); if (np->n_v4 != NULL) FREE((caddr_t)np->n_v4, M_NFSV4NODE); Modified: stable/11/sys/fs/nfsclient/nfs_clsubs.c ============================================================================== --- stable/11/sys/fs/nfsclient/nfs_clsubs.c Sun Sep 3 09:09:28 2017 (r323141) +++ stable/11/sys/fs/nfsclient/nfs_clsubs.c Sun Sep 3 09:12:02 2017 (r323142) @@ -135,30 +135,33 @@ ncl_dircookie_unlock(struct nfsnode *np) mtx_unlock(&np->n_mtx); } -int -ncl_upgrade_vnlock(struct vnode *vp) +bool +ncl_excl_start(struct vnode *vp) { - int old_lock; + struct nfsnode *np; + int vn_lk; - ASSERT_VOP_LOCKED(vp, "ncl_upgrade_vnlock"); - old_lock = NFSVOPISLOCKED(vp); - if (old_lock != LK_EXCLUSIVE) { - KASSERT(old_lock == LK_SHARED, - ("ncl_upgrade_vnlock: wrong old_lock %d", old_lock)); - /* Upgrade to exclusive lock, this might block */ - NFSVOPLOCK(vp, LK_UPGRADE | LK_RETRY); - } - return (old_lock); + ASSERT_VOP_LOCKED(vp, "ncl_excl_start"); + vn_lk = NFSVOPISLOCKED(vp); + if (vn_lk == LK_EXCLUSIVE) + return (false); + KASSERT(vn_lk == LK_SHARED, + ("ncl_excl_start: wrong vnode lock %d", vn_lk)); + /* Ensure exclusive access, this might block */ + np = VTONFS(vp); + lockmgr(&np->n_excl, LK_EXCLUSIVE, NULL); + return (true); } void -ncl_downgrade_vnlock(struct vnode *vp, int old_lock) +ncl_excl_finish(struct vnode *vp, bool old_lock) { - if (old_lock != LK_EXCLUSIVE) { - KASSERT(old_lock == LK_SHARED, ("wrong old_lock %d", old_lock)); - /* Downgrade from exclusive lock. */ - NFSVOPLOCK(vp, LK_DOWNGRADE | LK_RETRY); - } + struct nfsnode *np; + + if (!old_lock) + return; + np = VTONFS(vp); + lockmgr(&np->n_excl, LK_RELEASE, NULL); } #ifdef NFS_ACDEBUG Modified: stable/11/sys/fs/nfsclient/nfs_clvnops.c ============================================================================== --- stable/11/sys/fs/nfsclient/nfs_clvnops.c Sun Sep 3 09:09:28 2017 (r323141) +++ stable/11/sys/fs/nfsclient/nfs_clvnops.c Sun Sep 3 09:12:02 2017 (r323142) @@ -522,8 +522,6 @@ nfs_open(struct vop_open_args *ap) if (np->n_flag & NMODIFIED) { mtx_unlock(&np->n_mtx); error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - return (EBADF); if (error == EINTR || error == EIO) { if (NFS_ISV4(vp)) (void) nfsrpc_close(vp, 0, ap->a_td); @@ -560,8 +558,6 @@ nfs_open(struct vop_open_args *ap) np->n_direofoffset = 0; mtx_unlock(&np->n_mtx); error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - return (EBADF); if (error == EINTR || error == EIO) { if (NFS_ISV4(vp)) (void) nfsrpc_close(vp, 0, ap->a_td); @@ -582,8 +578,6 @@ nfs_open(struct vop_open_args *ap) if (np->n_directio_opens == 0) { mtx_unlock(&np->n_mtx); error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - return (EBADF); if (error) { if (NFS_ISV4(vp)) (void) nfsrpc_close(vp, 0, ap->a_td); @@ -724,8 +718,6 @@ nfs_close(struct vop_close_args *ap) } } else { error = ncl_vinvalbuf(vp, V_SAVE, ap->a_td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - return (EBADF); } mtx_lock(&np->n_mtx); } @@ -944,9 +936,7 @@ nfs_setattr(struct vop_setattr_args *ap) mtx_unlock(&np->n_mtx); error = ncl_vinvalbuf(vp, vap->va_size == 0 ? 0 : V_SAVE, td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - error = EBADF; - if (error != 0) { + if (error != 0) { vnode_pager_setsize(vp, tsize); return (error); } @@ -973,8 +963,6 @@ nfs_setattr(struct vop_setattr_args *ap) (np->n_flag & NMODIFIED) && vp->v_type == VREG) { mtx_unlock(&np->n_mtx); error = ncl_vinvalbuf(vp, V_SAVE, td, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - return (EBADF); if (error == EINTR || error == EIO) return (error); } else @@ -1678,9 +1666,7 @@ nfs_remove(struct vop_remove_args *ap) * unnecessary delayed writes later. */ error = ncl_vinvalbuf(vp, 0, cnp->cn_thread, 1); - if (error == 0 && (vp->v_iflag & VI_DOOMED) != 0) - error = EBADF; - else if (error != EINTR && error != EIO) + if (error != EINTR && error != EIO) /* Do the rpc */ error = nfs_removerpc(dvp, vp, cnp->cn_nameptr, cnp->cn_namelen, cnp->cn_cred, cnp->cn_thread); @@ -3091,10 +3077,6 @@ nfs_advlock(struct vop_advlock_args *ap) if ((np->n_flag & NMODIFIED) || ret || np->n_change != va.va_filerev) { (void) ncl_vinvalbuf(vp, V_SAVE, td, 1); - if ((vp->v_iflag & VI_DOOMED) != 0) { - NFSVOPUNLOCK(vp, 0); - return (EBADF); - } np->n_attrstamp = 0; KDTRACE_NFS_ATTRCACHE_FLUSH_DONE(vp); ret = VOP_GETATTR(vp, &va, cred); Modified: stable/11/sys/fs/nfsclient/nfsnode.h ============================================================================== --- stable/11/sys/fs/nfsclient/nfsnode.h Sun Sep 3 09:09:28 2017 (r323141) +++ stable/11/sys/fs/nfsclient/nfsnode.h Sun Sep 3 09:12:02 2017 (r323142) @@ -92,6 +92,8 @@ struct nfs_accesscache { */ struct nfsnode { struct mtx n_mtx; /* Protects all of these members */ + struct lock n_excl; /* Exclusive helper for shared + vnode lock */ u_quad_t n_size; /* Current size of file */ u_quad_t n_brev; /* Modify rev when cached */ u_quad_t n_lrev; /* Modify rev for lease */ @@ -184,8 +186,8 @@ int ncl_removeit(struct sillyrename *, struct vnode *) int ncl_nget(struct mount *, u_int8_t *, int, struct nfsnode **, int); nfsuint64 *ncl_getcookie(struct nfsnode *, off_t, int); void ncl_invaldir(struct vnode *); -int ncl_upgrade_vnlock(struct vnode *); -void ncl_downgrade_vnlock(struct vnode *, int); +bool ncl_excl_start(struct vnode *); +void ncl_excl_finish(struct vnode *, bool old_lock); void ncl_dircookie_lock(struct nfsnode *); void ncl_dircookie_unlock(struct nfsnode *);