semaphore: fix a hangup problem under load on NetBSD hosts.
Fix following bugs in "fallback implementation of counting semaphores
with mutex+condvar" added in c166cb72f1
:
- waiting threads are not restarted properly if more than one threads
are waiting unblock signals in qemu_sem_timedwait()
- possible missing pthread_cond_signal(3) calls when waiting threads
are returned by ETIMEDOUT
- fix an uninitialized variable
The problem is analyzed by and fix is provided by Noriyuki Soda.
Also put additional cleanup suggested by Laszlo Ersek:
- make QemuSemaphore.count unsigned (it won't be negative)
- check a return value of in pthread_cond_wait() in qemu_sem_wait()
Signed-off-by: Izumi Tsutsui <tsutsui@ceres.dti.ne.jp>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Message-id: 1372841894-10634-1-git-send-email-tsutsui@ceres.dti.ne.jp
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
This commit is contained in:
parent
e1d0fb378a
commit
79761c6681
@ -15,7 +15,7 @@ struct QemuSemaphore {
|
|||||||
#if defined(__APPLE__) || defined(__NetBSD__)
|
#if defined(__APPLE__) || defined(__NetBSD__)
|
||||||
pthread_mutex_t lock;
|
pthread_mutex_t lock;
|
||||||
pthread_cond_t cond;
|
pthread_cond_t cond;
|
||||||
int count;
|
unsigned int count;
|
||||||
#else
|
#else
|
||||||
sem_t sem;
|
sem_t sem;
|
||||||
#endif
|
#endif
|
||||||
|
@ -170,12 +170,11 @@ void qemu_sem_post(QemuSemaphore *sem)
|
|||||||
|
|
||||||
#if defined(__APPLE__) || defined(__NetBSD__)
|
#if defined(__APPLE__) || defined(__NetBSD__)
|
||||||
pthread_mutex_lock(&sem->lock);
|
pthread_mutex_lock(&sem->lock);
|
||||||
if (sem->count == INT_MAX) {
|
if (sem->count == UINT_MAX) {
|
||||||
rc = EINVAL;
|
rc = EINVAL;
|
||||||
} else if (sem->count++ < 0) {
|
|
||||||
rc = pthread_cond_signal(&sem->cond);
|
|
||||||
} else {
|
} else {
|
||||||
rc = 0;
|
sem->count++;
|
||||||
|
rc = pthread_cond_signal(&sem->cond);
|
||||||
}
|
}
|
||||||
pthread_mutex_unlock(&sem->lock);
|
pthread_mutex_unlock(&sem->lock);
|
||||||
if (rc != 0) {
|
if (rc != 0) {
|
||||||
@ -207,19 +206,21 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
|
|||||||
struct timespec ts;
|
struct timespec ts;
|
||||||
|
|
||||||
#if defined(__APPLE__) || defined(__NetBSD__)
|
#if defined(__APPLE__) || defined(__NetBSD__)
|
||||||
|
rc = 0;
|
||||||
compute_abs_deadline(&ts, ms);
|
compute_abs_deadline(&ts, ms);
|
||||||
pthread_mutex_lock(&sem->lock);
|
pthread_mutex_lock(&sem->lock);
|
||||||
--sem->count;
|
while (sem->count == 0) {
|
||||||
while (sem->count < 0) {
|
|
||||||
rc = pthread_cond_timedwait(&sem->cond, &sem->lock, &ts);
|
rc = pthread_cond_timedwait(&sem->cond, &sem->lock, &ts);
|
||||||
if (rc == ETIMEDOUT) {
|
if (rc == ETIMEDOUT) {
|
||||||
++sem->count;
|
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
if (rc != 0) {
|
if (rc != 0) {
|
||||||
error_exit(rc, __func__);
|
error_exit(rc, __func__);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (rc != ETIMEDOUT) {
|
||||||
|
--sem->count;
|
||||||
|
}
|
||||||
pthread_mutex_unlock(&sem->lock);
|
pthread_mutex_unlock(&sem->lock);
|
||||||
return (rc == ETIMEDOUT ? -1 : 0);
|
return (rc == ETIMEDOUT ? -1 : 0);
|
||||||
#else
|
#else
|
||||||
@ -249,16 +250,19 @@ int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
|
|||||||
|
|
||||||
void qemu_sem_wait(QemuSemaphore *sem)
|
void qemu_sem_wait(QemuSemaphore *sem)
|
||||||
{
|
{
|
||||||
#if defined(__APPLE__) || defined(__NetBSD__)
|
|
||||||
pthread_mutex_lock(&sem->lock);
|
|
||||||
--sem->count;
|
|
||||||
while (sem->count < 0) {
|
|
||||||
pthread_cond_wait(&sem->cond, &sem->lock);
|
|
||||||
}
|
|
||||||
pthread_mutex_unlock(&sem->lock);
|
|
||||||
#else
|
|
||||||
int rc;
|
int rc;
|
||||||
|
|
||||||
|
#if defined(__APPLE__) || defined(__NetBSD__)
|
||||||
|
pthread_mutex_lock(&sem->lock);
|
||||||
|
while (sem->count == 0) {
|
||||||
|
rc = pthread_cond_wait(&sem->cond, &sem->lock);
|
||||||
|
if (rc != 0) {
|
||||||
|
error_exit(rc, __func__);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
--sem->count;
|
||||||
|
pthread_mutex_unlock(&sem->lock);
|
||||||
|
#else
|
||||||
do {
|
do {
|
||||||
rc = sem_wait(&sem->sem);
|
rc = sem_wait(&sem->sem);
|
||||||
} while (rc == -1 && errno == EINTR);
|
} while (rc == -1 && errno == EINTR);
|
||||||
|
Loading…
Reference in New Issue
Block a user