From owner-svn-src-stable-8@FreeBSD.ORG Thu Dec 30 09:45:27 2010 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 03B66106564A; Thu, 30 Dec 2010 09:45:27 +0000 (UTC) (envelope-from pjd@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id E50648FC17; Thu, 30 Dec 2010 09:45:26 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id oBU9jQO0029338; Thu, 30 Dec 2010 09:45:26 GMT (envelope-from pjd@svn.freebsd.org) Received: (from pjd@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id oBU9jQXG029334; Thu, 30 Dec 2010 09:45:26 GMT (envelope-from pjd@svn.freebsd.org) Message-Id: <201012300945.oBU9jQXG029334@svn.freebsd.org> From: Pawel Jakub Dawidek Date: Thu, 30 Dec 2010 09:45:26 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r216815 - stable/8/sys/nfsserver X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Dec 2010 09:45:27 -0000 Author: pjd Date: Thu Dec 30 09:45:26 2010 New Revision: 216815 URL: http://svn.freebsd.org/changeset/base/216815 Log: MFC r216565,r216631,r216632,r216633,r216774: r216565: Reduce lock scope a little. r216631: On error, unbusy file system and jump to the end, so we won't try to unlock NULL *vpp. Reviewed by: kib r216632: - Move pubflag and lockflag handling from nfsrv_fhtovp() to nfs_namei() - this is the only place that is different from all the other nfsrv_fhtovp() consumers. This simplifies nfsrv_fhtovp() a bit and also eliminates one vn_lock/VOP_UNLOCK() cycle in case of NFSv3. - Implement NFSRV_FLAG_BUSY flag for nfsrv_fhtovp() that tells it to leave mount point busy. Reviewed by: kib r216633: Use newly added NFSRV_FLAG_BUSY flag for nfsrv_fhtovp() to keep mount point busy. This fixes a race where we can pass invalid mount point to VFS_VGET() via vp->v_mount when exported file system was forcibly unmounted between nfsrv_fhtovp() and VFS_VGET(). Reviewed by: kib r216774: ZFS might not return monotonically increasing directory offset cookies, so turn off UFS-specific hack that assumes so in ZFS case. Before the change we can miss returning some directory entries to a NFS client. I believe that the hack should be moved to ufs_readdir(), but until we find somebody who will do it, turn it off for ZFS in NFS server code. Submitted by: rmacklem Discussed with: rmacklem, mckusick Modified: stable/8/sys/nfsserver/nfs.h stable/8/sys/nfsserver/nfs_serv.c stable/8/sys/nfsserver/nfs_srvsubs.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) Modified: stable/8/sys/nfsserver/nfs.h ============================================================================== --- stable/8/sys/nfsserver/nfs.h Thu Dec 30 09:32:39 2010 (r216814) +++ stable/8/sys/nfsserver/nfs.h Thu Dec 30 09:45:26 2010 (r216815) @@ -239,6 +239,12 @@ extern int nfs_debug; #endif +/* + * The following flags can be passed to nfsrv_fhtovp() function. + */ +/* Leave file system busy on success. */ +#define NFSRV_FLAG_BUSY 0x01 + struct mbuf *nfs_rephead(int, struct nfsrv_descript *, int, struct mbuf **, caddr_t *); void nfsm_srvfattr(struct nfsrv_descript *, struct vattr *, @@ -264,7 +270,7 @@ int nfsrv_create(struct nfsrv_descript * struct mbuf **mrq); int nfsrv_fhtovp(fhandle_t *, int, struct vnode **, int *, struct nfsrv_descript *, struct nfssvc_sock *, struct sockaddr *, - int *, int); + int *); int nfsrv_setpublicfs(struct mount *, struct netexport *, struct export_args *); int nfs_ispublicfh(fhandle_t *); Modified: stable/8/sys/nfsserver/nfs_serv.c ============================================================================== --- stable/8/sys/nfsserver/nfs_serv.c Thu Dec 30 09:32:39 2010 (r216814) +++ stable/8/sys/nfsserver/nfs_serv.c Thu Dec 30 09:45:26 2010 (r216815) @@ -213,8 +213,7 @@ nfsrv3_access(struct nfsrv_descript *nfs fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); tl = nfsm_dissect_nonblock(u_int32_t *, NFSX_UNSIGNED); - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (error) { nfsm_reply(NFSX_UNSIGNED); nfsm_srvpostop_attr(1, NULL); @@ -280,8 +279,7 @@ nfsrv_getattr(struct nfsrv_descript *nfs vfslocked = 0; fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, nam, - &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (error) { nfsm_reply(0); error = 0; @@ -389,8 +387,7 @@ nfsrv_setattr(struct nfsrv_descript *nfs /* * Now that we have all the fields, lets do it. */ - error = nfsrv_fhtovp(fhp, 1, &vp, &tvfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &tvfslocked, nfsd, slp, nam, &rdonly); vfslocked = nfsrv_lockedpair(vfslocked, tvfslocked); if (error) { nfsm_reply(2 * NFSX_UNSIGNED); @@ -712,8 +709,7 @@ nfsrv_readlink(struct nfsrv_descript *nf uiop->uio_rw = UIO_READ; uiop->uio_segflg = UIO_SYSSPACE; uiop->uio_td = NULL; - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (error) { nfsm_reply(2 * NFSX_UNSIGNED); if (v3) @@ -808,8 +804,7 @@ nfsrv_read(struct nfsrv_descript *nfsd, * as well. */ - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (error) { vp = NULL; nfsm_reply(2 * NFSX_UNSIGNED); @@ -1109,8 +1104,7 @@ nfsrv_write(struct nfsrv_descript *nfsd, error = 0; goto nfsmout; } - error = nfsrv_fhtovp(fhp, 1, &vp, &tvfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &tvfslocked, nfsd, slp, nam, &rdonly); vfslocked = nfsrv_lockedpair(vfslocked, tvfslocked); if (error) { vp = NULL; @@ -2102,8 +2096,7 @@ nfsrv_link(struct nfsrv_descript *nfsd, nfsm_srvmtofh(dfhp); nfsm_srvnamesiz(len); - error = nfsrv_fhtovp(fhp, TRUE, &vp, &tvfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &tvfslocked, nfsd, slp, nam, &rdonly); vfslocked = nfsrv_lockedpair(vfslocked, tvfslocked); if (error) { nfsm_reply(NFSX_POSTOPATTR(v3) + NFSX_WCCDATA(v3)); @@ -2744,7 +2737,7 @@ nfsrv_readdir(struct nfsrv_descript *nfs int v3 = (nfsd->nd_flag & ND_NFSV3); u_quad_t off, toff, verf; u_long *cookies = NULL, *cookiep; /* needs to be int64_t or off_t */ - int vfslocked; + int vfslocked, not_zfs; nfsdbprintf(("%s %d\n", __FILE__, __LINE__)); vfslocked = 0; @@ -2770,8 +2763,7 @@ nfsrv_readdir(struct nfsrv_descript *nfs if (siz > xfer) siz = xfer; fullsiz = siz; - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (!error && vp->v_type != VDIR) { error = ENOTDIR; vput(vp); @@ -2809,6 +2801,7 @@ nfsrv_readdir(struct nfsrv_descript *nfs error = 0; goto nfsmout; } + not_zfs = strcmp(vp->v_mount->mnt_vfc->vfc_name, "zfs") != 0; VOP_UNLOCK(vp, 0); /* @@ -2826,11 +2819,11 @@ again: io.uio_rw = UIO_READ; io.uio_td = NULL; eofflag = 0; - vn_lock(vp, LK_SHARED | LK_RETRY); if (cookies) { free((caddr_t)cookies, M_TEMP); cookies = NULL; } + vn_lock(vp, LK_SHARED | LK_RETRY); error = VOP_READDIR(vp, &io, cred, &eofflag, &ncookies, &cookies); off = (off_t)io.uio_offset; if (!cookies && !error) @@ -2895,10 +2888,12 @@ again: * skip over the records that precede the requested offset. This * requires the assumption that file offset cookies monotonically * increase. + * Since the offset cookies don't monotonically increase for ZFS, + * this is not done when ZFS is the file system. */ while (cpos < cend && ncookies > 0 && (dp->d_fileno == 0 || dp->d_type == DT_WHT || - ((u_quad_t)(*cookiep)) <= toff)) { + (not_zfs != 0 && ((u_quad_t)(*cookiep)) <= toff))) { cpos += dp->d_reclen; dp = (struct dirent *)cpos; cookiep++; @@ -3044,9 +3039,12 @@ nfsrv_readdirplus(struct nfsrv_descript int v3 = (nfsd->nd_flag & ND_NFSV3); int usevget = 1, vfslocked; struct componentname cn; + struct mount *mntp = NULL; + int not_zfs; nfsdbprintf(("%s %d\n", __FILE__, __LINE__)); vfslocked = 0; + vp_locked = 0; if (!v3) panic("nfsrv_readdirplus: v3 proc called on a v2 connection"); fhp = &nfh.fh_generic; @@ -3066,14 +3064,17 @@ nfsrv_readdirplus(struct nfsrv_descript if (siz > xfer) siz = xfer; fullsiz = siz; - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); - vp_locked = 1; - if (!error && vp->v_type != VDIR) { - error = ENOTDIR; - vput(vp); - vp = NULL; - vp_locked = 0; + error = nfsrv_fhtovp(fhp, NFSRV_FLAG_BUSY, &vp, &vfslocked, nfsd, slp, + nam, &rdonly); + if (!error) { + vp_locked = 1; + mntp = vp->v_mount; + if (vp->v_type != VDIR) { + error = ENOTDIR; + vput(vp); + vp = NULL; + vp_locked = 0; + } } if (error) { nfsm_reply(NFSX_UNSIGNED); @@ -3100,6 +3101,7 @@ nfsrv_readdirplus(struct nfsrv_descript error = 0; goto nfsmout; } + not_zfs = strcmp(vp->v_mount->mnt_vfc->vfc_name, "zfs") != 0; VOP_UNLOCK(vp, 0); vp_locked = 0; rbuf = malloc(siz, M_TEMP, M_WAITOK); @@ -3114,12 +3116,12 @@ again: io.uio_rw = UIO_READ; io.uio_td = NULL; eofflag = 0; - vn_lock(vp, LK_SHARED | LK_RETRY); vp_locked = 1; if (cookies) { free((caddr_t)cookies, M_TEMP); cookies = NULL; } + vn_lock(vp, LK_SHARED | LK_RETRY); error = VOP_READDIR(vp, &io, cred, &eofflag, &ncookies, &cookies); off = (u_quad_t)io.uio_offset; getret = VOP_GETATTR(vp, &at, cred); @@ -3179,10 +3181,12 @@ again: * skip over the records that precede the requested offset. This * requires the assumption that file offset cookies monotonically * increase. + * Since the offset cookies don't monotonically increase for ZFS, + * this is not done when ZFS is the file system. */ while (cpos < cend && ncookies > 0 && (dp->d_fileno == 0 || dp->d_type == DT_WHT || - ((u_quad_t)(*cookiep)) <= toff)) { + (not_zfs != 0 && ((u_quad_t)(*cookiep)) <= toff))) { cpos += dp->d_reclen; dp = (struct dirent *)cpos; cookiep++; @@ -3215,8 +3219,8 @@ again: * For readdir_and_lookup get the vnode using * the file number. */ - error = VFS_VGET(vp->v_mount, dp->d_fileno, - LK_SHARED, &nvp); + error = VFS_VGET(mntp, dp->d_fileno, LK_SHARED, + &nvp); if (error != 0 && error != EOPNOTSUPP) { error = 0; goto invalid; @@ -3375,6 +3379,8 @@ invalid: nfsmout: if (vp) vrele(vp); + if (mntp) + vfs_unbusy(mntp); VFS_UNLOCK_GIANT(vfslocked); return(error); } @@ -3426,8 +3432,7 @@ nfsrv_commit(struct nfsrv_descript *nfsd off = fxdr_hyper(tl); tl += 2; cnt = fxdr_unsigned(int, *tl); - error = nfsrv_fhtovp(fhp, 1, &vp, &tvfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &tvfslocked, nfsd, slp, nam, &rdonly); vfslocked = nfsrv_lockedpair(vfslocked, tvfslocked); if (error) { nfsm_reply(2 * NFSX_UNSIGNED); @@ -3571,8 +3576,7 @@ nfsrv_statfs(struct nfsrv_descript *nfsd vfslocked = 0; fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (error) { nfsm_reply(NFSX_UNSIGNED); if (v3) @@ -3666,8 +3670,7 @@ nfsrv_fsinfo(struct nfsrv_descript *nfsd fhp = &nfh.fh_generic; vfslocked = 0; nfsm_srvmtofh(fhp); - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (error) { nfsm_reply(NFSX_UNSIGNED); nfsm_srvpostop_attr(getret, &at); @@ -3741,8 +3744,7 @@ nfsrv_pathconf(struct nfsrv_descript *nf vfslocked = 0; fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); - error = nfsrv_fhtovp(fhp, 1, &vp, &vfslocked, nfsd, slp, - nam, &rdonly, TRUE); + error = nfsrv_fhtovp(fhp, 0, &vp, &vfslocked, nfsd, slp, nam, &rdonly); if (error) { nfsm_reply(NFSX_UNSIGNED); nfsm_srvpostop_attr(getret, &at); Modified: stable/8/sys/nfsserver/nfs_srvsubs.c ============================================================================== --- stable/8/sys/nfsserver/nfs_srvsubs.c Thu Dec 30 09:32:39 2010 (r216814) +++ stable/8/sys/nfsserver/nfs_srvsubs.c Thu Dec 30 09:45:26 2010 (r216815) @@ -639,16 +639,18 @@ nfs_namei(struct nameidata *ndp, struct goto out; } + if (!pubflag && nfs_ispublicfh(fhp)) + return (ESTALE); + /* * Extract and set starting directory. */ - error = nfsrv_fhtovp(fhp, FALSE, &dp, &dvfslocked, - nfsd, slp, nam, &rdonly, pubflag); + error = nfsrv_fhtovp(fhp, 0, &dp, &dvfslocked, nfsd, slp, nam, &rdonly); if (error) goto out; vfslocked = VFS_LOCK_GIANT(dp->v_mount); if (dp->v_type != VDIR) { - vrele(dp); + vput(dp); error = ENOTDIR; goto out; } @@ -662,12 +664,12 @@ nfs_namei(struct nameidata *ndp, struct */ *retdirp = dp; if (v3) { - vn_lock(dp, LK_EXCLUSIVE | LK_RETRY); *retdirattr_retp = VOP_GETATTR(dp, retdirattrp, ndp->ni_cnd.cn_cred); - VOP_UNLOCK(dp, 0); } + VOP_UNLOCK(dp, 0); + if (pubflag) { /* * Oh joy. For WebNFS, handle those pesky '%' escapes, @@ -1051,12 +1053,11 @@ nfsm_srvfattr(struct nfsrv_descript *nfs * - look up fsid in mount list (if not found ret error) * - get vp and export rights by calling VFS_FHTOVP() * - if cred->cr_uid == 0 or MNT_EXPORTANON set it to credanon - * - if not lockflag unlock it with VOP_UNLOCK() */ int -nfsrv_fhtovp(fhandle_t *fhp, int lockflag, struct vnode **vpp, int *vfslockedp, +nfsrv_fhtovp(fhandle_t *fhp, int flags, struct vnode **vpp, int *vfslockedp, struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, - struct sockaddr *nam, int *rdonlyp, int pubflag) + struct sockaddr *nam, int *rdonlyp) { struct mount *mp; int i; @@ -1076,7 +1077,7 @@ nfsrv_fhtovp(fhandle_t *fhp, int lockfla *vpp = NULL; if (nfs_ispublicfh(fhp)) { - if (!pubflag || !nfs_pub.np_valid) + if (!nfs_pub.np_valid) return (ESTALE); fhp = &nfs_pub.np_handle; } @@ -1128,12 +1129,12 @@ nfsrv_fhtovp(fhandle_t *fhp, int lockfla } } error = VFS_FHTOVP(mp, &fhp->fh_fid, vpp); - if (error != 0) + if (error) { /* Make sure the server replies ESTALE to the client. */ error = ESTALE; - vfs_unbusy(mp); - if (error) + vfs_unbusy(mp); goto out; + } #ifdef MNT_EXNORESPORT if (!(exflags & (MNT_EXNORESPORT|MNT_EXPUBLIC))) { saddr = (struct sockaddr_in *)nam; @@ -1144,6 +1145,8 @@ nfsrv_fhtovp(fhandle_t *fhp, int lockfla vput(*vpp); *vpp = NULL; error = NFSERR_AUTHERR | AUTH_TOOWEAK; + vfs_unbusy(mp); + goto out; } } #endif @@ -1160,8 +1163,8 @@ nfsrv_fhtovp(fhandle_t *fhp, int lockfla else *rdonlyp = 0; - if (!lockflag) - VOP_UNLOCK(*vpp, 0); + if (!(flags & NFSRV_FLAG_BUSY)) + vfs_unbusy(mp); out: if (credanon != NULL) crfree(credanon);