Anthony Liguori wrote:
> On 06/16/2010 09:29 AM, Paolo Bonzini wrote:
> >On 06/16/2010 04:22 PM, Jamie Lokier wrote:
> >>Paolo Bonzini wrote:
> >>>These should be (at least for now) block-obj-$(CONFIG_POSIX).
> >>>
> >>>>+ while (QTAILQ_EMPTY(&(queue->request_list))&&
> >>>>+ (ret != ETIMEDOUT)) {
> >>>>+ ret = qemu_cond_timedwait(&(queue->cond),
> >>>>+ &(queue->lock), 10*100000);
> >>>>+ }
> >>>
> >>>Using qemu_cond_timedwait is a hack for not properly broadcasting the
> >>>condvar in flush_threadlet_queue.
> >>
> >>Are you sure? It looks like it also expires idle threads after a
> >>fixed amount of idle time.
> >
> >Unnecessary idle threads are immediately expired as soon as the
> >threadlet exits if ncecessary, since here
>
> If a threadlet is waiting to consume more work, unless we do a
> pthread_cancel (I dislike cancellation) it will keep waiting until it
> gets more work (which would mean it's not actually idle)...
There's some mild abuse of the mutex/condvar going on.
As (queue->exit || queue->idle_threads > queue->min_threads) is a
condition for breaking out of the loop, that condition ought to be
checked in the mutex->cond_wait region, but it isn't.
It doesn't matter here because the queue is empty when queue->exit,
and the idle > min_threads condition can't become true.
> >The min/max_threads parameters of the queue are currently immutable,
> >so it can never happen that a thread has to be expired while it's
> >waiting. It may well become true in the future, in which case the
> >condvar will have to be broadcast when min_threads changes.
Broadcasting when min_threads decreases wouldn't be enough, because
min_threads isn't checked inside the mutex->cond_wait region.
-- Jamie
