Change the representation of an NFS mount point by caching the root

nodes.

nfs_root() now returns a "locked" vnode, so vput(9) must be called to
release it.  Note that this has currently no effect as nfs_lock/unlock
are still stubs.

This will prevent some lock odering problems with upcoming NFSnode
locking.

Tested by landry@, sthen@, visa@, naddy@ and myself.

From NetBSD with some tweaks, ok visa@

1 files changed, 75 insertions, 27 deletions

diff --git a/sys/nfs/nfs_vfsops.c b/sys/nfs/nfs_vfsops.c
index 259b2f7f0d6..bd1074d76c2 100644
--- a/sys/nfs/nfs_vfsops.c
+++ b/sys/nfs/nfs_vfsops.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_vfsops.c,v 1.116 2018/02/10 05:24:23 deraadt Exp $	*/
+/*	$OpenBSD: nfs_vfsops.c,v 1.117 2018/04/09 09:39:53 mpi Exp $	*/
 /*	$NetBSD: nfs_vfsops.c,v 1.46.4.1 1996/05/25 22:40:35 fvdl Exp $	*/
 
 /*
@@ -71,11 +71,13 @@ extern struct nfsstats nfsstats;
 extern int nfs_ticks;
 extern u_int32_t nfs_procids[NFS_NPROCS];
 
-int		nfs_sysctl(int *, u_int, void *, size_t *, void *, size_t, struct proc *);
-int		nfs_checkexp(struct mount *, struct mbuf *, int *, struct ucred **);
-struct mount	*nfs_mount_diskless(struct nfs_dlmount *, char *, int);
+int	nfs_sysctl(int *, u_int, void *, size_t *, void *, size_t,
+	    struct proc *);
+int	nfs_checkexp(struct mount *, struct mbuf *, int *, struct ucred **);
+struct	mount *nfs_mount_diskless(struct nfs_dlmount *, char *, int,
+	    struct vnode **, struct proc *p);
 int	mountnfs(struct nfs_args *, struct mount *, struct mbuf *,
-	    const char *, char *);
+	    const char *, char *, struct vnode **, struct proc *p);
 int	nfs_quotactl(struct mount *, int, uid_t, caddr_t, struct proc *);
 int	nfs_root(struct mount *, struct vnode **);
 int	nfs_start(struct mount *, int, struct proc *);
@@ -123,15 +125,13 @@ nfs_statfs(struct mount *mp, struct statfs *sbp, struct proc *p)
 	struct nfsmount *nmp = VFSTONFS(mp);
 	int error = 0, retattr;
 	struct ucred *cred;
-	struct nfsnode *np;
 	u_quad_t tquad;
 
 	info.nmi_v3 = (nmp->nm_flag & NFSMNT_NFSV3);
 
-	error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np);
+	error = nfs_root(mp, &vp);
 	if (error)
 		return (error);
-	vp = NFSTOV(np);
 	cred = crget();
 	cred->cr_ngroups = 0;
 	if (info.nmi_v3 && (nmp->nm_flag & NFSMNT_GOTFSINFO) == 0)
@@ -178,7 +178,7 @@ nfs_statfs(struct mount *mp, struct statfs *sbp, struct proc *p)
 	copy_statfs_info(sbp, mp);
 	m_freem(info.nmi_mrep);
 nfsmout: 
-	vrele(vp);
+	vput(vp);
 	crfree(cred);
 	return (error);
 }
@@ -281,14 +281,14 @@ nfs_mountroot(void)
 	 */
 	if (nfs_boot_getfh(&nfs_diskless.nd_boot, "root", &nfs_diskless.nd_root, -1))
 		panic("nfs_mountroot: root");
-	mp = nfs_mount_diskless(&nfs_diskless.nd_root, "/", 0);
-	nfs_root(mp, &rootvp);
+	mp = nfs_mount_diskless(&nfs_diskless.nd_root, "/", 0, &vp, procp);
 	printf("root on %s\n", nfs_diskless.nd_root.ndm_host);
 
 	/*
 	 * Link it into the mount list.
 	 */
 	TAILQ_INSERT_TAIL(&mountlist, mp, mnt_list);
+	rootvp = vp;
 	vfs_unbusy(mp);
 
 	/* Get root attributes (for the time). */
@@ -333,8 +333,8 @@ nfs_mountroot(void)
 	 */
 	error = nfs_boot_getfh(&nfs_diskless.nd_boot, "swap", &nfs_diskless.nd_swap, 5);
 	if (!error) {
-		mp = nfs_mount_diskless(&nfs_diskless.nd_swap, "/swap", 0);
-		nfs_root(mp, &vp);
+		mp = nfs_mount_diskless(&nfs_diskless.nd_swap, "/swap", 0, &vp,
+		    procp);
 		vfs_unbusy(mp);
 
 		/*
@@ -376,7 +376,8 @@ nfs_mountroot(void)
  * Internal version of mount system call for diskless setup.
  */
 struct mount *
-nfs_mount_diskless(struct nfs_dlmount *ndmntp, char *mntname, int mntflag)
+nfs_mount_diskless(struct nfs_dlmount *ndmntp, char *mntname, int mntflag,
+    struct vnode **vpp, struct proc *p)
 {
 	struct mount *mp;
 	struct mbuf *m;
@@ -392,7 +393,7 @@ nfs_mount_diskless(struct nfs_dlmount *ndmntp, char *mntname, int mntflag)
 	    (m->m_len = ndmntp->ndm_args.addr->sa_len));
 
 	error = mountnfs(&ndmntp->ndm_args, mp, m, mntname,
-	    ndmntp->ndm_args.hostname);
+	    ndmntp->ndm_args.hostname, vpp, p);
 	if (error)
 		panic("nfs_mountroot: mount %s failed: %d", mntname, error);
 
@@ -556,6 +557,7 @@ nfs_mount(struct mount *mp, const char *path, void *data,
 	int error;
 	struct nfs_args *args = data;
 	struct mbuf *nam;
+	struct vnode *vp;
 	char hst[MNAMELEN];
 	size_t len;
 	u_char nfh[NFSX_V3FHMAX];
@@ -599,7 +601,7 @@ nfs_mount(struct mount *mp, const char *path, void *data,
 	if (error)
 		return (error);
 	args->fh = nfh;
-	error = mountnfs(args, mp, nam, path, hst);
+	error = mountnfs(args, mp, nam, path, hst, &vp, p);
 	return (error);
 }
 
@@ -608,9 +610,12 @@ nfs_mount(struct mount *mp, const char *path, void *data,
  */
 int
 mountnfs(struct nfs_args *argp, struct mount *mp, struct mbuf *nam,
-    const char *pth, char *hst)
+    const char *pth, char *hst, struct vnode **vpp, struct proc *p)
 {
 	struct nfsmount *nmp;
+	struct nfsnode *np;
+	struct vnode *vp;
+	struct vattr attr;
 	int error;
 
 	if (mp->mnt_flag & MNT_UPDATE) {
@@ -633,12 +638,10 @@ mountnfs(struct nfs_args *argp, struct mount *mp, struct mbuf *nam,
 	nmp->nm_readdirsize = NFS_READDIRSIZE;
 	nmp->nm_numgrps = NFS_MAXGRPS;
 	nmp->nm_readahead = NFS_DEFRAHEAD;
-	nmp->nm_fhsize = argp->fhsize;
 	nmp->nm_acregmin = NFS_MINATTRTIMO;
 	nmp->nm_acregmax = NFS_MAXATTRTIMO;
 	nmp->nm_acdirmin = NFS_MINATTRTIMO;
 	nmp->nm_acdirmax = NFS_MAXATTRTIMO;
-	bcopy(argp->fh, nmp->nm_fh, argp->fhsize);
 	mp->mnt_stat.f_namemax = MAXNAMLEN;
 	memset(mp->mnt_stat.f_mntonname, 0, MNAMELEN);
 	strlcpy(mp->mnt_stat.f_mntonname, pth, MNAMELEN);
@@ -673,6 +676,30 @@ mountnfs(struct nfs_args *argp, struct mount *mp, struct mbuf *nam,
 	 * point.
 	 */
 	mp->mnt_stat.f_iosize = NFS_MAXDGRAMDATA;
+	error = nfs_nget(mp, (nfsfh_t *)argp->fh, argp->fhsize, &np);
+	if (error)
+		goto bad;
+	vp = NFSTOV(np);
+	error = VOP_GETATTR(vp, &attr, p->p_ucred, p);
+	if (error) {
+		vput(vp);
+		goto bad;
+	}
+
+	/*
+	 * A reference count is needed on the nfsnode representing the
+	 * remote root.  If this object is not persistent, then backward
+	 * traversals of the mount point (i.e. "..") will not work if
+	 * the nfsnode gets flushed out of the cache. Ufs does not have
+	 * this problem, because one can identify root inodes by their
+	 * number == ROOTINO (2).  So, just unlock, but no rele.
+	 */
+	nmp->nm_vnode = vp;
+	if (vp->v_type == VNON)
+		vp->v_type = VDIR;
+	vp->v_flag = VROOT;
+	VOP_UNLOCK(vp, curproc);
+	*vpp = vp;
 
 	return (0);
 bad:
@@ -687,18 +714,35 @@ int
 nfs_unmount(struct mount *mp, int mntflags, struct proc *p)
 {
 	struct nfsmount *nmp;
-	int error, flags;
+	struct vnode *vp;
+	int error, flags = 0;
 
 	nmp = VFSTONFS(mp);
-	flags = 0;
+	error = nfs_root(mp, &vp);
+	if (error)
+		return (error);
+
+	if ((mntflags & MNT_FORCE) == 0 && vp->v_usecount > 2) {
+		vput(vp);
+		return (EBUSY);
+	}
 
 	if (mntflags & MNT_FORCE)
 		flags |= FORCECLOSE;
 
-	error = vflush(mp, NULL, flags);
-	if (error)
+	error = vflush(mp, vp, flags);
+	if (error) {
+		vput(vp);
 		return (error);
+	}
 
+	/*
+	 * There are two references count to get rid of here: one
+	 * from mountnfs() and one from nfs_root() above.
+	 */
+	vrele(vp);
+	vput(vp);
+	vgone(vp);
 	nfs_disconnect(nmp);
 	m_freem(nmp->nm_nam);
 	timeout_del(&nmp->nm_rtimeout);
@@ -713,15 +757,19 @@ nfs_unmount(struct mount *mp, int mntflags, struct proc *p)
 int
 nfs_root(struct mount *mp, struct vnode **vpp)
 {
+	struct vnode *vp;
 	struct nfsmount *nmp;
-	struct nfsnode *np;
 	int error;
 
 	nmp = VFSTONFS(mp);
-	error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np);
-	if (error)
+	vp = nmp->nm_vnode;
+	vref(vp);
+	error = vn_lock(vp, LK_EXCLUSIVE | LK_RETRY, curproc);
+	if (error) {
+		vrele(vp);
 		return (error);
-	*vpp = NFSTOV(np);
+	}
+	*vpp = vp;
 	return (0);
 }