Date: Fri, 17 Aug 2018 21:12:17 +0000 (UTC) From: Rick Macklem <rmacklem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r337990 - in head/sys/fs: nfs nfsserver Message-ID: <201808172112.w7HLCHoJ069203@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: rmacklem Date: Fri Aug 17 21:12:16 2018 New Revision: 337990 URL: https://svnweb.freebsd.org/changeset/base/337990 Log: Fix LORs between vn_start_write() and vn_lock() in the pNFS server. When coding the pNFS server, I added several vn_start_write() calls done while the vnode was locked, not realizing I had introduced LORs and possible deadlock when an exported file system on the MDS is suspended. This patch fixes this by removing the added vn_start_write() calls and modifying the code so that the extant vn_start_write() call before the NFS RPC/operation is done when needed by the pNFS server. Flags are changed so that LayoutCommit and LayoutReturn now get a vn_start_write() done for them. When the pNFS server is enabled, the code now also changes the flags for Getattr, so that the vn_start_write() is done for Getattr, since it may need to do a vn_set_extattr(). The nfs_writerpc flag array was made global to the NFS server and renamed nfsrv_writerpc, which is consistent naming for globals in the NFS server. Thanks go to kib@ for reporting that doing vn_start_write() while the vnode is locked results in a LOR. This patch only affects the behaviour of the pNFS server. Modified: head/sys/fs/nfs/nfs_commonsubs.c head/sys/fs/nfsserver/nfs_nfsdkrpc.c head/sys/fs/nfsserver/nfs_nfsdport.c head/sys/fs/nfsserver/nfs_nfsdsocket.c Modified: head/sys/fs/nfs/nfs_commonsubs.c ============================================================================== --- head/sys/fs/nfs/nfs_commonsubs.c Fri Aug 17 20:41:50 2018 (r337989) +++ head/sys/fs/nfs/nfs_commonsubs.c Fri Aug 17 21:12:16 2018 (r337990) @@ -156,9 +156,9 @@ struct nfsv4_opflag nfsv4_opflag[NFSV41_NOPS] = { { 0, 0, 0, 0, LK_EXCLUSIVE, 1, 1 }, /* Get Dir Deleg */ { 0, 0, 0, 0, LK_EXCLUSIVE, 1, 1 }, /* Get Device Info */ { 0, 0, 0, 0, LK_EXCLUSIVE, 1, 1 }, /* Get Device List */ - { 0, 1, 0, 0, LK_EXCLUSIVE, 1, 1 }, /* Layout Commit */ + { 0, 1, 0, 1, LK_EXCLUSIVE, 1, 1 }, /* Layout Commit */ { 0, 1, 0, 0, LK_EXCLUSIVE, 1, 1 }, /* Layout Get */ - { 0, 1, 0, 0, LK_EXCLUSIVE, 1, 0 }, /* Layout Return */ + { 0, 1, 0, 1, LK_EXCLUSIVE, 1, 0 }, /* Layout Return */ { 0, 0, 0, 0, LK_EXCLUSIVE, 1, 1 }, /* Secinfo No name */ { 0, 0, 0, 0, LK_EXCLUSIVE, 1, 0 }, /* Sequence */ { 0, 0, 0, 0, LK_EXCLUSIVE, 1, 1 }, /* Set SSV */ Modified: head/sys/fs/nfsserver/nfs_nfsdkrpc.c ============================================================================== --- head/sys/fs/nfsserver/nfs_nfsdkrpc.c Fri Aug 17 20:41:50 2018 (r337989) +++ head/sys/fs/nfsserver/nfs_nfsdkrpc.c Fri Aug 17 21:12:16 2018 (r337990) @@ -107,6 +107,9 @@ extern u_long sb_max_adj; extern int newnfs_numnfsd; extern struct proc *nfsd_master_proc; extern time_t nfsdev_time; +extern int nfsrv_writerpc[NFS_NPROCS]; +extern volatile int nfsrv_devidcnt; +extern struct nfsv4_opflag nfsv4_opflag[NFSV41_NOPS]; /* * NFS server system calls @@ -527,8 +530,21 @@ nfsrvd_nfsd(struct thread *td, struct nfsd_nfsd_args * nfsrvd_pool->sp_minthreads = args->minthreads; nfsrvd_pool->sp_maxthreads = args->maxthreads; + /* + * If this is a pNFS service, make Getattr do a + * vn_start_write(), so it can do a vn_set_extattr(). + */ + if (nfsrv_devidcnt > 0) { + nfsrv_writerpc[NFSPROC_GETATTR] = 1; + nfsv4_opflag[NFSV4OP_GETATTR].modifyfs = 1; + } + svc_run(nfsrvd_pool); + /* Reset Getattr to not do a vn_start_write(). */ + nfsrv_writerpc[NFSPROC_GETATTR] = 0; + nfsv4_opflag[NFSV4OP_GETATTR].modifyfs = 0; + if (principal[0] != '\0') { rpc_gss_clear_svc_name_call(NFS_PROG, NFS_VER2); rpc_gss_clear_svc_name_call(NFS_PROG, NFS_VER3); Modified: head/sys/fs/nfsserver/nfs_nfsdport.c ============================================================================== --- head/sys/fs/nfsserver/nfs_nfsdport.c Fri Aug 17 20:41:50 2018 (r337989) +++ head/sys/fs/nfsserver/nfs_nfsdport.c Fri Aug 17 21:12:16 2018 (r337990) @@ -128,7 +128,7 @@ static int nfsrv_getattrdsrpc(fhandle_t *, struct ucre static int nfsrv_putfhname(fhandle_t *, char *); static int nfsrv_pnfslookupds(struct vnode *, struct vnode *, struct pnfsdsfile *, struct vnode **, NFSPROC_T *); -static void nfsrv_pnfssetfh(struct vnode *, struct pnfsdsfile *, +static void nfsrv_pnfssetfh(struct vnode *, struct pnfsdsfile *, char *, char *, struct vnode *, NFSPROC_T *); static int nfsrv_dsremove(struct vnode *, char *, struct ucred *, NFSPROC_T *); static int nfsrv_dssetacl(struct vnode *, struct acl *, struct ucred *, @@ -4068,21 +4068,16 @@ nfsrv_pnfscreate(struct vnode *vp, struct vattr *vap, tpf->dsf_sin.sin_port = 0; } - error = vn_start_write(vp, &mp, V_WAIT); - if (error == 0) { + error = vn_extattr_set(vp, IO_NODELOCKED, + EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsfile", + sizeof(*pf) * nfsrv_maxpnfsmirror, (char *)pf, p); + if (error == 0) error = vn_extattr_set(vp, IO_NODELOCKED, - EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsfile", - sizeof(*pf) * nfsrv_maxpnfsmirror, (char *)pf, p); - if (error == 0) - error = vn_extattr_set(vp, IO_NODELOCKED, - EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsattr", - sizeof(dsattr), (char *)&dsattr, p); - vn_finished_write(mp); - if (error != 0) - printf("pNFS: pnfscreate setextattr=%d\n", - error); - } else - printf("pNFS: pnfscreate startwrite=%d\n", error); + EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsattr", + sizeof(dsattr), (char *)&dsattr, p); + if (error != 0) + printf("pNFS: pnfscreate setextattr=%d\n", + error); } else printf("pNFS: pnfscreate=%d\n", error); free(pf, M_TEMP); @@ -4415,6 +4410,9 @@ nfsrv_proxyds(struct nfsrv_descript *nd, struct vnode tryagain: if (error == 0) { buflen = 1024; + if (ioproc == NFSPROC_READDS && NFSVOPISLOCKED(vp) == + LK_EXCLUSIVE) + printf("nfsrv_proxyds: Readds vp exclusively locked\n"); error = nfsrv_dsgetsockmnt(vp, LK_SHARED, buf, &buflen, &mirrorcnt, p, dvp, fh, NULL, NULL, NULL, NULL, NULL, NULL, NULL); @@ -4673,6 +4671,8 @@ nfsrv_dsgetsockmnt(struct vnode *vp, int lktype, char if (fhiszero != 0) nfsrv_pnfssetfh( vp, pf, + devid, + fnamep, nvp, p); if (nvpp != NULL && *nvpp == NULL) { @@ -4746,21 +4746,15 @@ static int nfsrv_setextattr(struct vnode *vp, struct nfsvattr *nap, NFSPROC_T *p) { struct pnfsdsattr dsattr; - struct mount *mp; int error; ASSERT_VOP_ELOCKED(vp, "nfsrv_setextattr vp"); - error = vn_start_write(vp, &mp, V_WAIT); - if (error == 0) { - dsattr.dsa_filerev = nap->na_filerev; - dsattr.dsa_size = nap->na_size; - dsattr.dsa_atime = nap->na_atime; - dsattr.dsa_mtime = nap->na_mtime; - error = vn_extattr_set(vp, IO_NODELOCKED, - EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsattr", - sizeof(dsattr), (char *)&dsattr, p); - vn_finished_write(mp); - } + dsattr.dsa_filerev = nap->na_filerev; + dsattr.dsa_size = nap->na_size; + dsattr.dsa_atime = nap->na_atime; + dsattr.dsa_mtime = nap->na_mtime; + error = vn_extattr_set(vp, IO_NODELOCKED, EXTATTR_NAMESPACE_SYSTEM, + "pnfsd.dsattr", sizeof(dsattr), (char *)&dsattr, p); if (error != 0) printf("pNFS: setextattr=%d\n", error); return (error); @@ -5532,35 +5526,26 @@ nfsrv_pnfslookupds(struct vnode *vp, struct vnode *dvp * Set the file handle to the correct one. */ static void -nfsrv_pnfssetfh(struct vnode *vp, struct pnfsdsfile *pf, struct vnode *nvp, - NFSPROC_T *p) +nfsrv_pnfssetfh(struct vnode *vp, struct pnfsdsfile *pf, char *devid, + char *fnamep, struct vnode *nvp, NFSPROC_T *p) { - struct mount *mp; struct nfsnode *np; int ret; np = VTONFS(nvp); NFSBCOPY(np->n_fhp->nfh_fh, &pf->dsf_fh, NFSX_MYFH); /* - * We can only do a setextattr for an exclusively - * locked vp. Instead of trying to upgrade a shared - * lock, just leave dsf_fh zeroed out and it will - * keep doing this lookup until it is done with an - * exclusively locked vp. + * We can only do a vn_set_extattr() if the vnode is exclusively + * locked and vn_start_write() has been done. If devid != NULL or + * fnamep != NULL or the vnode is shared locked, vn_start_write() + * may not have been done. + * If not done now, it will be done on a future call. */ - if (NFSVOPISLOCKED(vp) == LK_EXCLUSIVE) { - ret = vn_start_write(vp, &mp, V_WAIT); - NFSD_DEBUG(4, "nfsrv_pnfssetfh: vn_start_write=%d\n", - ret); - if (ret == 0) { - ret = vn_extattr_set(vp, IO_NODELOCKED, - EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsfile", - sizeof(*pf), (char *)pf, p); - vn_finished_write(mp); - NFSD_DEBUG(4, "nfsrv_pnfslookupds: aft " - "vn_extattr_set=%d\n", ret); - } - } + if (devid == NULL && fnamep == NULL && NFSVOPISLOCKED(vp) == + LK_EXCLUSIVE) + ret = vn_extattr_set(vp, IO_NODELOCKED, + EXTATTR_NAMESPACE_SYSTEM, "pnfsd.dsfile", sizeof(*pf), + (char *)pf, p); NFSD_DEBUG(4, "eo nfsrv_pnfssetfh=%d\n", ret); } Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c ============================================================================== --- head/sys/fs/nfsserver/nfs_nfsdsocket.c Fri Aug 17 20:41:50 2018 (r337989) +++ head/sys/fs/nfsserver/nfs_nfsdsocket.c Fri Aug 17 21:12:16 2018 (r337990) @@ -361,7 +361,7 @@ static int nfsrv_nonidempotent[NFS_V3NPROCS] = { * This static array indicates whether or not the RPC modifies the * file system. */ -static int nfs_writerpc[NFS_NPROCS] = { 0, 0, 1, 0, 0, 0, 0, +int nfsrv_writerpc[NFS_NPROCS] = { 0, 0, 1, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1 }; @@ -517,10 +517,10 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u lktype = LK_EXCLUSIVE; if (nd->nd_flag & ND_PUBLOOKUP) nfsd_fhtovp(nd, &nfs_pubfh, lktype, &vp, &nes, - &mp, nfs_writerpc[nd->nd_procnum], p); + &mp, nfsrv_writerpc[nd->nd_procnum], p); else nfsd_fhtovp(nd, &fh, lktype, &vp, &nes, - &mp, nfs_writerpc[nd->nd_procnum], p); + &mp, nfsrv_writerpc[nd->nd_procnum], p); if (nd->nd_repstat == NFSERR_PROGNOTV4) goto out; } @@ -545,7 +545,7 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u nfsrvd_statstart(nfsv3to4op[nd->nd_procnum], /*now*/ NULL); nfsrvd_statend(nfsv3to4op[nd->nd_procnum], /*bytes*/ 0, /*now*/ NULL, /*then*/ NULL); - if (mp != NULL && nfs_writerpc[nd->nd_procnum] != 0) + if (mp != NULL && nfsrv_writerpc[nd->nd_procnum] != 0) vn_finished_write(mp); goto out; } @@ -576,7 +576,7 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u error = (*(nfsrv3_procs0[nd->nd_procnum]))(nd, isdgram, vp, p, &nes); } - if (mp != NULL && nfs_writerpc[nd->nd_procnum] != 0) + if (mp != NULL && nfsrv_writerpc[nd->nd_procnum] != 0) vn_finished_write(mp); nfsrvd_statend(nfsv3to4op[nd->nd_procnum], /*bytes*/ 0,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808172112.w7HLCHoJ069203>