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>
Wed, 31 Aug 2011 18:37:54 +0000 (11:37 -0700)
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.

Change-Id: I648900849a5a7349adc686658872706bd7024c90
Reviewed-on: http://gerrit.openafs.org/5303
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 ff2ee6f..259ca5b 100644 (file)
@@ -70,6 +70,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 b6dbed8..a59b416 100644 (file)
@@ -172,13 +172,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 29cb689..99e2128 100644 (file)
@@ -1001,7 +1001,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 38a9999..2d46ac2 100644 (file)
@@ -3972,7 +3972,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 f32c87c..dee3695 100644 (file)
@@ -861,7 +861,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);
@@ -882,7 +883,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,
@@ -1380,16 +1380,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 bcff775..1406ff1 100644 (file)
@@ -1583,16 +1583,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) {
@@ -1636,9 +1636,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;
@@ -1657,7 +1658,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);
     }
 }
 
@@ -1734,16 +1735,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;
@@ -1757,7 +1778,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);
@@ -1842,7 +1863,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;
        }
@@ -1923,7 +1944,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;
@@ -1946,6 +1967,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 f2148a5..a32c6da 100644 (file)
@@ -255,13 +255,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;
@@ -271,8 +275,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;
                }
            }
@@ -280,7 +286,6 @@ afs_ResetVolumes(struct server *srvp)
     }
 }
 
-
 /**
  *   Reset volume name to volume id mapping cache.
  * @param flags
@@ -620,13 +625,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);
@@ -882,51 +886,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)
@@ -936,8 +927,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
@@ -952,59 +943,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)
@@ -1014,8 +1005,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
@@ -1029,16 +1020,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;
@@ -1047,44 +1051,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
@@ -1099,8 +1090,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])
@@ -1150,13 +1141,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
@@ -1171,6 +1163,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 */