diff mbox

[2/5] semaphore: implement fallback counting semaphores with mutex+condvar

Message ID 1351862047-23172-3-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Nov. 2, 2012, 1:14 p.m. UTC 
OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
for them.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-thread-posix.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-thread-posix.h |  6 +++++
 2 file modificati, 80 inserzioni(+)

Comments

Peter Maydell Nov. 2, 2012, 1:50 p.m. UTC | #1 
On 2 November 2012 14:14, Paolo Bonzini <pbonzini@redhat.com> wrote:
> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
> +    struct timespec ts;
> +    clock_gettime(CLOCK_REALTIME, &ts);


qemu-thread-posix.c:198:5: warning: implicit declaration of function
'clock_gettime' is invalid in C99
      [-Wimplicit-function-declaration]
    clock_gettime(CLOCK_REALTIME, &ts);
    ^
qemu-thread-posix.c:198:19: error: use of undeclared identifier 'CLOCK_REALTIME'
    clock_gettime(CLOCK_REALTIME, &ts);
                  ^
1 warning and 1 error generated.
make: *** [qemu-thread-posix.o] Error 1

MacOS doesn't implement clock_gettime()...

-- PMM
Brad Smith Nov. 18, 2012, 9:09 a.m. UTC | #2 
On 11/02/12 09:14, Paolo Bonzini wrote:
> OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
> for them.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   qemu-thread-posix.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   qemu-thread-posix.h |  6 +++++
>   2 file modificati, 80 inserzioni(+)
>
> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> index 6a3d3a1..048db8f 100644
> --- a/qemu-thread-posix.c
> +++ b/qemu-thread-posix.c
> @@ -122,36 +122,100 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
>   {
>       int rc;
>
> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)

OpenBSD 5.2 & -current (libpthread) / NetBSD -current (librt) have 
supported sem_timedwait() for roughly 8 months now. Please change this 
to properly test for the presence of sem_timedwait() within the 
configure script.
Paolo Bonzini Nov. 18, 2012, 4:06 p.m. UTC | #3 
Il 18/11/2012 10:09, Brad Smith ha scritto:
> On 11/02/12 09:14, Paolo Bonzini wrote:
>> OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
>> for them.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>   qemu-thread-posix.c | 74
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-thread-posix.h |  6 +++++
>>   2 file modificati, 80 inserzioni(+)
>>
>> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
>> index 6a3d3a1..048db8f 100644
>> --- a/qemu-thread-posix.c
>> +++ b/qemu-thread-posix.c
>> @@ -122,36 +122,100 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
>>   {
>>       int rc;
>>
>> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
> 
> OpenBSD 5.2 & -current (libpthread) / NetBSD -current (librt) have
> supported sem_timedwait() for roughly 8 months now. Please change this
> to properly test for the presence of sem_timedwait() within the
> configure script.

Please submit a patch.  The patched code works, and it's not even
suboptimal because *BSD use a mutex/condvar to implement semaphores.  We
end up executing the very same code.

Paolo
Brad Smith Nov. 27, 2012, 2:56 a.m. UTC | #4 
On Sun, Nov 18, 2012 at 05:06:40PM +0100, Paolo Bonzini wrote:
> Il 18/11/2012 10:09, Brad Smith ha scritto:
> > On 11/02/12 09:14, Paolo Bonzini wrote:
> >> OpenBSD and Darwin do not have sem_timedwait.  Implement a fallback
> >> for them.
> >>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >>   qemu-thread-posix.c | 74
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>   qemu-thread-posix.h |  6 +++++
> >>   2 file modificati, 80 inserzioni(+)
> >>
> >> diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
> >> index 6a3d3a1..048db8f 100644
> >> --- a/qemu-thread-posix.c
> >> +++ b/qemu-thread-posix.c
> >> @@ -122,36 +122,100 @@ void qemu_sem_init(QemuSemaphore *sem, int init)
> >>   {
> >>       int rc;
> >>
> >> +#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
> > 
> > OpenBSD 5.2 & -current (libpthread) / NetBSD -current (librt) have
> > supported sem_timedwait() for roughly 8 months now. Please change this
> > to properly test for the presence of sem_timedwait() within the
> > configure script.
> 
> Please submit a patch.  The patched code works, and it's not even
> suboptimal because *BSD use a mutex/condvar to implement semaphores.  We
> end up executing the very same code.

I understand what you mean. It's more so out of principle to try and remove
workarounds wherever possible when it is possible to do so.
diff mbox

Patch

diff --git a/qemu-thread-posix.c b/qemu-thread-posix.c
index 6a3d3a1..048db8f 100644
--- a/qemu-thread-posix.c
+++ b/qemu-thread-posix.c
@@ -122,36 +122,100 @@  void qemu_sem_init(QemuSemaphore *sem, int init)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    rc = pthread_mutex_init(&sem->lock, NULL);
+    if (rc != 0) {
+        error_exit(rc, __func__);
+    }
+    rc = pthread_cond_init(&sem->cond, NULL);
+    if (rc != 0) {
+        error_exit(rc, __func__);
+    }
+    if (init < 0) {
+        error_exit(EINVAL, __func__);
+    }
+    sem->count = init;
+#else
     rc = sem_init(&sem->sem, 0, init);
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 void qemu_sem_destroy(QemuSemaphore *sem)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    rc = pthread_cond_destroy(&sem->cond);
+    if (rc < 0) {
+        error_exit(rc, __func__);
+    }
+    rc = pthread_mutex_destroy(&sem->lock);
+    if (rc < 0) {
+        error_exit(rc, __func__);
+    }
+#else
     rc = sem_destroy(&sem->sem);
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 void qemu_sem_post(QemuSemaphore *sem)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    pthread_mutex_lock(&sem->lock);
+    if (sem->count == INT_MAX) {
+        rc = EINVAL;
+    } else if (sem->count++ < 0) {
+        rc = pthread_cond_signal(&sem->cond);
+    } else {
+        rc = 0;
+    }
+    pthread_mutex_unlock(&sem->lock);
+    if (rc != 0) {
+        error_exit(rc, __func__);
+    }
+#else
     rc = sem_post(&sem->sem);
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
 {
     int rc;
 
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    struct timespec ts;
+    clock_gettime(CLOCK_REALTIME, &ts);
+    if (ms) {
+        int nsec = ts.tv_nsec + (ms % 1000) * 1000000;
+        ts.tv_sec += ms / 1000 + nsec / 1000000000;
+        ts.tv_nsec = nsec % 1000000000;
+    }
+
+    pthread_mutex_lock(&sem->lock);
+    --sem->count;
+    while (sem->count < 0) {
+        rc = pthread_cond_timedwait(&sem->cond, &sem->lock, &ts);
+        if (rc == ETIMEDOUT) {
+            break;
+        }
+        if (rc != 0) {
+            error_exit(rc, __func__);
+        }
+    }
+    pthread_mutex_unlock(&sem->lock);
+    return (rc == ETIMEDOUT ? -1 : 0);
+#else
     if (ms <= 0) {
         /* This is cheaper than sem_timedwait.  */
         do {
@@ -181,10 +245,19 @@  int qemu_sem_timedwait(QemuSemaphore *sem, int ms)
         error_exit(errno, __func__);
     }
     return 0;
+#endif
 }
 
 void qemu_sem_wait(QemuSemaphore *sem)
 {
+#if defined(__OpenBSD__) || 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;
 
     do {
@@ -193,6 +266,7 @@  void qemu_sem_wait(QemuSemaphore *sem)
     if (rc < 0) {
         error_exit(errno, __func__);
     }
+#endif
 }
 
 void qemu_thread_create(QemuThread *thread,
diff --git a/qemu-thread-posix.h b/qemu-thread-posix.h
index 2542c15..1c098c2 100644
--- a/qemu-thread-posix.h
+++ b/qemu-thread-posix.h
@@ -12,7 +12,13 @@  struct QemuCond {
 };
 
 struct QemuSemaphore {
+#if defined(__OpenBSD__) || defined(__APPLE__) || defined(__NetBSD__)
+    pthread_mutex_t lock;
+    pthread_cond_t cond;
+    int count;
+#else
     sem_t sem;
+#endif
 };
 
 struct QemuThread {