From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45968) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qm2xx-0005dH-Uc for qemu-devel@nongnu.org; Wed, 27 Jul 2011 08:11:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qm2xw-0005I8-Ob for qemu-devel@nongnu.org; Wed, 27 Jul 2011 08:11:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27161) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qm2xw-0005Hw-G5 for qemu-devel@nongnu.org; Wed, 27 Jul 2011 08:11:56 -0400 Message-ID: <4E30012D.4020209@redhat.com> Date: Wed, 27 Jul 2011 14:14:37 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1311672077-4592-1-git-send-email-stefanha@linux.vnet.ibm.com> <8739hs5a6n.fsf@skywalker.in.ibm.com> <4E2FE28D.5060206@redhat.com> <87tya83qc7.fsf@skywalker.in.ibm.com> In-Reply-To: <87tya83qc7.fsf@skywalker.in.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v8 0/5] Coroutines for better asynchronous programming List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Aneesh Kumar K.V" Cc: Blue Swirl , Anthony Liguori , Venkateswararao Jujjuri , Stefan Hajnoczi , qemu-devel@nongnu.org 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 > Date: Tue May 24 22:14:04 2011 +0530 > > coroutine: Add CoRwlock support > > Signed-off-by: Aneesh Kumar K.V 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