RE: [PATCH] thread-pool: signal condition variable while holding lock

Paolo Bonzini <[email protected]> wrote:
> On 3/30/26 15:01, Stepan Popov wrote:
> > Move qemu_cond_signal() inside the critical section protected by pool->lock.
> > Signaling while holding the lock imposes more predictable scheduling behavior.
> >
> > Signed-off-by: Stepan Popov <[email protected]>
> > ---
> >   util/thread-pool.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/util/thread-pool.c b/util/thread-pool.c index 
> > 8f8cb38d5c..8e55aefd07 100644
> > --- a/util/thread-pool.c
> > +++ b/util/thread-pool.c
> > @@ -265,8 +265,8 @@ BlockAIOCB *thread_pool_submit_aio(ThreadPoolFunc *func, void *arg,
> >           spawn_thread(pool);
> >       }
> >       QTAILQ_INSERT_TAIL(&pool->request_list, req, reqs);
> > -    qemu_mutex_unlock(&pool->lock);
> >       qemu_cond_signal(&pool->request_cond);
> > +    qemu_mutex_unlock(&pool->lock);
> >       return &req->common;
> >   }
>
> In this code, in the past even very small changes had big impact on 
> performance.  So, without some measurement I am wary of taking this change.
>
> What is "more predictable scheduling behavior" is unclear, too.  If 
> thead_pool_submit_aio() is preempted between qemu_cond_signal() and 
> qemu_mutex_unlock(), then not only the waiting thread cannot start the 
> work, but the mutex is taken and no one else can submit other requests.

Thank you for your feedback. I understand your concern about performance.

To explain the motivation: we have seen repetitive deadlocks in production after running qemu-img info --output=json image.qcow. 
Submitter is stuck in pthread_cond_signal (trying to __condvar_quiesce_and_switch_g1) and waiter in __pthread_cond_wait_common (trying to __condvar_cancel_waiting). 
Our glibc version has a known bug leading to deadlock (Ubuntu GLIBC 2.35-0ubuntu3.13):
https://sourceware.org/bugzilla/show_bug.cgi?id=25847 (pthread_cond_signal failed to wake up due to a bug in undoing stealing).

We are not sure if this change will fix it, 
but holding the mutex while signaling is preferred for predictable scheduling behaviour, 
as stated in the pthread man page (https://linux.die.net/man/3/pthread_cond_signal). 
Theoretically, it could help with similar problems.

To move forward, could you suggest a set of benchmarks that would be sensitive to this change? 
Maybe there are existing scripts or tools that you would recommend running? 
Which QEMU use cases (block I/O, virtio-scsi, file-backed disks, etc.) have historically shown performance changes with similar changes?

Thank you again.

Best regards,
Stepan