[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 2/3] thread-pool: replace semaphore with condition variabl
From: |
Nicolas Saenz Julienne |
Subject: |
Re: [PATCH v3 2/3] thread-pool: replace semaphore with condition variable |
Date: |
Tue, 17 May 2022 16:37:45 +0200 |
User-agent: |
Evolution 3.44.1 (3.44.1-1.fc36) |
On Tue, 2022-05-17 at 16:18 +0200, Paolo Bonzini wrote:
> On 5/17/22 14:46, Nicolas Saenz Julienne wrote:
> > > - while (!pool->stopping) {
> > > + while (!pool->stopping && pool->cur_threads <= pool->max_threads) {
> > > ThreadPoolElement *req;
> > > int ret;
> > >
> > > - do {
> > > + if (QTAILQ_EMPTY(&pool->request_list)) {
> > > pool->idle_threads++;
> > > - qemu_mutex_unlock(&pool->lock);
> > > - ret = qemu_sem_timedwait(&pool->sem, 10000);
> > > - qemu_mutex_lock(&pool->lock);
> > > + ret = qemu_cond_timedwait(&pool->request_cond, &pool->lock,
> > > 10000);
> > > pool->idle_threads--;
> > > - } while (back_to_sleep(pool, ret));
> > > - if (ret == -1 || pool->stopping ||
> > > - pool->cur_threads > pool->max_threads) {
> > > - break;
> > > + if (ret == 0 &&
> > > + QTAILQ_EMPTY(&pool->request_list) &&
> > > + pool->cur_threads > pool->min_threads) {
> > > + /* Timed out + no work to do + no need for warm threads
> > > = exit. */
> > > + break;
> > > + }
> >
> > Some comments:
> >
> > - A completely idle pool will now never be able to lose its threads, as the
> > 'pool->cur_threads <= pool->max_threads' condition is only checked after
> > a
> > non-timeout wakeup.
>
> Are you sure? The full code is:
>
> ret = qemu_cond_timedwait(&pool->request_cond, &pool->lock,
> 10000);
> pool->idle_threads--;
> if (ret == 0 &&
> QTAILQ_EMPTY(&pool->request_list) &&
> pool->cur_threads > pool->min_threads) {
> /* Timed out + no work to do + no need for warm threads
> exit. */
> break;
> }
> /*
> * Even if there was some work to do, check if there aren't
> * too many worker threads before picking it up.
> */
> continue;
>
OK, I somehow missed this 'continue', I wonder if I managed to re-review v2 by
accident... Anyway, it looks fine to me now.
Regards,
--
Nicolás Sáenz
[PATCH v3 3/3] thread-pool: remove stopping variable, Paolo Bonzini, 2022/05/14
Re: [PATCH v3 0/3] thread-pool: fix performance regression, Nicolas Saenz Julienne, 2022/05/17