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

[Stefan Hajnoczi <stefanha@gmail.com>]

On Mon, Jun 02, 2014 at 09:15:27AM +0200, Marcin Gibuła wrote:
> When two coroutines submit I/O and first coroutine depends on second to
> complete (by calling bdrv_drain_all), deadlock may occur.

bdrv_drain_all() is a very heavy-weight operation.  Coroutines should
avoid it if possible.  Please post the file/line/function where this
call was made, perhaps there is a better way to wait for the other
coroutine.  This isn't a fix for this bug but it's a cleanup.

> This is because both requests may have completed before thread pool notifier
> got called. Then, when notifier gets executed and first coroutine calls
> aio_pool() to make progress, it will hang forever, as notifier's descriptor
> has been already marked clear.
> 
> This patch fixes this, by deferring clearing notifier until no completions
> are pending.
> 
> 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.

Stefan