From e15092bec82b15c4936097f3d3f159f013dbd05b 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 34/36] 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 4f361012cc62..db1048ccbd77 100644
--- a/sys/kern/vfs_vnode.c
+++ b/sys/kern/vfs_vnode.c
@@ -1722,7 +1722,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 ||
@@ -1730,7 +1730,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
@@ -1773,8 +1772,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) {
@@ -1784,7 +1781,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 8819de4c7013..58e054bfe513 100644
--- a/sys/miscfs/specfs/spec_vnops.c
+++ b/sys/miscfs/specfs/spec_vnops.c
@@ -543,6 +543,7 @@ spec_node_revoke(vnode_t *vp)
{
specnode_t *sn;
specdev_t *sd;
+ struct vnode **vpp;
sn = vp->v_specnode;
sd = sn->sn_dev;
@@ -553,10 +554,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);
@@ -571,6 +572,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);
}
@@ -583,7 +600,6 @@ spec_node_destroy(vnode_t *vp)
{
specnode_t *sn;
specdev_t *sd;
- vnode_t **vpp, *vp2;
int refcnt;
sn = vp->v_specnode;
@@ -594,22 +610,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--;