Re: [Qemu-devel] qemu_cond_signal() taking a long time to complete.

[Stefan Hajnoczi]

On Mon, Oct 4, 2010 at 11:40 AM, Arun R Bharadwaj
<address@hidden> wrote:
> I suspect this is the reason why
> I am seeing performance hit with the threading model, because this
> time is much more than the time needed to complete the entire
> v9fs_read call in the non threading model case.

If the cost of the thread pool is in the same range or higher than the
cost of the operation itself, then the thread pool is not a good
choice.

I think 15 microseconds overhead sounds high but not massively.  Can
you profile it in more detail or collect lock statistics (hold time,
contention, etc)?

> ---
>
> /* Code to sumbit work to the queue */
> void submit_threadlet_to_queue(ThreadletQueue *queue, ThreadletWork
> *work)
> {
>    qemu_mutex_lock(&(queue->lock));
>    if (queue->idle_threads == 0 && queue->cur_threads <
> queue->max_threads) {
>        spawn_threadlet(queue);
>    }
>    QTAILQ_INSERT_TAIL(&(queue->request_list), work, node);
>    qemu_cond_signal(&(queue->cond));
>    qemu_mutex_unlock(&(queue->lock));
> }
>
> /* Worker thread code */
> static void *threadlet_worker(void *data)
> {
>    ThreadletQueue *queue = data;
>
>    while (1) {
>        ThreadletWork *work;
>        int ret = 0;
>        qemu_mutex_lock(&(queue->lock));

Move this above the while() loop.

>
>        while (QTAILQ_EMPTY(&(queue->request_list)) &&
>               (ret != ETIMEDOUT)) {
>            ret = qemu_cond_timedwait(&(queue->cond),
>                                         &(queue->lock), 10*100000);
>        }
>
>        assert(queue->idle_threads != 0);
>        if (QTAILQ_EMPTY(&(queue->request_list))) {
>            if (queue->cur_threads > queue->min_threads) {
>                /* We retain the minimum number of threads */
>                break;

Note we're breaking here with queue->lock still held!

>            }
>        } else {
>            work = QTAILQ_FIRST(&(queue->request_list));
>            QTAILQ_REMOVE(&(queue->request_list), work, node);
>
>            queue->idle_threads--;
>            qemu_mutex_unlock(&(queue->lock));
>
>            /* execute the work function */
>            work->func(work);
>
>            qemu_mutex_lock(&(queue->lock));
>            queue->idle_threads++;
>        }
>        qemu_mutex_unlock(&(queue->lock));

Move this down before return NULL.

>    }
>
>    queue->idle_threads--;
>    queue->cur_threads--;
>
>    return NULL;
> }