On Fri, Dec 08, 2017 at 11:55:53AM +0100, Paolo Bonzini wrote:
> @@ -88,9 +88,9 @@ static void *worker_thread(void *opaque)
>
> do {
> pool->idle_threads++;
> - qemu_mutex_unlock(&pool->lock);
> + qemu_lock_guard_unlock(&pool_guard);
> ret = qemu_sem_timedwait(&pool->sem, 10000);
> - qemu_mutex_lock(&pool->lock);
> + qemu_lock_guard_lock(&pool_guard);
Or:
QEMU_WITHOUT_LOCK_GUARD(pool_guard) {
ret = qemu_sem_timedwait(&pool->sem, 10000);
}
Seems funny but I like it. :)
Unfortunately it's tricky to come up with good semantics. A 'return'
statement inside QEMU_WITHOUT_LOCK_GUARD() should leave the lock
unlocked. But a 'break' statement may need to reacquire the lock...
> @@ -258,12 +254,12 @@ BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool,
>
> trace_thread_pool_submit(pool, req, arg);
>
> - qemu_mutex_lock(&pool->lock);
> + QEMU_LOCK_GUARD(QemuMutex, pool_guard, &pool->lock);
> if (pool->idle_threads == 0 && pool->cur_threads < pool->max_threads) {
> spawn_thread(pool);
> }
> QTAILQ_INSERT_TAIL(&pool->request_list, req, reqs);
> - qemu_mutex_unlock(&pool->lock);
> + qemu_lock_guard_unlock(&pool_guard);
Why not QEMU_WITH_LOCK()? Then you can get rid of the explicit unlock.
> @@ -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.
signature.asc
Description: PGP signature
