include/block/block.h | 7 +++---- include/block/block_int.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-)
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)" <fuweiwei2@huawei.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
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.
--
2.14.3
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)" <fuweiwei2@huawei.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> 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 <pbonzini@redhat.com>
At least, the assertion made it fail fast. :)
Paolo
Am 06.03.2018 um 11:53 hat Stefan Hajnoczi geschrieben: > 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)" <fuweiwei2@huawei.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Doesn't this conflict with your own AIO_WAIT_WHILE() patch? Kevin
On Tue, Mar 06, 2018 at 12:08:44PM +0100, Kevin Wolf wrote: > Am 06.03.2018 um 11:53 hat Stefan Hajnoczi geschrieben: > > 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)" <fuweiwei2@huawei.com> > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > Doesn't this conflict with your own AIO_WAIT_WHILE() patch? Yes, I wanted this patch to be easy for Weiwei to test without dependencies. AIO_WAIT_WHILE() has just hit qemu.git/master, so I'll rebase and send a v2. Stefan
© 2016 - 2025 Red Hat, Inc.