| Message ID | 87tya83qc7.fsf@skywalker.in.ibm.com |
|---|---|
| State | New |
| Headers | show |
Am 27.07.2011 13:39, schrieb Aneesh Kumar K.V: > Can you review the patch that add CoRWlock ? > > http://article.gmane.org/gmane.comp.emulators.qemu/105402 > Message-id:1307382497-3737-2-git-send-email-aneesh.kumar@linux.vnet.ibm.com > > commit 8c787d8b81aca1f4f7be45adb67b9e1a6dde7f1f > Author: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> > Date: Tue May 24 22:14:04 2011 +0530 > > coroutine: Add CoRwlock support > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> Nice! I think I'll need this, too. > diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c > index 5071fb8..5ecaa94 100644 > --- a/qemu-coroutine-lock.c > +++ b/qemu-coroutine-lock.c > @@ -115,3 +115,47 @@ void qemu_co_mutex_unlock(CoMutex *mutex) > > trace_qemu_co_mutex_unlock_return(mutex, self); > } > + > +void qemu_co_rwlock_init(CoRwlock *lock) > +{ > + memset(lock, 0, sizeof(*lock)); > + qemu_co_queue_init(&lock->queue); > +} > + > +void qemu_co_rwlock_rdlock(CoRwlock *lock) > +{ > + while (lock->writer) { > + qemu_co_queue_wait(&lock->queue); > + } > + lock->reader++; > +} > + > +void qemu_co_rwlock_unlock(CoRwlock *lock) > +{ > + assert(qemu_in_coroutine()); > + if (lock->writer) { > + lock->writer--;; Please don't do arithmetics on bools, just say lock->write = false; Also there's a double semicolon. > + assert(lock->writer == 0); > + while (!qemu_co_queue_empty(&lock->queue)) { > + /* > + * Wakeup every body. This will include some > + * writers too. > + */ > + qemu_co_queue_next(&lock->queue); > + } > + } else { > + lock->reader--; > + assert(lock->reader >= 0); > + /* Wakeup only one waiting writer */ > + qemu_co_queue_next(&lock->queue); This is only useful if lock->reader == 0. > + } > +} > + > +void qemu_co_rwlock_wrlock(CoRwlock *lock) > +{ > + while (lock->writer || lock->reader) { > + qemu_co_queue_wait(&lock->queue); > + } > + lock->writer++; > + assert(lock->writer == 1); > +} I wonder if we should have a mechanism that stops new readers from taking the lock while a writer is waiting in order to avoid starvation. Anyway, the locking itself looks correct. Kevin
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c index 5071fb8..5ecaa94 100644 --- a/qemu-coroutine-lock.c +++ b/qemu-coroutine-lock.c @@ -115,3 +115,47 @@ void qemu_co_mutex_unlock(CoMutex *mutex) trace_qemu_co_mutex_unlock_return(mutex, self); } + +void qemu_co_rwlock_init(CoRwlock *lock) +{ + memset(lock, 0, sizeof(*lock)); + qemu_co_queue_init(&lock->queue); +} + +void qemu_co_rwlock_rdlock(CoRwlock *lock) +{ + while (lock->writer) { + qemu_co_queue_wait(&lock->queue); + } + lock->reader++; +} + +void qemu_co_rwlock_unlock(CoRwlock *lock) +{ + assert(qemu_in_coroutine()); + if (lock->writer) { + lock->writer--;; + assert(lock->writer == 0); + while (!qemu_co_queue_empty(&lock->queue)) { + /* + * Wakeup every body. This will include some + * writers too. + */ + qemu_co_queue_next(&lock->queue); + } + } else { + lock->reader--; + assert(lock->reader >= 0); + /* Wakeup only one waiting writer */ + qemu_co_queue_next(&lock->queue); + } +} + +void qemu_co_rwlock_wrlock(CoRwlock *lock) +{ + while (lock->writer || lock->reader) { + qemu_co_queue_wait(&lock->queue); + } + lock->writer++; + assert(lock->writer == 1); +} diff --git a/qemu-coroutine.h b/qemu-coroutine.h index 71cfa5a..a9735fb 100644 --- a/qemu-coroutine.h +++ b/qemu-coroutine.h @@ -17,6 +17,7 @@ #include <stdbool.h> #include "qemu-queue.h" +#include "qemu-thread.h" /** * Coroutines are a mechanism for stack switching and can be used for @@ -114,4 +115,15 @@ void qemu_co_mutex_init(CoMutex *mutex); void qemu_co_mutex_lock(CoMutex *mutex); void qemu_co_mutex_unlock(CoMutex *mutex); +typedef struct CoRwlock { + bool writer; + int reader; + CoQueue queue; +} CoRwlock; + +void qemu_co_rwlock_init(CoRwlock *lock); +void qemu_co_rwlock_rdlock(CoRwlock *lock); +void qemu_co_rwlock_wrlock(CoRwlock *lock); +void qemu_co_rwlock_unlock(CoRwlock *lock); + #endif /* QEMU_COROUTINE_H */