Re: [Qemu-devel] [PATCH 5/5] thread-pool: convert to use lock guards

[Peter Xu]

On Fri, Dec 08, 2017 at 03:13:06PM +0000, Stefan Hajnoczi wrote:

[...]

> > @@ -330,7 +326,7 @@ void thread_pool_free(ThreadPool *pool)
> >  
> >      assert(QLIST_EMPTY(&pool->head));
> >  
> > -    qemu_mutex_lock(&pool->lock);
> > +    QEMU_LOCK_GUARD(QemuMutex, pool_guard, &pool->lock);
> >  
> >      /* Stop new threads from spawning */
> >      qemu_bh_delete(pool->new_thread_bh);
> > @@ -344,7 +340,7 @@ void thread_pool_free(ThreadPool *pool)
> >          qemu_cond_wait(&pool->worker_stopped, &pool->lock);
> >      }
> >  
> > -    qemu_mutex_unlock(&pool->lock);
> > +    qemu_lock_guard_unlock(&pool_guard);
> 
> Here too.  I don't see the advantage of replacing a single lock/unlock
> with QEMU_LOCK_GUARD/unlock, if it cannot be made shorter/safer then
> it's fine to use QemuMutex directly.

Agree.

For such use cases (and maybe mostly the cases that can use
QEMU_ADOPT_LOCK, QEMU_RETURN_LOCK, QEMU_TAKEN_LOCK) for me I would
still prefer the old ways, which is clearer to me (and faster?).  But
I do think the guard can really help for the specific case when
e.g. we need to take the lock at the entry of the function but need to
make sure it's released before leaving, especially when the function
contains multiple return points.

Maybe we can do this incrementally?  Say, we can at least start to use
it in new codes, and replace some obvious scenarios where proper.
After all it sounds good to have such an API that QEMU code can use
directly.

Thanks,

-- 
Peter Xu