FBSD: correct and simplify vcache eviction routines

osi_VM_FlushVCache and osi_TryEvictVCache were both attempting
to be wrappers around vgone(), with some checks before hand.
Implement the latter in terms of the former to prevent
code duplication and propagation of incorrect code.

Additionally, correct the locking around vgone().  The
vnode lock must be held, and we must also increase the vnode's
hold count so that it does not disappear out from under us.
As we need the interlock to check the usecount, keep it
locked until we lock the vnode lock, for extra protection.

As an added bonus, we no longer try to call vgonel(), which
is not an exported symbol and merely happened to work due
to the current kernel linker implementation.

Remove some stale comments.

With this change, a parallel buildworld completes on
my four-core machine.

Reviewed-on: http://gerrit.openafs.org/3196
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Derrick Brashear <shadow@dementia.org>
(cherry picked from commit aad83a30a82407bfa6ac15b49fd31d69b563e898)
Change-Id: Ibf7f1744f0030c92b45b1558d7f5e52409208e60
Reviewed-on: http://gerrit.openafs.org/3233
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/afs/FBSD/osi_vcache.c b/src/afs/FBSD/osi_vcache.c
index c2060c74f0155a610d2ea94f3c7f508e8ca4373a..d00ba68b4a6cf86854dd48ca2275c893da006dd5 100644 (file)
--- a/src/afs/FBSD/osi_vcache.c
+++ b/src/afs/FBSD/osi_vcache.c
@@ -17,44 +17,18 @@ 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) {
-       /*
-         * vgone() reclaims the vnode, which calls afs_FlushVCache(),
-         * then it puts the vnode on the free list.
-         * If we don't do this we end up with a cleaned vnode that's
-         * 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 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);
-#endif
-
-       AFS_GLOCK();
-       ObtainWriteLock(&afs_xvcache, 340);
+    /*
+     * essentially all we want to do here is check that the
+     * vcache is not in use, then call vgone() (which will call
+     * inactive and reclaim as needed).  This requires some
+     * kind of complicated locking, which we already need to implement
+     * for FlushVCache, so just call that routine here and check
+     * its return value for whether the vcache was evict-able.
+     */
+    if (osi_VM_FlushVCache(avc, slept) != 0)
+       return 0;
+    else
        return 1;
-    }
-    return 0;
 }
 
 struct vcache *

diff --git a/src/afs/FBSD/osi_vm.c b/src/afs/FBSD/osi_vm.c
index 6f49d7d232e8fe1a3d52623c77ade825eadf0c18..571fb67c10cc63f8ca0433178e61c8b7309e0922 100644 (file)
--- a/src/afs/FBSD/osi_vm.c
+++ b/src/afs/FBSD/osi_vm.c
@@ -20,12 +20,8 @@
 
 #include <afsconfig.h>
 #include "afs/param.h"
-#ifdef AFS_FBSD70_ENV
 #include <sys/param.h>
 #include <sys/vnode.h>
-     void
-     vgonel(struct vnode *vp, struct thread *td);
-#endif
 
 
 #include "afs/sysincludes.h"   /* Standard vendor system headers */
@@ -58,9 +54,11 @@
 
 #if defined(AFS_FBSD80_ENV)
 #define        lock_vnode(v)   vn_lock((v), LK_EXCLUSIVE | LK_RETRY)
+#define ilock_vnode(v) vn_lock((v), LK_INTERLOCK|LK_EXCLUSIVE|LK_RETRY);
 #define unlock_vnode(v)        VOP_UNLOCK((v), 0)
 #else
 #define        lock_vnode(v)   vn_lock((v), LK_EXCLUSIVE | LK_RETRY, curthread)
+#define ilock_vnode(v) vn_lock((v), LK_INTERLOCK|LK_EXCLUSIVE|LK_RETRY, curthread);
 #define unlock_vnode(v)        VOP_UNLOCK((v), 0, curthread)
 #endif
 
@@ -78,18 +76,18 @@
  * is not dropped and re-acquired for any platform.  It may be that *slept is
  * therefore obsolescent.
  *
- * OSF/1 Locking:  VN_LOCK has been called.
- * We do not lock the vnode here, but instead require that it be exclusive
- * locked by code calling osi_VM_StoreAllSegments directly, or scheduling it
- * from the bqueue - Matt
- * Maybe better to just call vnode_pager_setsize()?
  */
 int
 osi_VM_FlushVCache(struct vcache *avc, int *slept)
 {
     struct vm_object *obj;
-    struct vnode *vp;
-    if (VREFCOUNT(avc) > 1) {
+    struct vnode *vp = AFSTOV(avc);
+
+    if (!VI_TRYLOCK(vp)) /* need interlock to check usecount */
+       return EBUSY;
+
+    if (vp->v_usecount > 0) {
+       VI_UNLOCK(vp);
        return EBUSY;
     }
 
@@ -98,39 +96,33 @@ osi_VM_FlushVCache(struct vcache *avc, int *slept)
      * typically -1.  This was caused by incorrectly performing afs_close
      * processing on vnodes being recycled */
     if (avc->opens) {
+       VI_UNLOCK(vp);
        return EBUSY;
     }
 
     /* if a lock is held, give up */
     if (CheckLock(&avc->lock)) {
+       VI_UNLOCK(vp);
        return EBUSY;
     }
 
-    return(0);
+    if ((vp->v_iflag & VI_DOOMED) != 0) {
+       VI_UNLOCK(vp);
+       return (0);
+    }
 
+    /* must hold the vnode before calling vgone()
+     * This code largely copied from vfs_subr.c:vlrureclaim() */
+    vholdl(vp);
     AFS_GUNLOCK();
-    vp = AFSTOV(avc);
-#ifndef AFS_FBSD70_ENV
-    lock_vnode(vp);
-#endif
-    if (VOP_GETVOBJECT(vp, &obj) == 0) {
-       VM_OBJECT_LOCK(obj);
-       vm_object_page_remove(obj, 0, 0, FALSE);
-#if 1
-       if (obj->ref_count == 0) {
-           simple_lock(&vp->v_interlock);
-           vgonel(vp, curthread);
-           vp->v_tag = VT_AFS;
-           SetAfsVnode(vp);
-       }
-#endif
-       VM_OBJECT_UNLOCK(obj);
-    }
-#ifndef AFS_FBSD70_ENV
+    *slept = 1;
+    /* use the interlock while locking, so no one else can DOOM this */
+    ilock_vnode(vp);
+    vgone(vp);
     unlock_vnode(vp);
-#endif
-    AFS_GLOCK();
+    vdrop(vp);
 
+    AFS_GLOCK();
     return 0;
 }