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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 04/06/2014 12:01, Stefan Hajnoczi ha scritto:
>> > Without this patch, I could reproduce this bug with snapshot-commit with
>> > about 1 per 10 tries. With this patch, I couldn't reproduce it any more.
>> >
>> > Signed-off-by: Marcin Gibula <m.gibula@beyond.pl>
>> > ---
> This is an interesting bug that definitely needs a test case to prevent
> regressions in the future.
>
> Please take a look at tests/test-thread-pool.c and add a test to it.  It
> can be reproduced deterministically - just call aio_poll() after the
> dummy worker functions have both completed.  Then the next aio_poll()
> call in the thread pool callback will suffer the problem you described.

The question if we want to consider this thread-pool.c behavior a real 
bug or just a misfeature (the real bug being elsewhere).

Even though this patch avoids the performance problems of v1, we would 
have to fix at least two other cases and it's not obvious (a) that those 
two are the only ones (b) tgat those two can be fixed without affecting 
performance.

If the bottom half code is immune from this event notifier problem, 
bdrv_drain/bdrv_drain_all calls in coroutine context can defer the 
actual draining to a bottom half and reenter the coroutine afterwards; 
we can then audit that all other calls should come from the main loop 
rather than aio_poll.

Paolo