On Sun, Nov 01, 2015 at 04:55:51PM +0300, Denis V. Lunev wrote:
> 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.The owner thread field is only valid when nesting > 0. > with your proposal the function become racy if called from outer > thread. I need to think a bit here. This is thread-safe: bool owner; qemu_mutex_lock(&r->lock); owner = r->nesting > 0 && qemu_thread_is_self(&r->owner_thread); qemu_mutex_unlock(&r->lock); return owner;
signature.asc
Description: PGP signature
