From af5923c0507f45fc4124ed9ae5ac5ed014923034 Mon Sep 17 00:00:00 2001 From: Andrew Deason Date: Thu, 1 Apr 2010 16:42:25 -0500 Subject: [PATCH] tubik: Correct use of flags_cond and version_cond Waiters of flags_cond and version_cond were not doing so correctly; the correct way is to acquire a lock prior to their respective checks, and atomically drop/acquire that lock with pthread_cond_wait. Otherwise, we could miss a wakeup if a flag changed between our check and when we wait. To make this possible, make versionLock a normal pthread mutex in AFS_PTHREAD_ENV, so it is a lock we can pass to pthread_cond_wait. Make the waiters pass versionLock to pthread_cond_wait, and eliminate flags_mutex and version_mutex. Change-Id: Id33a72182b907d069e342cb049e23868ab2f7eb9 Reviewed-on: http://gerrit.openafs.org/1681 Tested-by: Andrew Deason Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear --- src/ubik/ubik.c | 26 +++++++++++++++----------- src/ubik/ubik.p.h | 15 ++++++++++----- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/ubik/ubik.c b/src/ubik/ubik.c index 4187ac0..9bb9a97 100644 --- a/src/ubik/ubik.c +++ b/src/ubik/ubik.c @@ -415,7 +415,11 @@ ubik_ServerInitCommon(afs_int32 myHost, short myPort, tdb->activeTrans = (struct ubik_trans *)0; memset(&tdb->version, 0, sizeof(struct ubik_version)); memset(&tdb->cachedVersion, 0, sizeof(struct ubik_version)); +#ifdef AFS_PTHREAD_ENV + assert(pthread_mutex_init(&tdb->versionLock, NULL) == 0); +#else Lock_Init(&tdb->versionLock); +#endif tdb->flags = 0; tdb->read = uphys_read; tdb->write = uphys_write; @@ -434,8 +438,6 @@ ubik_ServerInitCommon(afs_int32 myHost, short myPort, #ifdef AFS_PTHREAD_ENV assert(pthread_cond_init(&tdb->version_cond, NULL) == 0); assert(pthread_cond_init(&tdb->flags_cond, NULL) == 0); - assert(pthread_mutex_init(&tdb->version_mutex, NULL) == 0); - assert(pthread_mutex_init(&tdb->flags_mutex, NULL) == 0); #endif /* AFS_PTHREAD_ENV */ /* initialize RX */ @@ -662,16 +664,15 @@ BeginTrans(register struct ubik_dbase *dbase, afs_int32 transMode, if (transMode == UBIK_WRITETRANS) { /* if we're writing already, wait */ while (dbase->flags & DBWRITING) { - DBRELE(dbase); #ifdef AFS_PTHREAD_ENV - assert(pthread_mutex_lock(&dbase->flags_mutex) == 0); - assert(pthread_cond_wait(&dbase->flags_cond, &dbase->flags_mutex) == 0); - assert(pthread_mutex_unlock(&dbase->flags_mutex) == 0); + assert(pthread_cond_wait(&dbase->flags_cond, &dbase->versionLock) == 0); #else + DBRELE(dbase); LWP_WaitProcess(&dbase->flags); -#endif DBHOLD(dbase); +#endif } + if (!ubeacon_AmSyncSite()) { DBRELE(dbase); return UNOTSYNC; @@ -1194,16 +1195,19 @@ int ubik_WaitVersion(register struct ubik_dbase *adatabase, register struct ubik_version *aversion) { + DBHOLD(adatabase); while (1) { /* wait until version # changes, and then return */ - if (vcmp(*aversion, adatabase->version) != 0) + if (vcmp(*aversion, adatabase->version) != 0) { + DBRELE(adatabase); return 0; + } #ifdef AFS_PTHREAD_ENV - assert(pthread_mutex_lock(&adatabase->version_mutex) == 0); - assert(pthread_cond_wait(&adatabase->version_cond,&adatabase->version_mutex) == 0); - assert(pthread_mutex_unlock(&adatabase->version_mutex) == 0); + assert(pthread_cond_wait(&adatabase->version_cond, &adatabase->versionLock) == 0); #else + DBRELE(adatabase); LWP_WaitProcess(&adatabase->version); /* same vers, just wait */ + DBHOLD(adatabase); #endif } } diff --git a/src/ubik/ubik.p.h b/src/ubik/ubik.p.h index adcbdac..fcfd460 100644 --- a/src/ubik/ubik.p.h +++ b/src/ubik/ubik.p.h @@ -168,7 +168,9 @@ struct ubik_dbase { char *pathName; /*!< root name for dbase */ struct ubik_trans *activeTrans; /*!< active transaction list */ struct ubik_version version; /*!< version number */ -#if defined(UKERNEL) +#ifdef AFS_PTHREAD_ENV + pthread_mutex_t versionLock; /*!< lock on version number */ +#elif defined(UKERNEL) struct afs_lock versionLock; /*!< lock on version number */ #else /* defined(UKERNEL) */ struct Lock versionLock; /*!< lock on version number */ @@ -195,8 +197,6 @@ struct ubik_dbase { #ifdef AFS_PTHREAD_ENV pthread_cond_t version_cond; /*!< condition variable to manage changes to version */ pthread_cond_t flags_cond; /*!< condition variable to manage changes to flags */ - pthread_mutex_t version_mutex; - pthread_mutex_t flags_mutex; #endif }; @@ -291,8 +291,13 @@ struct ubik_server { }; /*! \name hold and release functions on a database */ -#define DBHOLD(a) ObtainWriteLock(&((a)->versionLock)) -#define DBRELE(a) ReleaseWriteLock(&((a)->versionLock)) +#ifdef AFS_PTHREAD_ENV +# define DBHOLD(a) assert(pthread_mutex_lock(&((a)->versionLock)) == 0) +# define DBRELE(a) assert(pthread_mutex_unlock(&((a)->versionLock)) == 0) +#else /* !AFS_PTHREAD_ENV */ +# define DBHOLD(a) ObtainWriteLock(&((a)->versionLock)) +# define DBRELE(a) ReleaseWriteLock(&((a)->versionLock)) +#endif /* !AFS_PTHREAD_ENV */ /*\}*/ /* globals */ -- 1.9.4