Windows: Fix cm_serverRef ref counts

Use Interlocked operations consistently

Simplify cm_ServerInsertList().  It no longer increments the
refCount on the serverRef object.  Instead it leaves the refCount
as is.  Its the caller's responsibility to add a reference if
required.

Add reference counts and hold locks in places where the
volume server list was used unprotected.

Change-Id: Ie65cdca4461e84c675e8a29e22cef3e15679fda7
Reviewed-on: http://gerrit.openafs.org/5248
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementix.org>
This commit is contained in:
Jeffrey Altman 2011-08-12 19:02:48 -04:00 committed by Derrick Brashear
parent 578db3bbec
commit 8f446c7463
4 changed files with 48 additions and 25 deletions

View File

@ -65,10 +65,6 @@ long cm_AddCellProc(void *rockp, struct sockaddr_in *addrp, char *hostnamep, uns
/* Insert the vlserver into a sorted list, sorted by server rank */
tsrp = cm_NewServerRef(tsp, 0);
cm_InsertServerList(&cellp->vlServersp, tsrp);
/* drop the allocation reference */
lock_ObtainWrite(&cm_serverLock);
tsrp->refCount--;
lock_ReleaseWrite(&cm_serverLock);
return 0;
}

View File

@ -950,6 +950,10 @@ cm_server_vols_t *cm_NewServerVols(void) {
return tsvp;
}
/*
* cm_NewServerRef() returns with the allocated cm_serverRef_t
* with a refCount of 1.
*/
cm_serverRef_t *cm_NewServerRef(cm_server_t *serverp, afs_uint32 volID)
{
cm_serverRef_t *tsrp;
@ -1010,6 +1014,34 @@ cm_serverRef_t *cm_NewServerRef(cm_server_t *serverp, afs_uint32 volID)
return tsrp;
}
void cm_GetServerRef(cm_serverRef_t *tsrp, int locked)
{
afs_int32 refCount;
if (!locked)
lock_ObtainRead(&cm_serverLock);
refCount = InterlockedIncrement(&tsrp->refCount);
if (!locked)
lock_ReleaseRead(&cm_serverLock);
}
afs_int32 cm_PutServerRef(cm_serverRef_t *tsrp, int locked)
{
afs_int32 refCount;
if (!locked)
lock_ObtainRead(&cm_serverLock);
refCount = InterlockedDecrement(&tsrp->refCount);
osi_assertx(refCount >= 0, "cm_serverRef_t refCount underflow");
if (!locked)
lock_ReleaseRead(&cm_serverLock);
return refCount;
}
LONG_PTR cm_ChecksumServerList(cm_serverRef_t *serversp)
{
LONG_PTR sum = 0;
@ -1035,7 +1067,7 @@ LONG_PTR cm_ChecksumServerList(cm_serverRef_t *serversp)
** Insert a server into the server list keeping the list sorted in
** ascending order of ipRank.
**
** The refCount of the cm_serverRef_t is increased
** The refCount of the cm_serverRef_t is not altered.
*/
void cm_InsertServerList(cm_serverRef_t** list, cm_serverRef_t* element)
{
@ -1046,8 +1078,6 @@ void cm_InsertServerList(cm_serverRef_t** list, cm_serverRef_t* element)
current=*list;
ipRank = element->server->ipRank;
element->refCount++; /* increase refCount */
/* insertion into empty list or at the beginning of the list */
if ( !current || (current->server->ipRank > ipRank) )
{
@ -1065,6 +1095,7 @@ void cm_InsertServerList(cm_serverRef_t** list, cm_serverRef_t* element)
}
element->next = current->next;
current->next = element;
lock_ReleaseWrite(&cm_serverLock);
}
/*
@ -1093,7 +1124,7 @@ long cm_ChangeRankServer(cm_serverRef_t** list, cm_server_t* server)
if ( (*current)->server == server)
{
element = (*current);
*current = (*current)->next; /* delete it */
*current = element->next; /* delete it */
break;
}
current = & ( (*current)->next);
@ -1107,10 +1138,6 @@ long cm_ChangeRankServer(cm_serverRef_t** list, cm_server_t* server)
/* re-insert deleted element into the list with modified rank*/
cm_InsertServerList(list, element);
/* reduce refCount which was increased by cm_InsertServerList */
lock_ObtainWrite(&cm_serverLock);
element->refCount--;
lock_ReleaseWrite(&cm_serverLock);
return 0;
}
/*
@ -1255,6 +1282,7 @@ void cm_FreeServerList(cm_serverRef_t** list, afs_uint32 flags)
cm_serverRef_t **current;
cm_serverRef_t **nextp;
cm_serverRef_t * next;
afs_int32 refCount;
lock_ObtainWrite(&cm_serverLock);
current = list;
@ -1267,7 +1295,8 @@ void cm_FreeServerList(cm_serverRef_t** list, afs_uint32 flags)
while (*current)
{
nextp = &(*current)->next;
if (--((*current)->refCount) == 0) {
refCount = cm_PutServerRef(*current, TRUE);
if (refCount == 0) {
next = *nextp;
if ((*current)->volID)

View File

@ -85,6 +85,10 @@ extern cm_server_t *cm_NewServer(struct sockaddr_in *addrp, int type,
extern cm_serverRef_t *cm_NewServerRef(struct cm_server *serverp, afs_uint32 volID);
extern void cm_GetServerRef(cm_serverRef_t *tsrp, int locked);
extern afs_int32 cm_PutServerRef(cm_serverRef_t *tsrp, int locked);
extern LONG_PTR cm_ChecksumServerList(cm_serverRef_t *serversp);
extern void cm_GetServer(cm_server_t *);

View File

@ -524,6 +524,7 @@ long cm_UpdateVolumeLocation(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *
tempAddr = htonl(serverNumber[i]);
tsockAddr.sin_addr.s_addr = tempAddr;
tsp = cm_FindServer(&tsockAddr, CM_SERVER_FILE);
#ifdef MULTIHOMED
if (tsp && (method == 2) && (tsp->flags & CM_SERVERFLAG_UUID)) {
/*
* Check to see if the uuid of the server we know at this address
@ -544,6 +545,7 @@ long cm_UpdateVolumeLocation(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *
osi_LogSaveString(afsd_logp, hoststr));
}
}
#endif
if (!tsp) {
/*
* cm_NewServer will probe the file server which in turn will
@ -580,20 +582,12 @@ long cm_UpdateVolumeLocation(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *
if ((tflags & VLSF_RWVOL) && (flags & VLF_RWEXISTS)) {
tsrp = cm_NewServerRef(tsp, rwID);
cm_InsertServerList(&volp->vol[RWVOL].serversp, tsrp);
lock_ObtainWrite(&cm_serverLock);
tsrp->refCount--; /* drop allocation reference */
lock_ReleaseWrite(&cm_serverLock);
if (!(tsp->flags & CM_SERVERFLAG_DOWN))
rwServers_alldown = 0;
}
if ((tflags & VLSF_ROVOL) && (flags & VLF_ROEXISTS)) {
tsrp = cm_NewServerRef(tsp, roID);
cm_InsertServerList(&volp->vol[ROVOL].serversp, tsrp);
lock_ObtainWrite(&cm_serverLock);
tsrp->refCount--; /* drop allocation reference */
lock_ReleaseWrite(&cm_serverLock);
ROcount++;
if (!(tsp->flags & CM_SERVERFLAG_DOWN))
@ -607,9 +601,6 @@ long cm_UpdateVolumeLocation(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *
if ((tflags & VLSF_RWVOL) && (flags & VLF_BACKEXISTS)) {
tsrp = cm_NewServerRef(tsp, bkID);
cm_InsertServerList(&volp->vol[BACKVOL].serversp, tsrp);
lock_ObtainWrite(&cm_serverLock);
tsrp->refCount--; /* drop allocation reference */
lock_ReleaseWrite(&cm_serverLock);
if (!(tsp->flags & CM_SERVERFLAG_DOWN))
bkServers_alldown = 0;
@ -1131,7 +1122,7 @@ cm_serverRef_t **cm_GetVolServers(cm_volume_t *volp, afs_uint32 volume, cm_user_
* They will be freed by cm_FreeServerList when they get to zero
*/
for (current = *serverspp; current; current = current->next)
current->refCount++;
cm_GetServerRef(current, TRUE);
lock_ReleaseWrite(&cm_serverLock);
@ -1229,6 +1220,7 @@ cm_CheckOfflineVolumeState(cm_volume_t *volp, cm_vol_state_t *statep, afs_uint32
*volumeUpdatedp = 1;
}
lock_ObtainRead(&cm_serverLock);
if (statep->serversp) {
alldown = 1;
alldeleted = 1;
@ -1243,6 +1235,7 @@ cm_CheckOfflineVolumeState(cm_volume_t *volp, cm_vol_state_t *statep, afs_uint32
if (serversp->status == srv_busy || serversp->status == srv_offline)
serversp->status = srv_not_busy;
}
lock_ReleaseRead(&cm_serverLock);
if (alldeleted && !(*volumeUpdatedp)) {
cm_InitReq(&req);
@ -1283,6 +1276,7 @@ cm_CheckOfflineVolumeState(cm_volume_t *volp, cm_vol_state_t *statep, afs_uint32
statep->state = vl_alldown;
}
} else if (statep->state != vl_alldown) {
lock_ReleaseRead(&cm_serverLock);
cm_VolumeStatusNotification(volp, statep->ID, statep->state, vl_alldown);
statep->state = vl_alldown;
}