I ran into an nfs client panic a couple of times in a row over the

last few days.  I tracked it down to the fact that nfs_reclaim()
is setting vp->v_data to NULL _before_ calling vnode_destroy_object().
After silence from the mailing list I checked further and discovered
that ufs_reclaim() is unique among FreeBSD filesystems for calling
vnode_destroy_object() early, long before tossing v_data or much
of anything else, for that matter.  The rest, including NFS, appear
to be identical, as if they were just clones of one original routine.

The enclosed patch fixes all file systems in essentially the same
way, by moving the call to vnode_destroy_object() to early in the
routine (before the call to vfs_hash_remove(), if any).  I have
only tested NFS, but I've now run for over eighteen hours with the
patch where I wouldn't get past four or five without it.

Submitted by: Frank Mayhar
Requested by: Mohan Srinivasan
MFC After: 1 week

9 files changed, 41 insertions, 9 deletions

diff --git a/sys/fs/cd9660/cd9660_node.c b/sys/fs/cd9660/cd9660_node.c
index 8d281de..69e1043 100644
--- a/sys/fs/cd9660/cd9660_node.c
+++ b/sys/fs/cd9660/cd9660_node.c
@@ -98,6 +98,10 @@ cd9660_reclaim(ap)
 	if (prtactive && vrefcnt(vp) != 0)
 		vprint("cd9660_reclaim: pushing active", vp);
 	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+	/*
 	 * Remove the inode from its hash chain.
 	 */
 	vfs_hash_remove(vp);
@@ -109,7 +113,6 @@ cd9660_reclaim(ap)
 		vrele(ip->i_mnt->im_devvp);
 	FREE(vp->v_data, M_ISOFSNODE);
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 	return (0);
 }
 
diff --git a/sys/fs/hpfs/hpfs_vnops.c b/sys/fs/hpfs/hpfs_vnops.c
index fd2e066..4c73fe4 100644
--- a/sys/fs/hpfs/hpfs_vnops.c
+++ b/sys/fs/hpfs/hpfs_vnops.c
@@ -600,12 +600,16 @@ hpfs_reclaim(ap)
 
 	dprintf(("hpfs_reclaim(0x%x0): \n", hp->h_no));
 
+	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+
 	vfs_hash_remove(vp);
 
 	mtx_destroy(&hp->h_interlock);
 
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 
 	FREE(hp, M_HPFSNO);
 
diff --git a/sys/fs/msdosfs/msdosfs_denode.c b/sys/fs/msdosfs/msdosfs_denode.c
index effc20b..e2dbc9b 100644
--- a/sys/fs/msdosfs/msdosfs_denode.c
+++ b/sys/fs/msdosfs/msdosfs_denode.c
@@ -549,6 +549,10 @@ msdosfs_reclaim(ap)
 	if (prtactive && vrefcnt(vp) != 0)
 		vprint("msdosfs_reclaim(): pushing active", vp);
 	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+	/*
 	 * Remove the denode from its hash chain.
 	 */
 	vfs_hash_remove(vp);
@@ -560,7 +564,6 @@ msdosfs_reclaim(ap)
 #endif
 	FREE(dep, M_MSDOSFSNODE);
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 
 	return (0);
 }
diff --git a/sys/fs/ntfs/ntfs_vnops.c b/sys/fs/ntfs/ntfs_vnops.c
index d5a017f..86c883b 100644
--- a/sys/fs/ntfs/ntfs_vnops.c
+++ b/sys/fs/ntfs/ntfs_vnops.c
@@ -248,6 +248,11 @@ ntfs_reclaim(ap)
 	if (ntfs_prtactive && vrefcnt(vp) != 0)
 		vprint("ntfs_reclaim: pushing active", vp);
 
+	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+
 	if ((error = ntfs_ntget(ip)) != 0)
 		return (error);
 	
@@ -255,7 +260,6 @@ ntfs_reclaim(ap)
 	ntfs_frele(fp);
 	ntfs_ntput(ip);
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 
 	return (0);
 }
diff --git a/sys/fs/nwfs/nwfs_node.c b/sys/fs/nwfs/nwfs_node.c
index 510f174..dc906ef 100644
--- a/sys/fs/nwfs/nwfs_node.c
+++ b/sys/fs/nwfs/nwfs_node.c
@@ -255,6 +255,11 @@ nwfs_reclaim(ap)
 	struct thread *td = ap->a_td;
 	
 	NCPVNDEBUG("%s,%d\n", np->n_name, vrefcnt(vp));
+	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+
 	if (np->n_flag & NREFPARENT) {
 		np->n_flag &= ~NREFPARENT;
 		if (nwfs_lookupnp(nmp, np->n_parent, td, &dnp) == 0) {
@@ -270,7 +275,6 @@ nwfs_reclaim(ap)
 		nmp->n_root = NULL;
 	}
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 	FREE(np, M_NWNODE);
 	if (dvp) {
 		vrele(dvp);
diff --git a/sys/fs/smbfs/smbfs_node.c b/sys/fs/smbfs/smbfs_node.c
index d48bdb76..45ab034 100644
--- a/sys/fs/smbfs/smbfs_node.c
+++ b/sys/fs/smbfs/smbfs_node.c
@@ -319,6 +319,10 @@ smbfs_reclaim(ap)
 	KASSERT((np->n_flag & NOPEN) == 0, ("file not closed before reclaim"));
 
 	smbfs_hash_lock(smp, td);
+	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
 
 	dvp = (np->n_parent && (np->n_flag & NREFPARENT)) ?
 	    np->n_parent : NULL;
@@ -330,7 +334,6 @@ smbfs_reclaim(ap)
 		smp->sm_root = NULL;
 	}
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 	smbfs_hash_unlock(smp, td);
 	if (np->n_name)
 		smbfs_name_free(np->n_name);
diff --git a/sys/fs/udf/udf_vnops.c b/sys/fs/udf/udf_vnops.c
index 5aa356f..0bb91ec 100644
--- a/sys/fs/udf/udf_vnops.c
+++ b/sys/fs/udf/udf_vnops.c
@@ -965,6 +965,11 @@ udf_reclaim(struct vop_reclaim_args *a)
 	vp = a->a_vp;
 	unode = VTON(vp);
 
+	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+
 	if (unode != NULL) {
 		vfs_hash_remove(vp);
 
@@ -973,7 +978,6 @@ udf_reclaim(struct vop_reclaim_args *a)
 		uma_zfree(udf_zone_node, unode);
 		vp->v_data = NULL;
 	}
-	vnode_destroy_vobject(vp);
 
 	return (0);
 }
diff --git a/sys/isofs/cd9660/cd9660_node.c b/sys/isofs/cd9660/cd9660_node.c
index 8d281de..69e1043 100644
--- a/sys/isofs/cd9660/cd9660_node.c
+++ b/sys/isofs/cd9660/cd9660_node.c
@@ -98,6 +98,10 @@ cd9660_reclaim(ap)
 	if (prtactive && vrefcnt(vp) != 0)
 		vprint("cd9660_reclaim: pushing active", vp);
 	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+	/*
 	 * Remove the inode from its hash chain.
 	 */
 	vfs_hash_remove(vp);
@@ -109,7 +113,6 @@ cd9660_reclaim(ap)
 		vrele(ip->i_mnt->im_devvp);
 	FREE(vp->v_data, M_ISOFSNODE);
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 	return (0);
 }
 
diff --git a/sys/nfsclient/nfs_node.c b/sys/nfsclient/nfs_node.c
index 47f5f4e..ada16f4 100644
--- a/sys/nfsclient/nfs_node.c
+++ b/sys/nfsclient/nfs_node.c
@@ -211,6 +211,11 @@ nfs_reclaim(struct vop_reclaim_args *ap)
 	if (prtactive && vrefcnt(vp) != 0)
 		vprint("nfs_reclaim: pushing active", vp);
 
+	/*
+	 * Destroy the vm object and flush associated pages.
+	 */
+	vnode_destroy_vobject(vp);
+
 	vfs_hash_remove(vp);
 
 	/*
@@ -232,6 +237,5 @@ nfs_reclaim(struct vop_reclaim_args *ap)
 
 	uma_zfree(nfsnode_zone, vp->v_data);
 	vp->v_data = NULL;
-	vnode_destroy_vobject(vp);
 	return (0);
 }