FBSD: Drop GLOCK when grabbing vnode locks 82/14182/7
authorAndrew Deason <adeason@dson.org>
Sun, 3 May 2020 02:01:50 +0000 (21:01 -0500)
committerBenjamin Kaduk <kaduk@mit.edu>
Thu, 21 Oct 2021 16:43:47 +0000 (12:43 -0400)
In a few places, we try to vn_lock() or VOP_LOCK() while holding
AFS_GLOCK (or do something like vrele(), which internally acquires the
vnode lock). This is against the FBSD locking rules, since our
AFS_GLOCK is a non-sleepable 'struct mtx' lock, and vnode locks are
sleepable lockmgr locks.

So, drop AFS_GLOCK while acquiring vnode locks.

Change-Id: I0ca449bb6398aa8ededd5bb67d56a7d3f13688f0
Reviewed-on: https://gerrit.openafs.org/14182
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

src/afs/FBSD/osi_vnodeops.c
src/afs/afs_init.c

index dce4e1c..bc226fd 100644 (file)
@@ -1078,12 +1078,12 @@ afs_vop_symlink(struct vop_symlink_args *ap)
                    cnp->cn_cred);
     if (error == 0) {
        error = afs_lookup(VTOAFS(dvp), name, &vcp, cnp->cn_cred);
-       if (error == 0) {
-           newvp = AFSTOV(vcp);
-           vn_lock(newvp, LK_EXCLUSIVE | LK_RETRY);
-       }
     }
     AFS_GUNLOCK();
+    if (error == 0) {
+       newvp = AFSTOV(vcp);
+       vn_lock(newvp, LK_EXCLUSIVE | LK_RETRY);
+    }
     DROPNAME();
     *(ap->a_vpp) = newvp;
     return error;
@@ -1201,7 +1201,9 @@ afs_vop_reclaim(struct vop_reclaim_args *ap)
      * vnode is already VI_DOOMED. We just want to lock it again, and skip the
      * VI_DOOMED check.
      */
+    AFS_GUNLOCK();
     VOP_LOCK(vp, LK_EXCLUSIVE);
+    AFS_GLOCK();
 
     code = afs_FlushVCache(avc, &slept);
 
index 066ca30..a5ae8c4 100644 (file)
@@ -695,14 +695,21 @@ shutdown_cache(void)
        pag_epoch = 0;
        pagCounter = 0;
 #if defined(AFS_XBSD_ENV)
-       /* memcache never sets this, so don't panic on shutdown */
-       if (volumeVnode != NULL) {
-           vrele(volumeVnode); /* let it go, finally. */
+       {
+           struct vnode *volumeVnode_free = volumeVnode;
+           struct vnode *cacheDev_free = cacheDev.held_vnode;
            volumeVnode = NULL;
-       }
-       if (cacheDev.held_vnode) {
-           vrele(cacheDev.held_vnode);
            cacheDev.held_vnode = NULL;
+
+           AFS_GUNLOCK();
+           /* memcache never sets these, so they may be NULL. */
+           if (volumeVnode_free != NULL) {
+               vrele(volumeVnode_free);        /* let it go, finally. */
+           }
+           if (cacheDev_free != NULL) {
+               vrele(cacheDev_free);
+           }
+           AFS_GLOCK();
        }
 #endif /* AFS_XBSD_ENV */
 #ifdef AFS_CACHE_VNODE_PATH