FBSD CM: don't call afs_close when recycling
authorMatt Benjamin <matt@linuxbox.com>
Wed, 25 Aug 2010 07:34:35 +0000 (03:34 -0400)
committerDerrick Brashear <shadow@dementia.org>
Wed, 25 Aug 2010 14:54:16 +0000 (07:54 -0700)
Don't call afs_close when handling VOP_CLOSE on a recycled
vnode, since there was no matching open.  This corrects the
opens count, which was seen to go have gone negative in the
reclaim vop.  For clarity, assert if afs_vop_close is entered
with a VI_DOOMED vnode and avc->opens != 0.

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

src/afs/FBSD/osi_vcache.c
src/afs/FBSD/osi_vm.c
src/afs/FBSD/osi_vnodeops.c
src/afs/VNOPS/afs_vnop_write.c

index 52d7ae3..c2060c7 100644 (file)
@@ -36,15 +36,15 @@ osi_TryEvictVCache(struct vcache *avc, int *slept) {
        *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(vp) < 1) {
+       /* vgone() is correct, but vgonel() panics if v_usecount is 0--
+         * this is particularly confusing since vgonel() will trigger
+         * vop_reclaim, in the call path of which we'll check v_usecount
+         * and decide that the vnode is busy.  Splat. */
+       if (vrefcnt(vp) < 1)
            vref(vp);
-       }
+
        vn_lock(vp, LK_EXCLUSIVE | LK_RETRY); /* !glocked */
 #endif
-
         vgone(vp);
 #if defined(AFS_FBSD80_ENV)
        VOP_UNLOCK(vp, 0);
index 0a813e8..6f49d7d 100644 (file)
@@ -89,15 +89,22 @@ osi_VM_FlushVCache(struct vcache *avc, int *slept)
 {
     struct vm_object *obj;
     struct vnode *vp;
-    if (VREFCOUNT(avc) > 1)
+    if (VREFCOUNT(avc) > 1) {
        return EBUSY;
+    }
 
-    if (avc->opens)
+    /* XXX
+     * The value of avc->opens here came to be, at some point,
+     * typically -1.  This was caused by incorrectly performing afs_close
+     * processing on vnodes being recycled */
+    if (avc->opens) {
        return EBUSY;
+    }
 
     /* if a lock is held, give up */
-    if (CheckLock(&avc->lock))
+    if (CheckLock(&avc->lock)) {
        return EBUSY;
+    }
 
     return(0);
 
index 12cacb0..ea3aa34 100644 (file)
@@ -666,8 +666,24 @@ afs_vop_close(ap)
                                 * struct thread *a_td;
                                 * } */ *ap;
 {
-    int code;
-    struct vcache *avc = VTOAFS(ap->a_vp);
+    int code, iflag;
+    struct vnode *vp = ap->a_vp;
+    struct vcache *avc = VTOAFS(vp);
+
+#if defined(AFS_FBSD80_ENV)
+    VI_LOCK(vp);
+    iflag = vp->v_iflag & VI_DOOMED;
+    VI_UNLOCK(vp);
+    if (iflag & VI_DOOMED) {
+        /* osi_FlushVCache (correctly) calls vgone() on recycled vnodes, we don't
+         * have an afs_close to process, in that case */
+        if (avc->opens != 0)
+            panic("afs_vop_close: doomed vnode %p has vcache %p with non-zero opens %d\n",
+                  vp, avc, avc->opens);
+        return 0;
+    }
+#endif
+
     AFS_GLOCK();
     if (ap->a_cred)
        code = afs_close(avc, ap->a_fflag, ap->a_cred);
@@ -1473,12 +1489,8 @@ afs_vop_reclaim(struct vop_reclaim_args *ap)
     if (!haveGlock)
        AFS_GUNLOCK();
 
-    /*
-     * XXX Pretend it worked, to prevent panic on shutdown
-     * Garrett, please fix - Jim Rees
-     */
     if (code) {
-       printf("afs_vop_reclaim: afs_FlushVCache failed code %d vnode\n", code);
+       afs_warn("afs_vop_reclaim: afs_FlushVCache failed code %d vnode\n", code);
        VOP_PRINT(vp);
     }
 
@@ -1539,7 +1551,7 @@ afs_vop_print(ap)
     struct vcache *vc = VTOAFS(ap->a_vp);
     int s = vc->f.states;
 
-    printf("tag %s, fid: %d.%d.%d.%d, opens %d, writers %d", vp->v_tag,
+    printf("vc %p vp %p tag %s, fid: %d.%d.%d.%d, opens %d, writers %d", vc, vp, vp->v_tag,
           (int)vc->f.fid.Cell, (u_int) vc->f.fid.Fid.Volume,
           (u_int) vc->f.fid.Fid.Vnode, (u_int) vc->f.fid.Fid.Unique, vc->opens,
           vc->execsOrWriters);
index 49367b3..ac0894e 100644 (file)
@@ -815,6 +815,22 @@ afs_close(OSI_VC_DECL(avc), afs_int32 aflags, afs_ucred_t *acred)
            code = avc->vc_error;
            avc->vc_error = 0;
        }
+#if defined(AFS_FBSD80_ENV)
+        /* XXX */
+        if (!avc->opens) {
+            afs_int32 opens, is_free, is_gone, is_doomed, iflag;
+            struct vnode *vp = AFSTOV(avc);
+            VI_LOCK(vp);
+            is_doomed =  vp->v_iflag & VI_DOOMED;
+            is_free = vp->v_iflag & VI_FREE;
+            is_gone = vp->v_iflag & VI_DOINGINACT;
+            iflag = vp->v_iflag;
+            VI_UNLOCK(vp);
+            opens = avc->opens;
+            afs_warn("afs_close avc %p vp %p opens %d free %d doinginact %d doomed %d iflag %d\n",
+                     avc, vp, opens, is_free, is_gone, is_doomed, iflag);
+        }
+#endif
        avc->opens--;
        ReleaseWriteLock(&avc->lock);
     }