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

[Paolo Bonzini]

Il 01/06/2014 21:02, Marcin Gibuła ha scritto:
> > 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.
>
> Ok.
>
> > 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;
> >      }
>
> I'll test it tomorrow. I assume you want to avoid calling
> event_notifier_set() until function is reentered via aio_pool?

Yes.  But actually, I need to check if it's possible to fix 
bdrv_drain_all.  If you're in coroutine context, you can defer the 
draining to a safe point using a bottom half.  If you're not in 
coroutine context, perhaps bdrv_drain_all has to be made illegal.  Which 
means a bunch of code auditing...

Paolo

> > Finally, the same bug is also in block/linux-aio.c and
> > block/win32-aio.c.
>
> I can try with linux-aio, but my knowledge of windows api is zero...