jimzah/openafs
1
0
Fork 0
mirror of https://git.openafs.org/openafs.git synced 2025-01-21 00:10:15 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Windows: Correct cm_volume locking

Browse Source 
cm_volume_t flags was used for two categories of flags.  The first
protected by the cm_volume_t->rw lock.  The second protected by
the global cm_volumeLock.  Separate the flags field into two
afs_uint16 fields and break the flag space into FLAG and QFLAG.

Add assertions to the volume LRU functions to ensure that they
are always called with cm_volumeLock write-locked.

Correct two locations where cm_AdjustVolumeLRU() was called
read-locked instead of write-locked.

Change-Id: I7991b995a3c981cd5d08d9cbba09badc81518a5a
Reviewed-on: http://gerrit.openafs.org/3760
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
This commit is contained in:
Jeffrey Altman 2011-01-26 20:10:57 -05:00 committed by Jeffrey Altman
parent 5817a13b84
commit 962a95c21c
3 changed files with 53 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/WINNT/afsd/cm_memmap.h
View File

@ -10,7 +10,7 @@
#ifndef CM_MEMMAP_H #ifndef CM_MEMMAP_H
#define CM_MEMMAP_H 1 #define CM_MEMMAP_H 1
#define CM_CONFIG_DATA_VERSION 8 #define CM_CONFIG_DATA_VERSION 14
#define CM_CONFIG_DATA_MAGIC ('A' | 'F'<<8 | 'S'<<16 | CM_CONFIG_DATA_VERSION<<24) #define CM_CONFIG_DATA_MAGIC ('A' | 'F'<<8 | 'S'<<16 | CM_CONFIG_DATA_VERSION<<24)
typedef struct cm_config_data { typedef struct cm_config_data {

78
src/WINNT/afsd/cm_volume.c
View File

@ -461,7 +461,7 @@ long cm_UpdateVolumeLocation(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *
osi_Log2(afsd_logp, "cm_UpdateVolume name %s -> %s", osi_Log2(afsd_logp, "cm_UpdateVolume name %s -> %s",
osi_LogSaveString(afsd_logp,volp->namep), osi_LogSaveString(afsd_logp,name)); osi_LogSaveString(afsd_logp,volp->namep), osi_LogSaveString(afsd_logp,name));
if (volp->flags & CM_VOLUMEFLAG_IN_HASH) if (volp->qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromNameHashTable(volp); cm_RemoveVolumeFromNameHashTable(volp);
strcpy(volp->namep, name); strcpy(volp->namep, name);
@ -477,37 +477,37 @@ long cm_UpdateVolumeLocation(struct cm_cell *cellp, cm_user_t *userp, cm_req_t *
if (flags & VLF_RWEXISTS) { if (flags & VLF_RWEXISTS) {
if (volp->vol[RWVOL].ID != rwID) { if (volp->vol[RWVOL].ID != rwID) {
if (volp->vol[RWVOL].flags & CM_VOLUMEFLAG_IN_HASH) if (volp->vol[RWVOL].qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromIDHashTable(volp, RWVOL); cm_RemoveVolumeFromIDHashTable(volp, RWVOL);
volp->vol[RWVOL].ID = rwID; volp->vol[RWVOL].ID = rwID;
cm_AddVolumeToIDHashTable(volp, RWVOL); cm_AddVolumeToIDHashTable(volp, RWVOL);
} }
} else { } else {
if (volp->vol[RWVOL].flags & CM_VOLUMEFLAG_IN_HASH) if (volp->vol[RWVOL].qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromIDHashTable(volp, RWVOL); cm_RemoveVolumeFromIDHashTable(volp, RWVOL);
volp->vol[RWVOL].ID = 0; volp->vol[RWVOL].ID = 0;
} }
if (flags & VLF_ROEXISTS) { if (flags & VLF_ROEXISTS) {
if (volp->vol[ROVOL].ID != roID) { if (volp->vol[ROVOL].ID != roID) {
if (volp->vol[ROVOL].flags & CM_VOLUMEFLAG_IN_HASH) if (volp->vol[ROVOL].qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromIDHashTable(volp, ROVOL); cm_RemoveVolumeFromIDHashTable(volp, ROVOL);
volp->vol[ROVOL].ID = roID; volp->vol[ROVOL].ID = roID;
cm_AddVolumeToIDHashTable(volp, ROVOL); cm_AddVolumeToIDHashTable(volp, ROVOL);
} }
} else { } else {
if (volp->vol[ROVOL].flags & CM_VOLUMEFLAG_IN_HASH) if (volp->vol[ROVOL].qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromIDHashTable(volp, ROVOL); cm_RemoveVolumeFromIDHashTable(volp, ROVOL);
volp->vol[ROVOL].ID = 0; volp->vol[ROVOL].ID = 0;
} }
if (flags & VLF_BACKEXISTS) { if (flags & VLF_BACKEXISTS) {
if (volp->vol[BACKVOL].ID != bkID) { if (volp->vol[BACKVOL].ID != bkID) {
if (volp->vol[BACKVOL].flags & CM_VOLUMEFLAG_IN_HASH) if (volp->vol[BACKVOL].qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromIDHashTable(volp, BACKVOL); cm_RemoveVolumeFromIDHashTable(volp, BACKVOL);
volp->vol[BACKVOL].ID = bkID; volp->vol[BACKVOL].ID = bkID;
cm_AddVolumeToIDHashTable(volp, BACKVOL); cm_AddVolumeToIDHashTable(volp, BACKVOL);
} }
} else { } else {
if (volp->vol[BACKVOL].flags & CM_VOLUMEFLAG_IN_HASH) if (volp->vol[BACKVOL].qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromIDHashTable(volp, BACKVOL); cm_RemoveVolumeFromIDHashTable(volp, BACKVOL);
volp->vol[BACKVOL].ID = 0; volp->vol[BACKVOL].ID = 0;
} }
@ -908,13 +908,13 @@ long cm_FindVolumeByName(struct cm_cell *cellp, char *volumeNamep,
* same object. This volp must be re-inserted back into * same object. This volp must be re-inserted back into
* the LRU queue before this function exits. * the LRU queue before this function exits.
*/ */
if (volp->flags & CM_VOLUMEFLAG_IN_LRU_QUEUE) if (volp->qflags & CM_VOLUME_QFLAG_IN_LRU_QUEUE)
cm_RemoveVolumeFromLRU(volp); cm_RemoveVolumeFromLRU(volp);
if (volp->flags & CM_VOLUMEFLAG_IN_HASH) if (volp->qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromNameHashTable(volp); cm_RemoveVolumeFromNameHashTable(volp);
for ( volType = RWVOL; volType < NUM_VOL_TYPES; volType++) { for ( volType = RWVOL; volType < NUM_VOL_TYPES; volType++) {
if (volp->vol[volType].flags & CM_VOLUMEFLAG_IN_HASH) if (volp->vol[volType].qflags & CM_VOLUME_QFLAG_IN_HASH)
cm_RemoveVolumeFromIDHashTable(volp, volType); cm_RemoveVolumeFromIDHashTable(volp, volType);
if (volp->vol[volType].ID) if (volp->vol[volType].ID)
cm_VolumeStatusNotification(volp, volp->vol[volType].ID, volp->vol[volType].state, vl_unknown); cm_VolumeStatusNotification(volp, volp->vol[volType].ID, volp->vol[volType].state, vl_unknown);
@ -976,22 +976,22 @@ long cm_FindVolumeByName(struct cm_cell *cellp, char *volumeNamep,
if (code == 0) { if (code == 0) {
*outVolpp = volp; *outVolpp = volp;
lock_ObtainRead(&cm_volumeLock); lock_ObtainWrite(&cm_volumeLock);
if (!(volp->flags & CM_VOLUMEFLAG_IN_LRU_QUEUE) || if (!(volp->qflags & CM_VOLUME_QFLAG_IN_LRU_QUEUE) ||
(flags & CM_GETVOL_FLAG_NO_LRU_UPDATE)) (flags & CM_GETVOL_FLAG_NO_LRU_UPDATE))
cm_AdjustVolumeLRU(volp); cm_AdjustVolumeLRU(volp);
lock_ReleaseRead(&cm_volumeLock); lock_ReleaseWrite(&cm_volumeLock);
} else { } else {
/* /*
* do not return it to the caller but do insert it in the LRU * do not return it to the caller but do insert it in the LRU
* otherwise it will be lost * otherwise it will be lost
*/ */
lock_ObtainRead(&cm_volumeLock); lock_ObtainWrite(&cm_volumeLock);
if (!(volp->flags & CM_VOLUMEFLAG_IN_LRU_QUEUE) || if (!(volp->qflags & CM_VOLUME_QFLAG_IN_LRU_QUEUE) ||
(flags & CM_GETVOL_FLAG_NO_LRU_UPDATE)) (flags & CM_GETVOL_FLAG_NO_LRU_UPDATE))
cm_AdjustVolumeLRU(volp); cm_AdjustVolumeLRU(volp);
cm_PutVolume(volp); cm_PutVolume(volp);
lock_ReleaseRead(&cm_volumeLock); lock_ReleaseWrite(&cm_volumeLock);
} }
if (code == CM_ERROR_NOSUCHVOLUME && cellp->linkedName[0] && if (code == CM_ERROR_NOSUCHVOLUME && cellp->linkedName[0] &&
@ -1327,7 +1327,7 @@ void cm_CheckOfflineVolumes(void)
for (volp = cm_data.volumeLRULastp; for (volp = cm_data.volumeLRULastp;
volp && !daemon_ShutdownFlag && !powerStateSuspended; volp && !daemon_ShutdownFlag && !powerStateSuspended;
volp=(cm_volume_t *) osi_QPrev(&volp->q)) { volp=(cm_volume_t *) osi_QPrev(&volp->q)) {
if (volp->flags & CM_VOLUMEFLAG_IN_HASH) { if (volp->qflags & CM_VOLUME_QFLAG_IN_HASH) {
InterlockedIncrement(&volp->refCount); InterlockedIncrement(&volp->refCount);
lock_ReleaseRead(&cm_volumeLock); lock_ReleaseRead(&cm_volumeLock);
cm_CheckOfflineVolume(volp, 0); cm_CheckOfflineVolume(volp, 0);
@ -1529,10 +1529,10 @@ int cm_DumpVolumes(FILE *outputFile, char *cookie, int lock)
} }
sprintf(output, sprintf(output,
"%s - volp=0x%p cell=%s name=%s rwID=%u roID=%u bkID=%u flags=0x%x " "%s - volp=0x%p cell=%s name=%s rwID=%u roID=%u bkID=%u flags=0x%x:%x "
"cbServerpRO='%s' cbExpiresRO='%s' creationDateRO='%s' refCount=%u\r\n", "cbServerpRO='%s' cbExpiresRO='%s' creationDateRO='%s' refCount=%u\r\n",
cookie, volp, volp->cellp->name, volp->namep, volp->vol[RWVOL].ID, cookie, volp, volp->cellp->name, volp->namep, volp->vol[RWVOL].ID,
volp->vol[ROVOL].ID, volp->vol[BACKVOL].ID, volp->flags, volp->vol[ROVOL].ID, volp->vol[BACKVOL].ID, volp->flags, volp->qflags,
srvStr ? srvStr : "<none>", cbt ? cbt : "<none>", cdrot ? cdrot : "<none>", srvStr ? srvStr : "<none>", cbt ? cbt : "<none>", cdrot ? cdrot : "<none>",
volp->refCount); volp->refCount);
WriteFile(outputFile, output, (DWORD)strlen(output), &zilch, NULL); WriteFile(outputFile, output, (DWORD)strlen(output), &zilch, NULL);
@ -1585,14 +1585,14 @@ void cm_AddVolumeToNameHashTable(cm_volume_t *volp)
{ {
int i; int i;
if (volp->flags & CM_VOLUMEFLAG_IN_HASH) if (volp->qflags & CM_VOLUME_QFLAG_IN_HASH)
return; return;
i = CM_VOLUME_NAME_HASH(volp->namep); i = CM_VOLUME_NAME_HASH(volp->namep);
volp->nameNextp = cm_data.volumeNameHashTablep[i]; volp->nameNextp = cm_data.volumeNameHashTablep[i];
cm_data.volumeNameHashTablep[i] = volp; cm_data.volumeNameHashTablep[i] = volp;
volp->flags |= CM_VOLUMEFLAG_IN_HASH; volp->qflags |= CM_VOLUME_QFLAG_IN_HASH;
} }
/* call with volume write-locked and mutex held */ /* call with volume write-locked and mutex held */
@ -1602,7 +1602,7 @@ void cm_RemoveVolumeFromNameHashTable(cm_volume_t *volp)
cm_volume_t *tvolp; cm_volume_t *tvolp;
int i; int i;
if (volp->flags & CM_VOLUMEFLAG_IN_HASH) { if (volp->qflags & CM_VOLUME_QFLAG_IN_HASH) {
/* hash it out first */ /* hash it out first */
i = CM_VOLUME_NAME_HASH(volp->namep); i = CM_VOLUME_NAME_HASH(volp->namep);
for (lvolpp = &cm_data.volumeNameHashTablep[i], tvolp = cm_data.volumeNameHashTablep[i]; for (lvolpp = &cm_data.volumeNameHashTablep[i], tvolp = cm_data.volumeNameHashTablep[i];
@ -1610,7 +1610,7 @@ void cm_RemoveVolumeFromNameHashTable(cm_volume_t *volp)
lvolpp = &tvolp->nameNextp, tvolp = tvolp->nameNextp) { lvolpp = &tvolp->nameNextp, tvolp = tvolp->nameNextp) {
if (tvolp == volp) { if (tvolp == volp) {
*lvolpp = volp->nameNextp; *lvolpp = volp->nameNextp;
volp->flags &= ~CM_VOLUMEFLAG_IN_HASH; volp->qflags &= ~CM_VOLUME_QFLAG_IN_HASH;
volp->nameNextp = NULL; volp->nameNextp = NULL;
break; break;
} }
@ -1626,7 +1626,7 @@ void cm_AddVolumeToIDHashTable(cm_volume_t *volp, afs_uint32 volType)
statep = cm_VolumeStateByType(volp, volType); statep = cm_VolumeStateByType(volp, volType);
if (statep->flags & CM_VOLUMEFLAG_IN_HASH) if (statep->qflags & CM_VOLUME_QFLAG_IN_HASH)
return; return;
i = CM_VOLUME_ID_HASH(statep->ID); i = CM_VOLUME_ID_HASH(statep->ID);
@ -1645,7 +1645,7 @@ void cm_AddVolumeToIDHashTable(cm_volume_t *volp, afs_uint32 volType)
cm_data.volumeBKIDHashTablep[i] = volp; cm_data.volumeBKIDHashTablep[i] = volp;
break; break;
} }
statep->flags |= CM_VOLUMEFLAG_IN_HASH; statep->qflags |= CM_VOLUME_QFLAG_IN_HASH;
} }
@ -1659,7 +1659,7 @@ void cm_RemoveVolumeFromIDHashTable(cm_volume_t *volp, afs_uint32 volType)
statep = cm_VolumeStateByType(volp, volType); statep = cm_VolumeStateByType(volp, volType);
if (statep->flags & CM_VOLUMEFLAG_IN_HASH) { if (statep->qflags & CM_VOLUME_QFLAG_IN_HASH) {
/* hash it out first */ /* hash it out first */
i = CM_VOLUME_ID_HASH(statep->ID); i = CM_VOLUME_ID_HASH(statep->ID);
@ -1682,7 +1682,7 @@ void cm_RemoveVolumeFromIDHashTable(cm_volume_t *volp, afs_uint32 volType)
do { do {
if (tvolp == volp) { if (tvolp == volp) {
*lvolpp = statep->nextp; *lvolpp = statep->nextp;
statep->flags &= ~CM_VOLUMEFLAG_IN_HASH; statep->qflags &= ~CM_VOLUME_QFLAG_IN_HASH;
statep->nextp = NULL; statep->nextp = NULL;
break; break;
} }
@ -1696,34 +1696,46 @@ void cm_RemoveVolumeFromIDHashTable(cm_volume_t *volp, afs_uint32 volType)
/* must be called with cm_volumeLock write-locked! */ /* must be called with cm_volumeLock write-locked! */
void cm_AdjustVolumeLRU(cm_volume_t *volp) void cm_AdjustVolumeLRU(cm_volume_t *volp)
{ {
lock_AssertWrite(&cm_volumeLock);
if (volp == cm_data.volumeLRUFirstp) if (volp == cm_data.volumeLRUFirstp)
return; return;
if (volp->flags & CM_VOLUMEFLAG_IN_LRU_QUEUE) if (volp->qflags & CM_VOLUME_QFLAG_IN_LRU_QUEUE)
osi_QRemoveHT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q); osi_QRemoveHT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q);
osi_QAddH((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q); osi_QAddH((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q);
volp->flags |= CM_VOLUMEFLAG_IN_LRU_QUEUE; volp->qflags |= CM_VOLUME_QFLAG_IN_LRU_QUEUE;
osi_assertx(cm_data.volumeLRULastp != NULL, "null cm_data.volumeLRULastp");
} }
/* must be called with cm_volumeLock write-locked! */ /* must be called with cm_volumeLock write-locked! */
void cm_MoveVolumeToLRULast(cm_volume_t *volp) void cm_MoveVolumeToLRULast(cm_volume_t *volp)
{ {
lock_AssertWrite(&cm_volumeLock);
if (volp == cm_data.volumeLRULastp) if (volp == cm_data.volumeLRULastp)
return; return;
if (volp->flags & CM_VOLUMEFLAG_IN_LRU_QUEUE) if (volp->qflags & CM_VOLUME_QFLAG_IN_LRU_QUEUE)
osi_QRemoveHT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q); osi_QRemoveHT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q);
osi_QAddT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q); osi_QAddT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q);
volp->flags |= CM_VOLUMEFLAG_IN_LRU_QUEUE; volp->qflags |= CM_VOLUME_QFLAG_IN_LRU_QUEUE;
osi_assertx(cm_data.volumeLRULastp != NULL, "null cm_data.volumeLRULastp");
} }
/* must be called with cm_volumeLock write-locked! */ /* must be called with cm_volumeLock write-locked! */
void cm_RemoveVolumeFromLRU(cm_volume_t *volp) void cm_RemoveVolumeFromLRU(cm_volume_t *volp)
{ {
if (volp->flags & CM_VOLUMEFLAG_IN_LRU_QUEUE) { lock_AssertWrite(&cm_volumeLock);
if (volp->qflags & CM_VOLUME_QFLAG_IN_LRU_QUEUE) {
osi_QRemoveHT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q); osi_QRemoveHT((osi_queue_t **) &cm_data.volumeLRUFirstp, (osi_queue_t **) &cm_data.volumeLRULastp, &volp->q);
volp->flags &= ~CM_VOLUMEFLAG_IN_LRU_QUEUE; volp->qflags &= ~CM_VOLUME_QFLAG_IN_LRU_QUEUE;
} }
osi_assertx(cm_data.volumeLRULastp != NULL, "null cm_data.volumeLRULastp");
} }
static char * volstatus_str(enum volstatus vs) static char * volstatus_str(enum volstatus vs)

11
src/WINNT/afsd/cm_volume.h
View File

@ -24,7 +24,8 @@ typedef struct cm_vol_state {
struct cm_fid dotdotFid; /* parent of volume root */ struct cm_fid dotdotFid; /* parent of volume root */
cm_serverRef_t *serversp; /* by cm_serverLock */ cm_serverRef_t *serversp; /* by cm_serverLock */
enum volstatus state; /* by rw */ enum volstatus state; /* by rw */
afs_uint32 flags; /* by rw */ afs_uint16 flags; /* by rw */
afs_uint16 qflags; /* by cm_volumeLock */
} cm_vol_state_t; } cm_vol_state_t;
/* RWVOL, ROVOL, BACKVOL are defined in cm.h */ /* RWVOL, ROVOL, BACKVOL are defined in cm.h */
@ -40,7 +41,8 @@ typedef struct cm_volume {
/* by cm_volumeLock */ /* by cm_volumeLock */
struct cm_vol_state vol[NUM_VOL_TYPES]; /* by cm_volumeLock */ struct cm_vol_state vol[NUM_VOL_TYPES]; /* by cm_volumeLock */
osi_rwlock_t rw; osi_rwlock_t rw;
afs_uint32 flags; /* by rw */ afs_uint16 flags; /* by rw */
afs_uint16 qflags; /* by cm_volumeLock */
afs_int32 refCount; /* by Interlocked operations */ afs_int32 refCount; /* by Interlocked operations */
struct cm_server *cbServerpRO; /* server granting RO callback; by cm_scacheLock */ struct cm_server *cbServerpRO; /* server granting RO callback; by cm_scacheLock */
time_t cbExpiresRO; /* latest RO expiration time; by cm_scacheLock */ time_t cbExpiresRO; /* latest RO expiration time; by cm_scacheLock */
@ -49,12 +51,13 @@ typedef struct cm_volume {
} cm_volume_t; } cm_volume_t;
#define CM_VOLUMEFLAG_RESET 1 /* reload this info on next use */ #define CM_VOLUMEFLAG_RESET 1 /* reload this info on next use */
#define CM_VOLUMEFLAG_IN_HASH 2
#define CM_VOLUMEFLAG_IN_LRU_QUEUE 4
#define CM_VOLUMEFLAG_UPDATING_VL 8 #define CM_VOLUMEFLAG_UPDATING_VL 8
#define CM_VOLUMEFLAG_DFS_VOLUME 16 #define CM_VOLUMEFLAG_DFS_VOLUME 16
#define CM_VOLUMEFLAG_NOEXIST 32 #define CM_VOLUMEFLAG_NOEXIST 32
#define CM_VOLUME_QFLAG_IN_HASH 1
#define CM_VOLUME_QFLAG_IN_LRU_QUEUE 2
typedef struct cm_volumeRef { typedef struct cm_volumeRef {
struct cm_volumeRef * next; struct cm_volumeRef * next;
afs_uint32 volID; afs_uint32 volID;