diff mbox

[v2] block: make BDRV_POLL_WHILE() re-entrancy safe

Message ID 015d4216-88f1-ea17-fefa-c4e93ec56ddd@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Blake March 7, 2018, 11:19 p.m. UTC 
On 03/07/2018 06:46 AM, Stefan Hajnoczi wrote:
> Nested BDRV_POLL_WHILE() calls can occur.  Currently
> assert(!wait_->wakeup) fails in AIO_WAIT_WHILE() when this happens.
> 
> This patch converts the bool wait_->need_kick flag to an unsigned
> wait_->num_waiters counter.
> 
> Nesting works correctly because outer AIO_WAIT_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>
> ---
> v2:
>   * Rebase onto qemu.git/master now that AIO_WAIT_WHILE() has landed
>     [Kevin]
> 
>   include/block/aio-wait.h | 61 ++++++++++++++++++++++++------------------------

Looks big due to whitespace change when column for trailing \ changed. 
Viewing the diff with whitespace ignored made it easier to review.

Reviewed-by: Eric Blake <eblake@redhat.com>

  /**
@@ -84,9 +84,8 @@ typedef struct {
      } else {                                                       \
          assert(qemu_get_current_aio_context() ==                   \
                 qemu_get_aio_context());                            \
-        assert(!wait_->need_kick);                          \
-        /* Set wait_->need_kick before evaluating cond.  */ \
-        atomic_mb_set(&wait_->need_kick, true);             \
+        /* Increment wait_->num_waiters before evaluating cond. */ \
+        atomic_inc(&wait_->num_waiters);                           \
          while (busy_) {                                            \
              if ((cond)) {                                          \
                  waited_ = busy_ = true;                            \
@@ -98,7 +97,7 @@ typedef struct {
                  waited_ |= busy_;                                  \
              }                                                      \
          }                                                          \
-        atomic_set(&wait_->need_kick, false);               \
+        atomic_dec(&wait_->num_waiters);                           \
      }                                                              \
      waited_; })
diff mbox

Patch

diff --git c/include/block/aio-wait.h w/include/block/aio-wait.h
index a48c744fa87..74cde07bef3 100644
--- c/include/block/aio-wait.h
+++ w/include/block/aio-wait.h
@@ -50,8 +50,8 @@ 
   *   }
   */
  typedef struct {
-    /* Is the main loop waiting for a kick?  Accessed with atomic ops. */
-    bool need_kick;
+    /* Number of waiting AIO_WAIT_WHILE() callers. Accessed with atomic 
ops. */
+    unsigned num_waiters;
  } AioWait;