libafs: Drop xvcache for AllocCBR

Normally when we AllocCBR, we are holding xvcache write-locked, since
it is called from FlushVCache. Before
a309e274632993c5aeec04c6e090f5ac95837a40, when AllocCBR needs to flush
CBRs due to a lack of space, we hit the net, giving up callbacks on
fileservers.

This can cause a problem if one of those fileservers needs to contact
us in order to complete that request, since the callback service
thread may be waiting for xvcache, causing a deadlock (that is
eventually broken by network timeouts).

To avoid this, drop xvcache if AllocCBR looks like it does not have
sufficient space. Fix all callers of afs_FlushVCache to handle the
case where we sleep, since with this change, afs_FlushVCache can sleep
on all platforms.

This partially reverts a309e274632993c5aeec04c6e090f5ac95837a40, as it
contains an alternative method of avoiding the xvcache lock in this
situation. This commit restores much of the code path to be much more
similar to how it used to be, except that it allows for dropping
xvcache for AllocCBR. This should make any change to our prior
behavior smaller/simpler, and thus safer and more consistent with
existing clients. This reintroduces the hard limit to how much space
we allocate for CBRs, although the part of
a309e274632993c5aeec04c6e090f5ac95837a40 that raised this limit is
retained.

Change-Id: Id4aaa941b3908f59390873e83e23429041c0828f
Reviewed-on: http://gerrit.openafs.org/3958
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

diff --git a/src/afs/IRIX/osi_vfsops.c b/src/afs/IRIX/osi_vfsops.c
index d8bfda3274fe027c36578cec54aa03931905de96..7aa2a316f55435b7a3b5a6eda65bb9c8c5263183 100644 (file)
--- a/src/afs/IRIX/osi_vfsops.c
+++ b/src/afs/IRIX/osi_vfsops.c
@@ -216,11 +216,12 @@ afs_unmount(OSI_VFS_ARG(afsp), flags, cr)
      * EBUSY if any still in use
      */
     ObtainWriteLock(&afs_xvcache, 172);
+ retry:
     for (tq = VLRU.prev; tq != &VLRU; tq = uq) {
        tvc = QTOV(tq);
        uq = QPrev(tq);
        vp = (vnode_t *) tvc;
-       if (error = afs_FlushVCache(tvc, &fv_slept))
+       if (error = afs_FlushVCache(tvc, &fv_slept)) {
            if (vp->v_flag & VROOT) {
                rootvp = vp;
                continue;
@@ -228,6 +229,10 @@ afs_unmount(OSI_VFS_ARG(afsp), flags, cr)
                ReleaseWriteLock(&afs_xvcache);
                return error;
            }
+       }
+       if (fv_slept) {
+           goto retry;
+       }
     }
 
     /*

diff --git a/src/afs/LINUX/osi_vfsops.c b/src/afs/LINUX/osi_vfsops.c
index 8abe6b149b1e74f7596a9ce0a74bc38d7a73588b..04ddafc9f0c90bb4fe2878589ddda6e3e5a3f959 100644 (file)
--- a/src/afs/LINUX/osi_vfsops.c
+++ b/src/afs/LINUX/osi_vfsops.c
@@ -477,13 +477,17 @@ osi_linux_free_inode_pages(void)
     struct vcache *tvc, *nvc;
     extern struct vcache *afs_vhashT[VCSIZE];
 
+ retry:
     for (i = 0; i < VCSIZE; i++) {
        for (tvc = afs_vhashT[i]; tvc; ) {
            int slept;
        
            nvc = tvc->hnext;
-           if (afs_FlushVCache(tvc, &slept))           /* slept always 0 for linux? */
+           if (afs_FlushVCache(tvc, &slept))
                printf("Failed to invalidate all pages on inode 0x%p\n", tvc);
+           if (slept) {
+               goto retry;
+           }
            tvc = nvc;
        }
     }

diff --git a/src/afs/LINUX24/osi_vfsops.c b/src/afs/LINUX24/osi_vfsops.c
index 81aa3a019dabf27e916f130b555a9ed215c69d03..702177c0a3dfcf28d33685da9680abd27074cd60 100644 (file)
--- a/src/afs/LINUX24/osi_vfsops.c
+++ b/src/afs/LINUX24/osi_vfsops.c
@@ -465,13 +465,17 @@ osi_linux_free_inode_pages(void)
     struct vcache *tvc, *nvc;
     extern struct vcache *afs_vhashT[VCSIZE];
 
+ retry:
     for (i = 0; i < VCSIZE; i++) {
        for (tvc = afs_vhashT[i]; tvc; ) {
            int slept;
        
            nvc = tvc->hnext;
-           if (afs_FlushVCache(tvc, &slept))           /* slept always 0 for linux? */
+           if (afs_FlushVCache(tvc, &slept))
                printf("Failed to invalidate all pages on inode 0x%p\n", tvc);
+           if (slept) {
+               goto retry;
+           }
            tvc = nvc;
        }
     }

diff --git a/src/afs/afs.h b/src/afs/afs.h
index 73d59474aad8af04e9b23b0bba3ac197c417254e..ac0699814fbbb38fc9614d6340038bc2b1a2c455 100644 (file)
--- a/src/afs/afs.h
+++ b/src/afs/afs.h
@@ -260,7 +260,6 @@ struct afs_cbr {
     struct afs_cbr *hash_next;
 
     struct AFSFid fid;
-    unsigned int dynalloc:1;
 };
 
 #ifdef AFS_LINUX22_ENV

diff --git a/src/afs/afs_call.c b/src/afs/afs_call.c
index 1db4b89005fa0615b872dd479e1213aa3050a6fd..0ed56ef27b37e63aa7bb142f3a640964ad4e6529 100644 (file)
--- a/src/afs/afs_call.c
+++ b/src/afs/afs_call.c
@@ -1366,7 +1366,6 @@ afs_shutdown(void)
 
     if (afs_shuttingdown)
        return;
-    afs_FlushVCBs(2);       /* Reasonable effort to free dynamically allocated callback returns */
 
     afs_shuttingdown = 1;
     if (afs_cold_shutdown)

diff --git a/src/afs/afs_vcache.c b/src/afs/afs_vcache.c
index 55128d80b731ba9943c9e70982c0a9a072f01c7b..45118d73fbe3a161b554e8d8fec2ba8c0ae84563 100644 (file)
--- a/src/afs/afs_vcache.c
+++ b/src/afs/afs_vcache.c
@@ -83,7 +83,7 @@ static int afs_nextVcacheSlot = 0;
 static struct afs_slotlist *afs_freeSlotList = NULL;
 
 /* Forward declarations */
-static afs_int32 afs_QueueVCB(struct vcache *avc);
+static afs_int32 afs_QueueVCB(struct vcache *avc, int *slept);
 
 /*!
  * Generate an index into the hash table for a given Fid.
@@ -206,7 +206,7 @@ afs_FlushVCache(struct vcache *avc, int *slept)
 #endif
     afs_FreeAllAxs(&(avc->Access));
     if (!afs_shuttingdown)
-       afs_QueueVCB(avc);
+       afs_QueueVCB(avc, slept);
     ObtainWriteLock(&afs_xcbhash, 460);
     afs_DequeueCallback(avc);  /* remove it from queued callbacks list */
     avc->f.states &= ~(CStatd | CUnique);
@@ -301,14 +301,10 @@ afs_AllocCBR(void)
     struct afs_cbr *tsp;
     int i;
 
-    if (!afs_cbrSpace) {
-       afs_osi_CancelWait(&AFS_WaitHandler);   /* trigger FlushVCBs asap */
-
+    while (!afs_cbrSpace) {
        if (afs_stats_cmperf.CallBackAlloced >= sizeof(afs_cbrHeads)/sizeof(afs_cbrHeads[0])) {
            /* don't allocate more than 16 * AFS_NCBRS for now */
-           tsp = (struct afs_cbr *)osi_AllocSmallSpace(sizeof(*tsp));
-           tsp->dynalloc = 1;
-           tsp->next = NULL;
+           afs_FlushVCBs(0);
            afs_stats_cmperf.CallBackFlushes++;
        } else {
            /* try allocating */
@@ -316,18 +312,15 @@ afs_AllocCBR(void)
            osi_Assert(tsp != NULL);
            for (i = 0; i < AFS_NCBRS - 1; i++) {
                tsp[i].next = &tsp[i + 1];
-               tsp[i].dynalloc = 0;
            }
            tsp[AFS_NCBRS - 1].next = 0;
-           tsp[AFS_NCBRS - 1].dynalloc = 0;
-           afs_cbrSpace = tsp->next;
+           afs_cbrSpace = tsp;
            afs_cbrHeads[afs_stats_cmperf.CallBackAlloced] = tsp;
            afs_stats_cmperf.CallBackAlloced++;
        }
-    } else {
-       tsp = afs_cbrSpace;
-       afs_cbrSpace = tsp->next;
     }
+    tsp = afs_cbrSpace;
+    afs_cbrSpace = tsp->next;
     return tsp;
 }
 
@@ -351,12 +344,8 @@ afs_FreeCBR(struct afs_cbr *asp)
     if (asp->hash_next)
        asp->hash_next->hash_pprev = asp->hash_pprev;
 
-    if (asp->dynalloc) {
-       osi_FreeSmallSpace(asp);
-    } else {
-       asp->next = afs_cbrSpace;
-       afs_cbrSpace = asp;
-    }
+    asp->next = afs_cbrSpace;
+    afs_cbrSpace = asp;
     return 0;
 }
 
@@ -533,17 +522,20 @@ afs_FlushVCBs(afs_int32 lockit)
  * Environment:
  *     Locks the xvcb lock.
  *     Called when the xvcache lock is already held.
+ * RACE: afs_xvcache may be dropped and reacquired
  *
  * \param avc vcache entry
+ * \param slep Set to 1 if we dropped afs_xvcache
  * \return 1 if queued, 0 otherwise
  */
 
 static afs_int32
-afs_QueueVCB(struct vcache *avc)
+afs_QueueVCB(struct vcache *avc, int *slept)
 {
     int queued = 0;
     struct server *tsp;
     struct afs_cbr *tcbp;
+    int reacquire = 0;
 
     AFS_STATCNT(afs_QueueVCB);
 
@@ -560,6 +552,15 @@ afs_QueueVCB(struct vcache *avc)
     /* The callback is really just a struct server ptr. */
     tsp = (struct server *)(avc->callback);
 
+    if (!afs_cbrSpace) {
+       /* If we don't have CBR space, AllocCBR may block or hit the net for
+        * clearing up CBRs. Hitting the net may involve a fileserver
+        * needing to contact us, so we must drop xvcache so we don't block
+        * those requests from going through. */
+       reacquire = *slept = 1;
+       ReleaseWriteLock(&afs_xvcache);
+    }
+
     /* we now have a pointer to the server, so we just allocate
      * a queue entry and queue it.
      */
@@ -579,6 +580,11 @@ afs_QueueVCB(struct vcache *avc)
  done:
     /* now release locks and return */
     ReleaseWriteLock(&afs_xvcb);
+
+    if (reacquire) {
+       /* make sure this is after dropping xvcb, for locking order */
+       ObtainWriteLock(&afs_xvcache, 279);
+    }
     return queued;
 }
 
@@ -3114,13 +3120,18 @@ afs_DisconGiveUpCallbacks(void)
 
     ObtainWriteLock(&afs_xvcache, 1002); /* XXX - should be a unique number */
 
+ retry:
     /* Somehow, walk the set of vcaches, with each one coming out as tvc */
     for (i = 0; i < VCSIZE; i++) {
         for (tvc = afs_vhashT[i]; tvc; tvc = tvc->hnext) {
-            if (afs_QueueVCB(tvc)) {
+           int slept = 0;
+            if (afs_QueueVCB(tvc, &slept)) {
                 tvc->callback = NULL;
                 nq++;
             }
+           if (slept) {
+               goto retry;
+           }
         }
     }