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.

Reviewed-on: http://gerrit.openafs.org/3958
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 76158df491f47de56d1febe1d1d2d17d316c9a74)

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

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 c23a0498ad4ecb8e822d11682098483e4d7ed4ab..c44c2bb6f4addaed06ca3cd72baa1c24ee4656a1 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 ddacaf7f6acb5985a7686574e6babdb55153bcde..02db212b9c0676ef1a8f29d25da2afb3ae5da36a 100644 (file)
--- a/src/afs/afs_call.c
+++ b/src/afs/afs_call.c
@@ -1245,7 +1245,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 58c56cd4ae164b1e772d93a641fac21657ba990c..3d79d23d2ef921cfd9927e76373ef3ef6562e333 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.
@@ -205,7 +205,8 @@ afs_FlushVCache(struct vcache *avc, int *slept)
     vn_reinit(AFSTOV(avc));
 #endif
     afs_FreeAllAxs(&(avc->Access));
-    afs_QueueVCB(avc);
+    if (!afs_shuttingdown)
+       afs_QueueVCB(avc, slept);
     ObtainWriteLock(&afs_xcbhash, 460);
     afs_DequeueCallback(avc);  /* remove it from queued callbacks list */
     avc->f.states &= ~(CStatd | CUnique);
@@ -300,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 +313,15 @@ afs_AllocCBR(void)
                                                sizeof(struct afs_cbr));
            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 +345,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;
 }
 
@@ -486,17 +476,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);
 
@@ -513,6 +506,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.
      */
@@ -532,6 +534,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;
 }
 
@@ -3068,13 +3075,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;
+           }
         }
     }