On 06/03/2018 11:53, Stefan Hajnoczi wrote:
> Nested BDRV_POLL_WHILE() calls can occur. Currently
> assert(!bs_->wakeup) will fail when this happens.
>
> This patch converts bs->wakeup from bool to a counter.
>
> Nesting works correctly because outer BDRV_POLL_WHILE() callers evaluate
> the condition again after the inner caller completes (invoking the inner
> caller counts as aio_poll() progress).
>
> Reported-by: "fuweiwei (C)"
> Cc: Paolo Bonzini
> Signed-off-by: Stefan Hajnoczi
> ---
> Fu Weiwei: Please retest and let us know if this fixes the assertion
> failure you hit. Thanks!
> ---
> include/block/block.h | 7 +++
> include/block/block_int.h | 2 +-
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index fac401ba3e..990b97f0ad 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -385,9 +385,8 @@ void bdrv_drain_all(void);
> * other I/O threads' AioContexts (see for example \
> * block_job_defer_to_main_loop for how to do it). \
> */\
> -assert(!bs_->wakeup); \
> -/* Set bs->wakeup before evaluating cond. */ \
> -atomic_mb_set(&bs_->wakeup, true); \
> +/* Increment bs->wakeup before evaluating cond. */ \
> +atomic_inc(&bs_->wakeup); \
> while (busy_) {\
> if ((cond)) { \
> waited_ = busy_ = true;\
> @@ -399,7 +398,7 @@ void bdrv_drain_all(void);
> waited_ |= busy_; \
> } \
> } \
> -atomic_set(&bs_->wakeup, false); \
> +atomic_dec(&bs_->wakeup); \
> } \
> waited_; })
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 5ea63f8fa8..0f360c0ed5 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -712,7 +712,7 @@ struct BlockDriverState {
> /* Internal to BDRV_POLL_WHILE and bdrv_wakeup. Accessed with atomic
> * ops.
> */
> -bool wakeup;
> +unsigned wakeup;
>
> /* counter for nested bdrv_io_plug.
> * Accessed with atomic ops.
>
Reviewed-by: Paolo Bonzini
At least, the assertion made it fail fast. :)
Paolo