Re: [Qemu-devel] [PATCH] thread-pool: fix deadlock when callbacks depends on each other

[Paolo Bonzini]

Il 31/05/2014 20:29, Marcin Gibuła ha scritto:
> @@ -185,6 +191,14 @@ restart:
>          }
>          if (elem->state == THREAD_DONE && elem->common.cb) {
>              QLIST_REMOVE(elem, all);
> +            /* If more completed requests are waiting, notifier needs
> +             * to be rearmed so callback can progress with aio_pool().
> +             */
> +            pool->pending_completions--;
> +            if (pool->pending_completions) {
> +                event_notifier_set(notifier);
> +            }
> +
>              /* Read state before ret.  */
>              smp_rmb();
>              elem->common.cb(elem->common.opaque, elem->ret);

Good catch!  The main problem with the patch is that you need to use 
atomic_inc/atomic_dec to increment and decrement pool->pending_completions.

Secondarily, event_notifier_set is pretty heavy-weight, does it work if 
you wrap the loop like this?

restart:
    QLIST_FOREACH_SAFE(elem, &pool->head, all, next) {
        ...
    }
    if (pool->pending_completions) {
        goto restart;
    }
    event_notifier_test_and_clear(notifier);
    if (pool->pending_completions) {
        event_notifier_set(notifier);
        goto restart;
    }

Finally, the same bug is also in block/linux-aio.c and block/win32-aio.c.

Paolo