From 6e8f77263593b2ca2b44fc6f38075fe5db218703 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh@NetBSD.org>
Date: Mon, 31 Jan 2022 01:07:56 +0000
Subject: [PATCH 33/37] specfs: Remove specnode from hash table in
 spec_node_revoke.

This way, it is safe to vcache_vget a vnode in the hash table even if
that fails -- which we will need in order to let vdevgone wait for
pending revoke to complete.

This requires calling spec_node_revoke unconditionally for device
special nodes, not just for active ones.  Might introduce slightly
more contention on device_lock but not much because we already have
to take it in this path anyway a little later in spec_node_destroy.
---
 sys/kern/vfs_vnode.c           |  7 ++-----
 sys/miscfs/specfs/spec_vnops.c | 36 +++++++++++++++++-----------------
 2 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/sys/kern/vfs_vnode.c b/sys/kern/vfs_vnode.c
index 58db8e0e97e7..ee365aaaba08 100644
--- a/sys/kern/vfs_vnode.c
+++ b/sys/kern/vfs_vnode.c
@@ -1814,7 +1814,7 @@ vcache_reclaim(vnode_t *vp)
 	uint32_t hash;
 	uint8_t temp_buf[64], *temp_key;
 	size_t temp_key_len;
-	bool recycle, active;
+	bool recycle;
 	int error;
 
 	KASSERT((vp->v_vflag & VV_LOCKSWORK) == 0 ||
@@ -1822,7 +1822,6 @@ vcache_reclaim(vnode_t *vp)
 	KASSERT(mutex_owned(vp->v_interlock));
 	KASSERT(vrefcnt(vp) != 0);
 
-	active = (vrefcnt(vp) > 1);
 	temp_key_len = vip->vi_key.vk_key_len;
 	/*
 	 * Prevent the vnode from being recycled or brought into use
@@ -1865,8 +1864,6 @@ vcache_reclaim(vnode_t *vp)
 
 	/*
 	 * Clean out any cached data associated with the vnode.
-	 * If purging an active vnode, it must be closed and
-	 * deactivated before being reclaimed.
 	 */
 	error = vinvalbuf(vp, V_SAVE, NOCRED, l, 0, 0);
 	if (error != 0) {
@@ -1876,7 +1873,7 @@ vcache_reclaim(vnode_t *vp)
 	}
 	KASSERTMSG((error == 0), "vinvalbuf failed: %d", error);
 	KASSERT((vp->v_iflag & VI_ONWORKLST) == 0);
-	if (active && (vp->v_type == VBLK || vp->v_type == VCHR)) {
+	if (vp->v_type == VBLK || vp->v_type == VCHR) {
 		 spec_node_revoke(vp);
 	}
 
diff --git a/sys/miscfs/specfs/spec_vnops.c b/sys/miscfs/specfs/spec_vnops.c
index 3dce4cc7a83f..324297488d5d 100644
--- a/sys/miscfs/specfs/spec_vnops.c
+++ b/sys/miscfs/specfs/spec_vnops.c
@@ -591,6 +591,7 @@ spec_node_revoke(vnode_t *vp)
 {
 	specnode_t *sn;
 	specdev_t *sd;
+	struct vnode **vpp;
 
 	sn = vp->v_specnode;
 	sd = sn->sn_dev;
@@ -601,10 +602,10 @@ spec_node_revoke(vnode_t *vp)
 
 	mutex_enter(&device_lock);
 	KASSERT(sn->sn_opencnt <= sd->sd_opencnt);
+	sn->sn_gone = true;
 	if (sn->sn_opencnt != 0) {
 		sd->sd_opencnt -= (sn->sn_opencnt - 1);
 		sn->sn_opencnt = 1;
-		sn->sn_gone = true;
 		mutex_exit(&device_lock);
 
 		VOP_CLOSE(vp, FNONBLOCK, NOCRED);
@@ -619,6 +620,22 @@ spec_node_revoke(vnode_t *vp)
 	 */
 	while (sd->sd_closing)
 		cv_wait(&specfs_iocv, &device_lock);
+
+	/*
+	 * Remove from the hash so lookups stop returning this
+	 * specnode.  We will dissociate it from the specdev -- and
+	 * possibly free the specdev -- in spec_node_destroy.
+	 */
+	KASSERT(sn->sn_gone);
+	KASSERT(sn->sn_opencnt == 0);
+	for (vpp = &specfs_hash[SPECHASH(vp->v_rdev)];;
+	     vpp = &(*vpp)->v_specnext) {
+		if (*vpp == vp) {
+			*vpp = vp->v_specnext;
+			vp->v_specnext = NULL;
+			break;
+		}
+	}
 	mutex_exit(&device_lock);
 }
 
@@ -631,7 +648,6 @@ spec_node_destroy(vnode_t *vp)
 {
 	specnode_t *sn;
 	specdev_t *sd;
-	vnode_t **vpp, *vp2;
 	int refcnt;
 
 	sn = vp->v_specnode;
@@ -642,22 +658,6 @@ spec_node_destroy(vnode_t *vp)
 	KASSERT(sn->sn_opencnt == 0);
 
 	mutex_enter(&device_lock);
-	/* Remove from the hash and destroy the node. */
-	vpp = &specfs_hash[SPECHASH(vp->v_rdev)];
-	for (vp2 = *vpp;; vp2 = vp2->v_specnext) {
-		if (vp2 == NULL) {
-			panic("spec_node_destroy: corrupt hash");
-		}
-		if (vp2 == vp) {
-			KASSERT(vp == *vpp);
-			*vpp = vp->v_specnext;
-			break;
-		}
-		if (vp2->v_specnext == vp) {
-			vp2->v_specnext = vp->v_specnext;
-			break;
-		}
-	}
 	sn = vp->v_specnode;
 	vp->v_specnode = NULL;
 	refcnt = sd->sd_refcnt--;