From a251a9a8bc3081b2e7e8a8a4cdc53e28aeaf463b Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Wed, 19 Jan 2022 01:11:45 +0000 Subject: [PATCH 18/37] specfs: sn_gone cannot be set while we hold the vnode lock. Revoke runs with the vnode lock too, which is exclusive. Add an assertion to this effect in spec_node_revoke to make it clear. XXX Exception: For file systems with nonworking locks, this doesn't work. But I'm not sure suspending such file systems works either. Another reason to get rid of file systems with nonworking locks -- which is only deadfs as used by bdevvp/cdevvp. --- sys/miscfs/specfs/spec_vnops.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/sys/miscfs/specfs/spec_vnops.c b/sys/miscfs/specfs/spec_vnops.c index 54232be1ef98..8bfc08edf20f 100644 --- a/sys/miscfs/specfs/spec_vnops.c +++ b/sys/miscfs/specfs/spec_vnops.c @@ -402,6 +402,9 @@ spec_node_revoke(vnode_t *vp) specnode_t *sn; specdev_t *sd; + KASSERT(VOP_ISLOCKED(vp) == LK_EXCLUSIVE || + (vp->v_vflag & VV_LOCKSWORK) == 0); + sn = vp->v_specnode; sd = sn->sn_dev; @@ -552,11 +555,8 @@ spec_open(void *v) /* * Acquire an open reference -- as long as we hold onto it, and - * the vnode isn't revoked, it can't be closed. - * - * But first check whether it has been revoked -- if so, we - * can't acquire more open references and we must fail - * immediately with EBADF. + * the vnode isn't revoked, it can't be closed, and the vnode + * can't be revoked until we release the vnode lock. */ mutex_enter(&device_lock); switch (vp->v_type) { @@ -565,10 +565,7 @@ spec_open(void *v) * Character devices can accept opens from multiple * vnodes. */ - if (sn->sn_gone) { - error = EBADF; - break; - } + KASSERT(!sn->sn_gone); sd->sd_opencnt++; sn->sn_opencnt++; break; @@ -581,10 +578,7 @@ spec_open(void *v) * Treat zero opencnt with non-NULL mountpoint as open. * This may happen after forced detach of a mounted device. */ - if (sn->sn_gone) { - error = EBADF; - break; - } + KASSERT(!sn->sn_gone); if (sd->sd_opencnt != 0 || sd->sd_mountpoint != NULL) { error = EBUSY; break;