On 10/30/2015 06:41 PM, Stefan Hajnoczi wrote:
On Wed, Oct 28, 2015 at 06:01:02PM +0300, Denis V. Lunev wrote:
+int rfifolock_is_locked(RFifoLock *r);
Please use bool instead of int.
diff --git a/util/rfifolock.c b/util/rfifolock.c
index afbf748..8ac58cb 100644
--- a/util/rfifolock.c
+++ b/util/rfifolock.c
@@ -48,7 +48,7 @@ void rfifolock_lock(RFifoLock *r)
/* Take a ticket */
unsigned int ticket = r->tail++;
- if (r->nesting > 0 && qemu_thread_is_self(&r->owner_thread)) {
+ if (r->nesting > 0 && rfifolock_is_locked(r)) {
r->tail--; /* put ticket back, we're nesting */
} else {
while (ticket != r->head) {
@@ -69,10 +69,15 @@ void rfifolock_unlock(RFifoLock *r)
{
qemu_mutex_lock(&r->lock);
assert(r->nesting > 0);
- assert(qemu_thread_is_self(&r->owner_thread));
+ assert(rfifolock_is_locked(r));
if (--r->nesting == 0) {
r->head++;
qemu_cond_broadcast(&r->cond);
}
qemu_mutex_unlock(&r->lock);
}
+
+int rfifolock_is_locked(RFifoLock *r)
+{
+ return qemu_thread_is_self(&r->owner_thread);
+}
The function name confused me since "does the current thread hold the
lock?" != "does anyone currently hold the lock?".
I suggest:
bool rfifolock_held_by_current_thread(RFifoLock *r) {
return r->nesting > 0 && qemu_thread_is_self(&r->owner_thread);
}
Then the r->nesting > 0 testing can also be dropped by callers, which is
good since rfifolock_is_locked() does not return a meaningful result
when r->nesting == 0.
actually the function is broken in the current state:
aio_context_acquire()
aio_context_release()
aio_context_is_locked() will return true.
the problem is that owner thread is not reset on lock release.
with your proposal the function become racy if called from outer
thread. I need to think a bit here.
May be we'll need 2 versions - locked and unlocked or
something other should be added. I am in progress of
invention :)
Den
