FBSD: do not recurse on the afs_xvcache write lock
authorBen Kaduk <kaduk@mit.edu>
Thu, 1 Jul 2010 16:47:55 +0000 (12:47 -0400)
committerDerrick Brashear <shadow@dementia.org>
Thu, 1 Jul 2010 17:05:00 +0000 (10:05 -0700)
afs_ShakeLooseVcaches grabs the write lock before calling
osi_TryEvictVCache.  The latter calls vgone(l), which sometimes
calls into VOP_CLOSE, which again trys to acquire the write lock,
leading to deadlock.  Drop the write lock and reaquire it in
TryEvictVCache as a fix.

While here, set *slept to 1 since we drop the glock.

This churn was enough that the gcc no longer cached the value of
AFSTOV(avc), which gets set to 0 in VOP_RECLAIM, so the subsequent
VOP_UNLOCK dereferenced a null pointer.  Cache the vnode pointer
in a local variable for the entire function instead of using
AFSTOV() everywhere.

Change-Id: Ic826e7888cb400c19857c58cb6ed88b9bdd0dddc
Reviewed-on: http://gerrit.openafs.org/2315
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>

src/afs/FBSD/osi_vcache.c

index dd295f2..52d7ae3 100644 (file)
@@ -15,6 +15,8 @@
 
 int
 osi_TryEvictVCache(struct vcache *avc, int *slept) {
+    struct vnode *vp = AFSTOV(avc);
+
     if (!VREFCOUNT_GT(avc,0)
         && avc->opens == 0 && (avc->f.states & CUnlinkedDel) == 0) {
        /*
@@ -24,24 +26,32 @@ osi_TryEvictVCache(struct vcache *avc, int *slept) {
          * not on the free list.
          * XXX assume FreeBSD is the same for now.
          */
+       /*
+        * We only have one caller (afs_ShakeLooseVCaches), which already
+        * holds the write lock.  vgonel() sometimes calls VOP_CLOSE(),
+        * so we must drop the write lock around our call to vgone().
+        */
+       ReleaseWriteLock(&afs_xvcache);
         AFS_GUNLOCK();
+       *slept = 1;
 
 #if defined(AFS_FBSD80_ENV)
        /* vgone() is correct, but v_usecount is assumed not
         * to be 0, and I suspect that currently our usage ensures that
         * in fact it will */
-       if (vrefcnt(AFSTOV(avc)) < 1) {
-           vref(AFSTOV(avc));
+       if (vrefcnt(vp) < 1) {
+           vref(vp);
        }
-       vn_lock(AFSTOV(avc), LK_EXCLUSIVE | LK_RETRY); /* !glocked */
+       vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); /* !glocked */
 #endif
 
-        vgone(AFSTOV(avc));
+        vgone(vp);
 #if defined(AFS_FBSD80_ENV)
-       VOP_UNLOCK(AFSTOV(avc), 0);
+       VOP_UNLOCK(vp, 0);
 #endif
 
        AFS_GLOCK();
+       ObtainWriteLock(&afs_xvcache, 340);
        return 1;
     }
     return 0;