From 97ebc776712b455b1e85df598b61ba6c847ca0a6 Mon Sep 17 00:00:00 2001 From: Chaskiel M Grundman Date: Sat, 24 Dec 2005 00:20:18 +0000 Subject: [PATCH] tiger-fixes-20051215 potential reclaim in progress fix, and per Chaskiel, "I don't remember why I put it there, but the fact that it gets triggered means that we're leaking a vcache object lock. It looks like the "rename to .__afsXXXX" codepath is responsible (as afsrename does not use the fact that adp (or aodp) is locked by afs_remove, and locks it again. I'm surprised it's not deadlocking)" so i coded up a fix ==================== This delta was composed from multiple commits as part of the CVS->Git migration. The checkin message with each commit was inconsistent. The following are the additional commit messages. ==================== chaskiel says The RHS shouldn't be a double negative... There's no bug (other than the assert itself) --- src/afs/VNOPS/afs_vnop_remove.c | 2 +- src/afs/afs_vcache.c | 35 +++++++++++++++++++++------------ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/afs/VNOPS/afs_vnop_remove.c b/src/afs/VNOPS/afs_vnop_remove.c index 5ea337ba7..92e344f0c 100644 --- a/src/afs/VNOPS/afs_vnop_remove.c +++ b/src/afs/VNOPS/afs_vnop_remove.c @@ -404,7 +404,7 @@ afs_remove(OSI_VC_ARG(adp), aname, acred) code = afsremove(adp, tdc, tvc, aname, acred, &treq); } afs_PutFakeStat(&fakestate); - osi_Assert(!WriteLocked(&adp->lock) || !(adp->lock.pid_writer != MyPidxx)); + osi_Assert(!WriteLocked(&adp->lock) || (adp->lock.pid_writer != MyPidxx)); return code; } diff --git a/src/afs/afs_vcache.c b/src/afs/afs_vcache.c index 0aba05759..b059ef18d 100644 --- a/src/afs/afs_vcache.c +++ b/src/afs/afs_vcache.c @@ -759,19 +759,28 @@ restart: && tvc->opens == 0 && (tvc->states & CUnlinkedDel) == 0) { #if defined (AFS_DARWIN_ENV) || defined(AFS_XBSD_ENV) #ifdef AFS_DARWIN80_ENV - fv_slept=1; - /* must release lock, since vnode_recycle will immediately - reclaim if there are no other users */ - ReleaseWriteLock(&afs_xvcache); - AFS_GUNLOCK(); - /* VREFCOUNT_GT only sees usecounts, not iocounts */ - /* so this may fail to actually recycle the vnode now */ - if (vnode_recycle(AFSTOV(tvc))) - code=0; - else - code=EBUSY; - AFS_GLOCK(); - ObtainWriteLock(&afs_xvcache, 336); + vnode_t tvp = AFSTOV(tvc); + fv_slept=1; + /* must release lock, since vnode_recycle will immediately + reclaim if there are no other users */ + ReleaseWriteLock(&afs_xvcache); + AFS_GUNLOCK(); + /* VREFCOUNT_GT only sees usecounts, not iocounts */ + /* so this may fail to actually recycle the vnode now */ + /* must call vnode_get to avoid races. */ + if (vnode_get(tvp) == 0) { + vnode_recycle(tvp); + vnode_put(tvp); + } + AFS_GLOCK(); + ObtainWriteLock(&afs_xvcache, 336); + /* we can't use the vnode_recycle return value to figure + * this out, since the iocount we have to hold makes it + * always "fail" */ + if (AFSTOV(tvc) == tvp) + code = EBUSY; + else + code = 0; #else /* * vgone() reclaims the vnode, which calls afs_FlushVCache(), -- 2.39.5