]> git.michaelhowe.org Git - packages/o/openafs.git/commitdiff
xserver lock order violation
authorDerrick Brashear <shadow@dementix.org>
Tue, 23 Aug 2011 04:20:37 +0000 (00:20 -0400)
committerDerrick Brashear <shadow@dementix.org>
Fri, 16 Dec 2011 11:04:36 +0000 (03:04 -0800)
individual volume locks are pretty far down, well after afs_xserver.

afs_SetupVolume (with tv->lock)-> InstallUVolumeEntry-> afs_GetServer.

Install*Volume is careful to protect against recursing into the volume
lock via ResetVolumeInfo. Unfortunately, GetServer acquires xserver,
and then if it needs to call GetCapabilities, it drops and reacquires
xserver.

turns out the volume locks weren't protecting much. they also aren't
grabbed before xvolume is dropped. fine, so, restructure to do all the
work, then merge the result.

Reviewed-on: http://gerrit.openafs.org/5303
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
(cherry picked from commit 16dff61e148ce6893a68dda6e05e84f96fa753ac)

Change-Id: I7ca73fe9cf76e9a47cdccfc6cf0e9188fce9f5a6
Reviewed-on: http://gerrit.openafs.org/6309
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
src/afs/DOC/afs_rwlocks
src/afs/afs_analyze.c
src/afs/afs_cell.c
src/afs/afs_pioctl.c
src/afs/afs_prototypes.h
src/afs/afs_server.c
src/afs/afs_volume.c

index ce28ce111c54cf9b63807181c2719382f43d41ff..3ce9cff56dbde52309a54ee5093fafc0f904c2c0 100644 (file)
@@ -66,6 +66,10 @@ iterate over all volumes without others being inserted/deleted.  Same
 hack doesn't work for cache entry locks since we need to be able to
 lock multiple cache entries (but not multiple volumes) simultaneously.
 
+In practice this appears to only be used to protect the status, name,
+and root vnode and uniq. other users are not excluded, although
+exclusion of multiple installs of a volume entry have been poorly done.
+
 15. afs_xdnlc -- locked after afs_xvcache in afs_osidnlc.c.  Shouldn't 
 interact with any of the other locks. 
 
index a5333a1967030373eaebc4cf365b354d38cbb57c..4defe46852f7681b587ecc88e8ac7dab8f420141 100644 (file)
@@ -174,13 +174,14 @@ VLDB_Same(struct VenusFid *afid, struct vrequest *areq)
        for (i = 0; i < NMAXNSERVERS && tvp->serverHost[i]; i++) {
            oldhosts[i] = tvp->serverHost[i];
        }
+       ReleaseWriteLock(&tvp->lock);
 
        if (type == 2) {
-           InstallUVolumeEntry(tvp, &v->utve, afid->Cell, tcell, &treq);
+           LockAndInstallUVolumeEntry(tvp, &v->utve, afid->Cell, tcell, &treq);
        } else if (type == 1) {
-           InstallNVolumeEntry(tvp, &v->ntve, afid->Cell);
+           LockAndInstallNVolumeEntry(tvp, &v->ntve, afid->Cell);
        } else {
-           InstallVolumeEntry(tvp, &v->tve, afid->Cell);
+           LockAndInstallVolumeEntry(tvp, &v->tve, afid->Cell);
        }
 
        if (i < NMAXNSERVERS && tvp->serverHost[i]) {
index 9b3fd9a176cea1c2e3af3eb83b4c3eb2d59f915b..f68868992a2e310eb24041c084ac7b16e4d39d76 100644 (file)
@@ -978,7 +978,7 @@ afs_NewCell(char *acellName, afs_int32 * acellHosts, int aflags,
        afs_uint32 temp = acellHosts[i];
        if (!temp)
            break;
-       ts = afs_GetServer(&temp, 1, 0, tc->vlport, WRITE_LOCK, NULL, 0);
+       ts = afs_GetServer(&temp, 1, 0, tc->vlport, WRITE_LOCK, NULL, 0, NULL);
        ts->cell = tc;
        ts->flags &= ~SRVR_ISGONE;
        /* Set the server as a host of the new cell. */
index 9e64b03f4397a83c749724b7432856dde55c73c8..bc6f5e24f9422eec17fc05a3e63270aeec5a5d97 100644 (file)
@@ -3903,7 +3903,7 @@ afs_setsprefs(struct spref *sp, unsigned int num, unsigned int vlonly)
            afs_uint32 temp = sp->host.s_addr;
            srvr =
                afs_GetServer(&temp, 1, 0, (vlonly ? AFS_VLPORT : AFS_FSPORT),
-                             WRITE_LOCK, (afsUUID *) 0, 0);
+                             WRITE_LOCK, (afsUUID *) 0, 0, NULL);
            srvr->addr->sa_iprank = sp->rank + afs_randomMod15();
            afs_PutServer(srvr, WRITE_LOCK);
        }
index 73ba1402b942f6d31aaf84d1dc5547e29964956d..59b84117bca84471a27685b83a50f4c33d6c3477 100644 (file)
@@ -846,7 +846,8 @@ extern struct server *afs_FindServer(afs_int32 aserver, afs_uint16 aport,
 extern struct server *afs_GetServer(afs_uint32 * aserver, afs_int32 nservers,
                                    afs_int32 acell, u_short aport,
                                    afs_int32 locktype, afsUUID * uuidp,
-                                   afs_int32 addr_uniquifier);
+                                   afs_int32 addr_uniquifier,
+                                   struct volume *tv);
 extern void afs_GetCapabilities(struct server *ts);
 extern void ForceAllNewConnections(void);
 extern void afs_MarkServerUpOrDown(struct srvAddr *sa, int a_isDown);
@@ -858,7 +859,6 @@ extern int afs_randomMod15(void);
 extern int afs_randomMod127(void);
 extern void afs_SortOneServer(struct server *asp);
 extern void afs_SortServers(struct server *aservers[], int count);
-extern void afs_RemoveSrvAddr(struct srvAddr *sap);
 extern void afs_ActivateServer(struct srvAddr *sap);
 #ifdef AFS_USERSPACE_IP_ADDR
 extern void afsi_SetServerIPRank(struct srvAddr *sa, afs_int32 addr,
@@ -1336,16 +1336,16 @@ extern struct volume *afs_FindVolume(struct VenusFid *afid,
                                     afs_int32 locktype);
 extern struct volume *afs_freeVolList;
 extern afs_int32 fvTable[NFENTRIES];
-extern void InstallVolumeEntry(struct volume *av, struct vldbentry *ve,
-                              int acell);
-extern void InstallNVolumeEntry(struct volume *av, struct nvldbentry *ve,
-                               int acell);
-extern void InstallUVolumeEntry(struct volume *av, struct uvldbentry *ve,
-                               int acell, struct cell *tcell,
-                               struct vrequest *areq);
+extern void LockAndInstallVolumeEntry(struct volume *av, struct vldbentry *ve,
+                                     int acell);
+extern void LockAndInstallNVolumeEntry(struct volume *av, struct nvldbentry *ve,
+                                      int acell);
+extern void LockAndInstallUVolumeEntry(struct volume *av, struct uvldbentry *ve,
+                                      int acell, struct cell *tcell,
+                                      struct vrequest *areq);
 extern void afs_ResetVolumeInfo(struct volume *tv);
 extern struct volume *afs_MemGetVolSlot(void);
-extern void afs_ResetVolumes(struct server *srvp);
+extern void afs_ResetVolumes(struct server *srvp, struct volume *tv);
 extern struct volume *afs_GetVolume(struct VenusFid *afid,
                                    struct vrequest *areq,
                                    afs_int32 locktype);
index 122fbfb487ab9e556fee762175b074a6acee6985..c9cc0cd145395b2f939706b658566eff88df5b9b 100644 (file)
@@ -1615,16 +1615,16 @@ afs_SetServerPrefs(struct srvAddr *sa)
  * The afs_xserver, afs_xvcb and afs_xsrvAddr locks are assumed taken.
  */
 static void
-afs_FlushServer(struct server *srvp)
+afs_FlushServer(struct server *srvp, struct volume *tv)
 {
     afs_int32 i;
     struct server *ts, **pts;
 
     /* Find any volumes residing on this server and flush their state */
-      afs_ResetVolumes(srvp);
+    afs_ResetVolumes(srvp, tv);
 
     /* Flush all callbacks in the all vcaches for this specific server */
-      afs_FlushServerCBs(srvp);
+    afs_FlushServerCBs(srvp);
 
     /* Remove all the callbacks structs */
     if (srvp->cbrs) {
@@ -1668,9 +1668,10 @@ afs_FlushServer(struct server *srvp)
  * remains connected to a server struct.
  * The afs_xserver and afs_xsrvAddr locks are assumed taken.
  *    It is not removed from the afs_srvAddrs hash chain.
+ * If resetting volumes, do not reset volume tv
  */
-void
-afs_RemoveSrvAddr(struct srvAddr *sap)
+static void
+afs_RemoveSrvAddr(struct srvAddr *sap, struct volume *tv)
 {
     struct srvAddr **psa, *sa;
     struct server *srv;
@@ -1689,7 +1690,7 @@ afs_RemoveSrvAddr(struct srvAddr *sap)
        sa->server = 0;
 
        /* Flush the server struct since it's IP address has changed */
-       afs_FlushServer(srv);
+       afs_FlushServer(srv, tv);
     }
 }
 
@@ -1767,16 +1768,36 @@ afs_SearchServer(u_short aport, afsUUID * uuidp, afs_int32 locktype,
     return NULL;
 }
 
-/* afs_GetServer()
- * Return an updated and properly initialized server structure
- * corresponding to the server ID, cell, and port specified.
- * If one does not exist, then one will be created.
- * aserver and aport must be in NET byte order.
+/*!
+ * Return an updated and properly initialized server structure.
+ *
+ * Takes a server ID, cell, and port.
+ * If server does not exist, then one will be created.
+ * @param[in] aserverp
+ *      The server address in network byte order
+ * @param[in] nservers
+ *      The number of IP addresses claimed by the server
+ * @param[in] acell
+ *      The cell the server is in
+ * @param[in] aport
+ *      The port for the server (fileserver or vlserver) in network byte order
+ * @param[in] locktype
+ *      The type of lock to hold when iterating server hash (unused).
+ * @param[in] uuidp
+ *      The uuid for servers supporting one.
+ * @param[in] addr_uniquifier
+ *      The vldb-provider per-instantiated-server uniquifer counter.
+ * @param[in] tv
+ *      A volume not to reset information for if the server addresses
+ *      changed.
+ *
+ * @return
+ *      A server structure matching the request.
  */
 struct server *
-afs_GetServer(afs_uint32 * aserverp, afs_int32 nservers, afs_int32 acell,
+afs_GetServer(afs_uint32 *aserverp, afs_int32 nservers, afs_int32 acell,
              u_short aport, afs_int32 locktype, afsUUID * uuidp,
-             afs_int32 addr_uniquifier)
+             afs_int32 addr_uniquifier, struct volume *tv)
 {
     struct server *oldts = 0, *ts, *newts, *orphts = 0;
     struct srvAddr *oldsa, *newsa, *nextsa, *orphsa;
@@ -1791,7 +1812,7 @@ afs_GetServer(afs_uint32 * aserverp, afs_int32 nservers, afs_int32 acell,
     /* Check if the server struct exists and is up to date */
     if (!uuidp) {
        if (nservers != 1)
-           panic("afs_GetServer: incorect count of servers");
+           panic("afs_GetServer: incorrect count of servers");
        ObtainReadLock(&afs_xsrvAddr);
        ts = afs_FindServer(aserverp[0], aport, NULL, locktype);
        ReleaseReadLock(&afs_xsrvAddr);
@@ -1878,7 +1899,7 @@ afs_GetServer(afs_uint32 * aserverp, afs_int32 nservers, afs_int32 acell,
                break;
        }
        if (oldsa && (oldsa->server != newts)) {
-           afs_RemoveSrvAddr(oldsa);   /* Remove from its server struct */
+           afs_RemoveSrvAddr(oldsa, tv);       /* Remove from its server struct */
            oldsa->next_sa = newts->addr;       /* Add to the  new server struct */
            newts->addr = oldsa;
        }
@@ -1960,7 +1981,7 @@ afs_GetServer(afs_uint32 * aserverp, afs_int32 nservers, afs_int32 acell,
            /* Hang the srvAddr struct off of the server structure. The server
             * may have multiple srvAddrs, but it won't be marked multihomed.
             */
-           afs_RemoveSrvAddr(orphsa);  /* remove */
+           afs_RemoveSrvAddr(orphsa, tv);      /* remove */
            orphsa->next_sa = orphts->addr;     /* hang off server struct */
            orphts->addr = orphsa;
            orphsa->server = orphts;
@@ -1983,6 +2004,8 @@ afs_GetServer(afs_uint32 * aserverp, afs_int32 nservers, afs_int32 acell,
        if (afs_stats_cmperf.srvRecords > afs_stats_cmperf.srvRecordsHWM)
            afs_stats_cmperf.srvRecordsHWM = afs_stats_cmperf.srvRecords;
     }
+    /* We can't need this below, and won't reacquire */
+    ReleaseWriteLock(&afs_xvcb);
 
     ReleaseWriteLock(&afs_xsrvAddr);
 
index 5fd5a8a222a6944b0c8f1da5624d2f28b49c7c0a..3ca2d492e532f57d7ac4b7bf28d085cac469ea13 100644 (file)
@@ -256,13 +256,17 @@ afs_MemGetVolSlot(void)
 
 }                              /*afs_MemGetVolSlot */
 
-/**
- *   Reset volume information for all volume structs that
- * point to a speicific server.
- * @param srvp
+/*!
+ * Reset volume information for all volume structs that
+ * point to a speicific server, skipping a given volume if provided.
+ *
+ * @param[in] srvp
+ *      The server to reset volume info about
+ * @param[in] tv
+ *      The volume to skip resetting info about
  */
 void
-afs_ResetVolumes(struct server *srvp)
+afs_ResetVolumes(struct server *srvp, struct volume *tv)
 {
     int j, k;
     struct volume *vp;
@@ -272,8 +276,10 @@ afs_ResetVolumes(struct server *srvp)
        for (vp = afs_volumes[j]; vp; vp = vp->next) {
            for (k = 0; k < AFS_MAXHOSTS; k++) {
                if (!srvp || (vp->serverHost[k] == srvp)) {
-                   vp->serverHost[k] = 0;
-                   afs_ResetVolumeInfo(vp);
+                   if (tv && tv != vp) {
+                       vp->serverHost[k] = 0;
+                       afs_ResetVolumeInfo(vp);
+                   }
                    break;
                }
            }
@@ -281,7 +287,6 @@ afs_ResetVolumes(struct server *srvp)
     }
 }
 
-
 /**
  *   Reset volume name to volume id mapping cache.
  * @param flags
@@ -621,13 +626,12 @@ afs_SetupVolume(afs_int32 volid, char *aname, void *ve, struct cell *tcell,
     tv->states &= ~VRecheck;   /* just checked it */
     tv->accessTime = osi_Time();
     ReleaseWriteLock(&afs_xvolume);
-    ObtainWriteLock(&tv->lock, 111);
     if (type == 2) {
-       InstallUVolumeEntry(tv, uve, tcell->cellNum, tcell, areq);
+       LockAndInstallUVolumeEntry(tv, uve, tcell->cellNum, tcell, areq);
     } else if (type == 1)
-       InstallNVolumeEntry(tv, nve, tcell->cellNum);
+       LockAndInstallNVolumeEntry(tv, nve, tcell->cellNum);
     else
-       InstallVolumeEntry(tv, ove, tcell->cellNum);
+       LockAndInstallVolumeEntry(tv, ove, tcell->cellNum);
     if (agood) {
        if (!tv->name) {
            tv->name = afs_osi_Alloc(strlen(aname) + 1);
@@ -881,51 +885,38 @@ afs_NewVolumeByName(char *aname, afs_int32 acell, int agood,
  * @param acell
  */
 void
-InstallVolumeEntry(struct volume *av, struct vldbentry *ve, int acell)
+LockAndInstallVolumeEntry(struct volume *av, struct vldbentry *ve, int acell)
 {
     struct server *ts;
     struct cell *cellp;
     int i, j;
     afs_int32 mask;
     afs_uint32 temp;
+    char types = 0;
+    struct server *serverHost[AFS_MAXHOSTS];
 
     AFS_STATCNT(InstallVolumeEntry);
 
+    memset(serverHost, 0, sizeof(serverHost));
+
     /* Determine the type of volume we want */
     if ((ve->flags & VLF_RWEXISTS) && (av->volume == ve->volumeId[RWVOL])) {
        mask = VLSF_RWVOL;
     } else if ((ve->flags & VLF_ROEXISTS)
               && (av->volume == ve->volumeId[ROVOL])) {
        mask = VLSF_ROVOL;
-       av->states |= VRO;
+       types |= VRO;
     } else if ((ve->flags & VLF_BACKEXISTS)
               && (av->volume == ve->volumeId[BACKVOL])) {
        /* backup always is on the same volume as parent */
        mask = VLSF_RWVOL;
-       av->states |= (VRO | VBackup);
+       types |= (VRO | VBackup);
     } else {
        mask = 0;               /* Can't find volume in vldb entry */
     }
 
-    /* fill in volume types */
-    av->rwVol = ((ve->flags & VLF_RWEXISTS) ? ve->volumeId[RWVOL] : 0);
-    av->roVol = ((ve->flags & VLF_ROEXISTS) ? ve->volumeId[ROVOL] : 0);
-    av->backVol = ((ve->flags & VLF_BACKEXISTS) ? ve->volumeId[BACKVOL] : 0);
-
-    if (ve->flags & VLF_DFSFILESET)
-       av->states |= VForeign;
-
     cellp = afs_GetCell(acell, 0);
 
-    /* This volume, av, is locked. Zero out the serverHosts[] array
-     * so that if afs_GetServer() decides to replace the server
-     * struct, we don't deadlock trying to afs_ResetVolumeInfo()
-     * this volume.
-     */
-    for (j = 0; j < AFS_MAXHOSTS; j++) {
-       av->serverHost[j] = 0;
-    }
-
     /* Step through the VLDB entry making sure each server listed is there */
     for (i = 0, j = 0; i < ve->nServers; i++) {
        if (((ve->serverFlags[i] & mask) == 0)
@@ -935,8 +926,8 @@ InstallVolumeEntry(struct volume *av, struct vldbentry *ve, int acell)
 
        temp = htonl(ve->serverNumber[i]);
        ts = afs_GetServer(&temp, 1, acell, cellp->fsport, WRITE_LOCK,
-                          (afsUUID *) 0, 0);
-       av->serverHost[j] = ts;
+                          (afsUUID *) 0, 0, av);
+       serverHost[j] = ts;
 
        /*
         * The cell field could be 0 if the server entry was created
@@ -951,59 +942,59 @@ InstallVolumeEntry(struct volume *av, struct vldbentry *ve, int acell)
        afs_PutServer(ts, WRITE_LOCK);
        j++;
     }
-    if (j < AFS_MAXHOSTS) {
-       av->serverHost[j++] = 0;
-    }
+
+    ObtainWriteLock(&av->lock, 109);
+
+    memcpy(av->serverHost, serverHost, sizeof(serverHost));
+
+    /* from above */
+    av->states |= types;
+
+    /* fill in volume types */
+    av->rwVol = ((ve->flags & VLF_RWEXISTS) ? ve->volumeId[RWVOL] : 0);
+    av->roVol = ((ve->flags & VLF_ROEXISTS) ? ve->volumeId[ROVOL] : 0);
+    av->backVol = ((ve->flags & VLF_BACKEXISTS) ? ve->volumeId[BACKVOL] : 0);
+
+    if (ve->flags & VLF_DFSFILESET)
+       av->states |= VForeign;
+
     afs_SortServers(av->serverHost, AFS_MAXHOSTS);
 }                              /*InstallVolumeEntry */
 
 
 void
-InstallNVolumeEntry(struct volume *av, struct nvldbentry *ve, int acell)
+LockAndInstallNVolumeEntry(struct volume *av, struct nvldbentry *ve, int acell)
 {
     struct server *ts;
     struct cell *cellp;
     int i, j;
     afs_int32 mask;
     afs_uint32 temp;
+    char types = 0;
+    struct server *serverHost[AFS_MAXHOSTS];
 
     AFS_STATCNT(InstallVolumeEntry);
 
+    memset(serverHost, 0, sizeof(serverHost));
+
     /* Determine type of volume we want */
     if ((ve->flags & VLF_RWEXISTS) && (av->volume == ve->volumeId[RWVOL])) {
        mask = VLSF_RWVOL;
     } else if ((ve->flags & VLF_ROEXISTS)
               && (av->volume == ve->volumeId[ROVOL])) {
        mask = VLSF_ROVOL;
-       av->states |= VRO;
+       types |= VRO;
     } else if ((ve->flags & VLF_BACKEXISTS)
               && (av->volume == ve->volumeId[BACKVOL])) {
        /* backup always is on the same volume as parent */
        mask = VLSF_RWVOL;
-       av->states |= (VRO | VBackup);
+       types |= (VRO | VBackup);
     } else {
        mask = 0;               /* Can't find volume in vldb entry */
     }
 
-    /* fill in volume types */
-    av->rwVol = ((ve->flags & VLF_RWEXISTS) ? ve->volumeId[RWVOL] : 0);
-    av->roVol = ((ve->flags & VLF_ROEXISTS) ? ve->volumeId[ROVOL] : 0);
-    av->backVol = ((ve->flags & VLF_BACKEXISTS) ? ve->volumeId[BACKVOL] : 0);
-
-    if (ve->flags & VLF_DFSFILESET)
-       av->states |= VForeign;
-
     cellp = afs_GetCell(acell, 0);
 
-    /* This volume, av, is locked. Zero out the serverHosts[] array
-     * so that if afs_GetServer() decides to replace the server
-     * struct, we don't deadlock trying to afs_ResetVolumeInfo()
-     * this volume.
-     */
-    for (j = 0; j < AFS_MAXHOSTS; j++) {
-       av->serverHost[j] = 0;
-    }
-
     /* Step through the VLDB entry making sure each server listed is there */
     for (i = 0, j = 0; i < ve->nServers; i++) {
        if (((ve->serverFlags[i] & mask) == 0)
@@ -1013,8 +1004,8 @@ InstallNVolumeEntry(struct volume *av, struct nvldbentry *ve, int acell)
 
        temp = htonl(ve->serverNumber[i]);
        ts = afs_GetServer(&temp, 1, acell, cellp->fsport, WRITE_LOCK,
-                          (afsUUID *) 0, 0);
-       av->serverHost[j] = ts;
+                          (afsUUID *) 0, 0, av);
+       serverHost[j] = ts;
        /*
         * The cell field could be 0 if the server entry was created
         * first with the 'fs setserverprefs' call which doesn't set
@@ -1028,16 +1019,29 @@ InstallNVolumeEntry(struct volume *av, struct nvldbentry *ve, int acell)
        afs_PutServer(ts, WRITE_LOCK);
        j++;
     }
-    if (j < AFS_MAXHOSTS) {
-       av->serverHost[j++] = 0;
-    }
+
+    ObtainWriteLock(&av->lock, 110);
+
+    memcpy(av->serverHost, serverHost, sizeof(serverHost));
+
+    /* from above */
+    av->states |= types;
+
+    /* fill in volume types */
+    av->rwVol = ((ve->flags & VLF_RWEXISTS) ? ve->volumeId[RWVOL] : 0);
+    av->roVol = ((ve->flags & VLF_ROEXISTS) ? ve->volumeId[ROVOL] : 0);
+    av->backVol = ((ve->flags & VLF_BACKEXISTS) ? ve->volumeId[BACKVOL] : 0);
+
+    if (ve->flags & VLF_DFSFILESET)
+       av->states |= VForeign;
+
     afs_SortServers(av->serverHost, AFS_MAXHOSTS);
 }                              /*InstallNVolumeEntry */
 
 
 void
-InstallUVolumeEntry(struct volume *av, struct uvldbentry *ve, int acell,
-                   struct cell *tcell, struct vrequest *areq)
+LockAndInstallUVolumeEntry(struct volume *av, struct uvldbentry *ve, int acell,
+                          struct cell *tcell, struct vrequest *areq)
 {
     struct server *ts;
     struct afs_conn *tconn;
@@ -1046,44 +1050,31 @@ InstallUVolumeEntry(struct volume *av, struct uvldbentry *ve, int acell,
     afs_uint32 serverid;
     afs_int32 mask;
     int k;
+    char type = 0;
+    struct server *serverHost[AFS_MAXHOSTS];
 
     AFS_STATCNT(InstallVolumeEntry);
 
+    memset(serverHost, 0, sizeof(serverHost));
+
     /* Determine type of volume we want */
     if ((ve->flags & VLF_RWEXISTS) && (av->volume == ve->volumeId[RWVOL])) {
        mask = VLSF_RWVOL;
     } else if ((ve->flags & VLF_ROEXISTS)
               && av->volume == ve->volumeId[ROVOL]) {
        mask = VLSF_ROVOL;
-       av->states |= VRO;
+       type |= VRO;
     } else if ((ve->flags & VLF_BACKEXISTS)
               && (av->volume == ve->volumeId[BACKVOL])) {
        /* backup always is on the same volume as parent */
        mask = VLSF_RWVOL;
-       av->states |= (VRO | VBackup);
+       type |= (VRO | VBackup);
     } else {
        mask = 0;               /* Can't find volume in vldb entry */
     }
 
-    /* fill in volume types */
-    av->rwVol = ((ve->flags & VLF_RWEXISTS) ? ve->volumeId[RWVOL] : 0);
-    av->roVol = ((ve->flags & VLF_ROEXISTS) ? ve->volumeId[ROVOL] : 0);
-    av->backVol = ((ve->flags & VLF_BACKEXISTS) ? ve->volumeId[BACKVOL] : 0);
-
-    if (ve->flags & VLF_DFSFILESET)
-       av->states |= VForeign;
-
     cellp = afs_GetCell(acell, 0);
 
-    /* This volume, av, is locked. Zero out the serverHosts[] array
-     * so that if afs_GetServer() decides to replace the server
-     * struct, we don't deadlock trying to afs_ResetVolumeInfo()
-     * this volume.
-     */
-    for (j = 0; j < AFS_MAXHOSTS; j++) {
-       av->serverHost[j] = 0;
-    }
-
     /* Gather the list of servers the VLDB says the volume is on
      * and initialize the ve->serverHost[] array. If a server struct
      * is not found, then get the list of addresses for the
@@ -1098,8 +1089,8 @@ InstallUVolumeEntry(struct volume *av, struct uvldbentry *ve, int acell,
        if (!(ve->serverFlags[i] & VLSERVER_FLAG_UUID)) {
            /* The server has no uuid */
            serverid = htonl(ve->serverNumber[i].time_low);
-           ts = afs_GetServer(&serverid, 1, acell, cellp->fsport, WRITE_LOCK,
-                              (afsUUID *) 0, 0);
+           ts = afs_GetServer(&serverid, 1, acell, cellp->fsport,
+                              WRITE_LOCK, (afsUUID *) 0, 0, av);
        } else {
            ts = afs_FindServer(0, cellp->fsport, &ve->serverNumber[i], 0);
            if (ts && (ts->sr_addr_uniquifier == ve->serverUnique[i])
@@ -1149,13 +1140,14 @@ InstallUVolumeEntry(struct volume *av, struct uvldbentry *ve, int acell,
                for (k = 0; k < nentries; k++) {
                    addrp[k] = htonl(addrp[k]);
                }
-               ts = afs_GetServer(addrp, nentries, acell, cellp->fsport,
-                                  WRITE_LOCK, &ve->serverNumber[i],
-                                  ve->serverUnique[i]);
+               ts = afs_GetServer(addrp, nentries, acell,
+                                  cellp->fsport, WRITE_LOCK,
+                                  &ve->serverNumber[i],
+                                  ve->serverUnique[i], av);
                xdr_free((xdrproc_t) xdr_bulkaddrs, &addrs);
            }
        }
-       av->serverHost[j] = ts;
+       serverHost[j] = ts;
 
        /* The cell field could be 0 if the server entry was created
         * first with the 'fs setserverprefs' call which doesn't set
@@ -1170,6 +1162,21 @@ InstallUVolumeEntry(struct volume *av, struct uvldbentry *ve, int acell,
        j++;
     }
 
+    ObtainWriteLock(&av->lock, 111);
+
+    memcpy(av->serverHost, serverHost, sizeof(serverHost));
+
+    /* from above */
+    av->states |= type;
+
+    /* fill in volume types */
+    av->rwVol = ((ve->flags & VLF_RWEXISTS) ? ve->volumeId[RWVOL] : 0);
+    av->roVol = ((ve->flags & VLF_ROEXISTS) ? ve->volumeId[ROVOL] : 0);
+    av->backVol = ((ve->flags & VLF_BACKEXISTS) ? ve->volumeId[BACKVOL] : 0);
+
+    if (ve->flags & VLF_DFSFILESET)
+       av->states |= VForeign;
+
     afs_SortServers(av->serverHost, AFS_MAXHOSTS);
 }                              /*InstallVolumeEntry */