[Qemu-devel] [PATCH v5 00/22] block: Asynchronous request cancellation

[Qemu-devel] [PATCH v5 00/22] block: Asynchronous request cancellation

[Fam Zheng]

v5: Fix IDE callback. (Paolo)
    Fix blkdebug. (Paolo)
    Drop the DMA fix which is independent of this series. (Paolo)
    Incorperate Yuan's patch on quorum_aio_cancel. (Benoît)
    Commit message wording fix. (Benoît)
    Rename qemu_aio_release to qemu_aio_unref. (Benoît)

v4: Drop AIOCBInfo.cancel.

This series adds a new block layer API:

  void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);

And use it to emulate bdrv_aio_cancel.

The function is similar to bdrv_aio_cancel in that it cancels an AIO request,
but different that it doesn't block until the request is completely cancelled
or done.

More importantly, the completion callback, BlockDriverAIOCB.cb, is guaranteed
to be called, so that the cb can take care of resource releasing and status
reporting to guest, etc.

In the following work, scsi emulation code will be shifted to use the async
cancelling.

One major benefit would be that when guest tries to cancel a request, where the
request cannot be cancelled easily, (due to throttled BlockDriverState, a lost
connection, or a large request queue), we don't need to block the whole vm with
a busy loop, which is how bdrv_aio_cancel is implemented now.

A test case that is easy to reproduce is, throttle a scsi-disk to a very low
limit, for example 50 bps, then stress the guest block device with dd or fio.

Currently, the vm will quickly hang when it loses patience and send a tmf
command to cancel the request, at which point we will busy wait in
bdrv_aio_cancel, until the request is slowly spit out from throttled_reqs.

Later, we will change scsi device code to make this asynchronous, on top of
bdrv_aio_cancel_async.

Fam


Fam Zheng (21):
  ide/ahci: Check for -ECANCELED in aio callbacks
  block: Add refcnt in BlockDriverAIOCB
  block: Add bdrv_aio_cancel_async
  block: Drop bdrv_em_co_aiocb_info.cancel
  block: Convert bdrv_em_aiocb_info.cancel to .cancel_async
  thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async
  linux-aio: Convert laio_aiocb_info.cancel to .cancel_async
  dma: Convert dma_aiocb_info.cancel to .cancel_async
  iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async
  archipelago: Drop archipelago_aiocb_info.cancel
  blkdebug: Drop blkdebug_aiocb_info.cancel
  blkverify: Drop blkverify_aiocb_info.cancel
  curl: Drop curl_aiocb_info.cancel
  qed: Drop qed_aiocb_info.cancel
  quorum: Convert quorum_aiocb_info.cancel to .cancel_async
  rbd: Drop rbd_aiocb_info.cancel
  sheepdog: Convert sd_aiocb_info.cancel to .cancel_async
  win32-aio: Drop win32_aiocb_info.cancel
  ide: Convert trim_aiocb_info.cancel to .cancel_async
  block: Drop AIOCBInfo.cancel
  block: Rename qemu_aio_release -> qemu_aio_unref

Liu Yuan (1):
  quorum: fix quorum_aio_cancel()

 block.c                  | 69 ++++++++++++++++++++++++++++--------------------
 block/archipelago.c      | 19 ++-----------
 block/blkdebug.c         | 17 ++----------
 block/blkverify.c        | 21 +--------------
 block/curl.c             | 16 ++++-------
 block/iscsi.c            | 23 ++++------------
 block/linux-aio.c        | 34 +++++++-----------------
 block/qed.c              | 23 +---------------
 block/quorum.c           | 11 ++++----
 block/rbd.c              | 25 ++----------------
 block/sheepdog.c         | 54 ++++++++++++++++---------------------
 block/win32-aio.c        | 18 ++-----------
 dma-helpers.c            | 20 +++-----------
 hw/ide/ahci.c            |  3 +++
 hw/ide/core.c            | 26 ++++++++++++------
 include/block/aio.h      |  7 +++--
 include/block/block.h    |  1 +
 tests/test-thread-pool.c | 34 ++++++++++++++++++------
 thread-pool.c            | 36 +++++++++++--------------
 19 files changed, 172 insertions(+), 285 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v5 01/22] ide/ahci: Check for -ECANCELED in aio callbacks

[Fam Zheng]

Before, bdrv_aio_cancel will either complete the request (like normal)
and call CB with an actual return code, or skip calling the request (for
example when the IO req is not submitted by thread pool yet).

We will change bdrv_aio_cancel to do it differently: always call CB
before return, with either [1] a normal req completion ret code, or [2]
ret == -ECANCELED. So the callers' callback must accept both cases. The
existing logic works with case [1], but not [2].

The simplest transition of callback code is do nothing in case [2], just
as if the CB is not called by the bdrv_aio_cancel() call.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/ide/ahci.c |  3 +++
 hw/ide/core.c | 12 ++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 0ee713b..3067aa2 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -791,6 +791,9 @@ static void ncq_cb(void *opaque, int ret)
     NCQTransferState *ncq_tfs = (NCQTransferState *)opaque;
     IDEState *ide_state = &ncq_tfs->drive->port.ifs[0];
 
+    if (ret == -ECANCELED) {
+        return;
+    }
     /* Clear bit for this tag in SActive */
     ncq_tfs->drive->port_regs.scr_act &= ~(1 << ncq_tfs->tag);
 
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 191f893..2f2d3b4 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -568,6 +568,9 @@ static void ide_sector_read_cb(void *opaque, int ret)
     s->pio_aiocb = NULL;
     s->status &= ~BUSY_STAT;
 
+    if (ret == -ECANCELED) {
+        return;
+    }
     bdrv_acct_done(s->bs, &s->acct);
     if (ret != 0) {
         if (ide_handle_rw_error(s, -ret, IDE_RETRY_PIO |
@@ -677,6 +680,9 @@ void ide_dma_cb(void *opaque, int ret)
     int64_t sector_num;
     bool stay_active = false;
 
+    if (ret == -ECANCELED) {
+        return;
+    }
     if (ret < 0) {
         int op = IDE_RETRY_DMA;
 
@@ -802,6 +808,9 @@ static void ide_sector_write_cb(void *opaque, int ret)
     IDEState *s = opaque;
     int n;
 
+    if (ret == -ECANCELED) {
+        return;
+    }
     bdrv_acct_done(s->bs, &s->acct);
 
     s->pio_aiocb = NULL;
@@ -880,6 +889,9 @@ static void ide_flush_cb(void *opaque, int ret)
 
     s->pio_aiocb = NULL;
 
+    if (ret == -ECANCELED) {
+        return;
+    }
     if (ret < 0) {
         /* XXX: What sector number to set here? */
         if (ide_handle_rw_error(s, -ret, IDE_RETRY_FLUSH)) {
-- 
1.9.3

[Qemu-devel] [PATCH v5 02/22] block: Add refcnt in BlockDriverAIOCB

[Fam Zheng]

This will be useful in synchronous cancel emulation with
bdrv_aio_cancel_async.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c             | 12 +++++++++++-
 include/block/aio.h |  2 ++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index d06dd51..c1ee860 100644
--- a/block.c
+++ b/block.c
@@ -4885,13 +4885,23 @@ void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
     acb->bs = bs;
     acb->cb = cb;
     acb->opaque = opaque;
+    acb->refcnt = 1;
     return acb;
 }
 
+void qemu_aio_ref(void *p)
+{
+    BlockDriverAIOCB *acb = p;
+    acb->refcnt++;
+}
+
 void qemu_aio_release(void *p)
 {
     BlockDriverAIOCB *acb = p;
-    g_slice_free1(acb->aiocb_info->aiocb_size, acb);
+    assert(acb->refcnt > 0);
+    if (--acb->refcnt == 0) {
+        g_slice_free1(acb->aiocb_info->aiocb_size, acb);
+    }
 }
 
 /**************************************************************/
diff --git a/include/block/aio.h b/include/block/aio.h
index 4603c0f..2626fc7 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -35,11 +35,13 @@ struct BlockDriverAIOCB {
     BlockDriverState *bs;
     BlockDriverCompletionFunc *cb;
     void *opaque;
+    int refcnt;
 };
 
 void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
                    BlockDriverCompletionFunc *cb, void *opaque);
 void qemu_aio_release(void *p);
+void qemu_aio_ref(void *p);
 
 typedef struct AioHandler AioHandler;
 typedef void QEMUBHFunc(void *opaque);
-- 
1.9.3

[Qemu-devel] [PATCH v5 03/22] block: Add bdrv_aio_cancel_async

[Fam Zheng]

This is the async version of bdrv_aio_cancel, which doesn't block the
caller. It guarantees that the cb is called either before returning or
some time later.

bdrv_aio_cancel can base on bdrv_aio_cancel_async, later we can convert
all .io_cancel implementations to .io_cancel_async, and the aio_poll is
the common logic. In the end, .io_cancel can be dropped.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c               | 27 ++++++++++++++++++++++++++-
 include/block/aio.h   |  2 ++
 include/block/block.h |  1 +
 3 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index c1ee860..371082b 100644
--- a/block.c
+++ b/block.c
@@ -4634,7 +4634,32 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
 
 void bdrv_aio_cancel(BlockDriverAIOCB *acb)
 {
-    acb->aiocb_info->cancel(acb);
+    if (acb->aiocb_info->cancel) {
+        acb->aiocb_info->cancel(acb);
+    } else {
+        qemu_aio_ref(acb);
+        bdrv_aio_cancel_async(acb);
+        while (acb->refcnt > 1) {
+            if (acb->aiocb_info->get_aio_context) {
+                aio_poll(acb->aiocb_info->get_aio_context(acb), true);
+            } else if (acb->bs) {
+                aio_poll(bdrv_get_aio_context(acb->bs), true);
+            } else {
+                abort();
+            }
+        }
+        qemu_aio_release(acb);
+    }
+}
+
+/* Async version of aio cancel. The caller is not blocked if the acb implements
+ * cancel_async, otherwise we do nothing and let the request normally complete.
+ * In either case the completion callback must be called. */
+void bdrv_aio_cancel_async(BlockDriverAIOCB *acb)
+{
+    if (acb->aiocb_info->cancel_async) {
+        acb->aiocb_info->cancel_async(acb);
+    }
 }
 
 /**************************************************************/
diff --git a/include/block/aio.h b/include/block/aio.h
index 2626fc7..ad361e3 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -27,6 +27,8 @@ typedef void BlockDriverCompletionFunc(void *opaque, int ret);
 
 typedef struct AIOCBInfo {
     void (*cancel)(BlockDriverAIOCB *acb);
+    void (*cancel_async)(BlockDriverAIOCB *acb);
+    AioContext *(*get_aio_context)(BlockDriverAIOCB *acb);
     size_t aiocb_size;
 } AIOCBInfo;
 
diff --git a/include/block/block.h b/include/block/block.h
index 8f4ad16..35a2448 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -336,6 +336,7 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
                                    int64_t sector_num, int nb_sectors,
                                    BlockDriverCompletionFunc *cb, void *opaque);
 void bdrv_aio_cancel(BlockDriverAIOCB *acb);
+void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);
 
 typedef struct BlockRequest {
     /* Fields to be filled by multiwrite caller */
-- 
1.9.3

[Qemu-devel] [PATCH v5 04/22] block: Drop bdrv_em_co_aiocb_info.cancel

[Fam Zheng]

Also drop the now unused ->done pointer.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/block.c b/block.c
index 371082b..6193883 100644
--- a/block.c
+++ b/block.c
@@ -4757,22 +4757,8 @@ typedef struct BlockDriverAIOCBCoroutine {
     QEMUBH* bh;
 } BlockDriverAIOCBCoroutine;
 
-static void bdrv_aio_co_cancel_em(BlockDriverAIOCB *blockacb)
-{
-    AioContext *aio_context = bdrv_get_aio_context(blockacb->bs);
-    BlockDriverAIOCBCoroutine *acb =
-        container_of(blockacb, BlockDriverAIOCBCoroutine, common);
-    bool done = false;
-
-    acb->done = &done;
-    while (!done) {
-        aio_poll(aio_context, true);
-    }
-}
-
 static const AIOCBInfo bdrv_em_co_aiocb_info = {
     .aiocb_size         = sizeof(BlockDriverAIOCBCoroutine),
-    .cancel             = bdrv_aio_co_cancel_em,
 };
 
 static void bdrv_co_em_bh(void *opaque)
@@ -4781,10 +4767,6 @@ static void bdrv_co_em_bh(void *opaque)
 
     acb->common.cb(acb->common.opaque, acb->req.error);
 
-    if (acb->done) {
-        *acb->done = true;
-    }
-
     qemu_bh_delete(acb->bh);
     qemu_aio_release(acb);
 }
@@ -4825,7 +4807,6 @@ static BlockDriverAIOCB *bdrv_co_aio_rw_vector(BlockDriverState *bs,
     acb->req.qiov = qiov;
     acb->req.flags = flags;
     acb->is_write = is_write;
-    acb->done = NULL;
 
     co = qemu_coroutine_create(bdrv_co_do_rw);
     qemu_coroutine_enter(co, acb);
@@ -4852,7 +4833,6 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs,
     BlockDriverAIOCBCoroutine *acb;
 
     acb = qemu_aio_get(&bdrv_em_co_aiocb_info, bs, cb, opaque);
-    acb->done = NULL;
 
     co = qemu_coroutine_create(bdrv_aio_flush_co_entry);
     qemu_coroutine_enter(co, acb);
@@ -4882,7 +4862,6 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
     acb = qemu_aio_get(&bdrv_em_co_aiocb_info, bs, cb, opaque);
     acb->req.sector = sector_num;
     acb->req.nb_sectors = nb_sectors;
-    acb->done = NULL;
     co = qemu_coroutine_create(bdrv_aio_discard_co_entry);
     qemu_coroutine_enter(co, acb);
 
-- 
1.9.3

[Qemu-devel] [PATCH v5 05/22] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Fam Zheng]

All the difference is that the old .cancel doesn't call cb, but
.cancel_async does.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block.c b/block.c
index 6193883..f594d56 100644
--- a/block.c
+++ b/block.c
@@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
 {
     BlockDriverAIOCBSync *acb =
         container_of(blockacb, BlockDriverAIOCBSync, common);
+
+    acb->ret = -ECANCELED;
+    acb->common.cb(acb->common.opaque, acb->ret);
     qemu_bh_delete(acb->bh);
     acb->bh = NULL;
     qemu_aio_release(acb);
@@ -4686,7 +4689,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
 
 static const AIOCBInfo bdrv_em_aiocb_info = {
     .aiocb_size         = sizeof(BlockDriverAIOCBSync),
-    .cancel             = bdrv_aio_cancel_em,
+    .cancel_async       = bdrv_aio_cancel_em,
 };
 
 static void bdrv_aio_bh_cb(void *opaque)
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 05/22] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Paolo Bonzini]

Il 10/09/2014 07:59, Fam Zheng ha scritto:
> @@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
>  {
>      BlockDriverAIOCBSync *acb =
>          container_of(blockacb, BlockDriverAIOCBSync, common);
> +
> +    acb->ret = -ECANCELED;
> +    acb->common.cb(acb->common.opaque, acb->ret);
>      qemu_bh_delete(acb->bh);
>      acb->bh = NULL;
>      qemu_aio_release(acb);

This could call the callback before I/O is finished.  I/O can then
complete and write to disk stuff that was not meant to be written.

I think there is a pre-existing bug, which should be fixed with a "bool
*done" member similar to BlockDriverAIOCBCoroutine's.  But for the sake
of conversion to async cancellation, you can just empty bdrv_aio_cancel_em.

Paolo

Re: [Qemu-devel] [PATCH v5 05/22] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Fam Zheng]

On Wed, 09/10 10:20, Paolo Bonzini wrote:
> Il 10/09/2014 07:59, Fam Zheng ha scritto:
> > @@ -4679,6 +4679,9 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
> >  {
> >      BlockDriverAIOCBSync *acb =
> >          container_of(blockacb, BlockDriverAIOCBSync, common);
> > +
> > +    acb->ret = -ECANCELED;
> > +    acb->common.cb(acb->common.opaque, acb->ret);
> >      qemu_bh_delete(acb->bh);
> >      acb->bh = NULL;
> >      qemu_aio_release(acb);
> 
> This could call the callback before I/O is finished.  I/O can then
> complete and write to disk stuff that was not meant to be written.

I think the request is already completed when bdrv_aio_rw_vector returns this
blockacb. I shouldn't override the return code anyway, but perhaps a nop
bdrv_aio_cancel_em is better.

> 
> I think there is a pre-existing bug, which should be fixed with a "bool
> *done" member similar to BlockDriverAIOCBCoroutine's.  But for the sake
> of conversion to async cancellation, you can just empty bdrv_aio_cancel_em.
> 

BTW, why is it "bool *done" instead of just "bool done"?

Fam

Re: [Qemu-devel] [PATCH v5 05/22] block: Convert bdrv_em_aiocb_info.cancel to .cancel_async

[Paolo Bonzini]

Il 10/09/2014 11:36, Fam Zheng ha scritto:
>> > 
>> > This could call the callback before I/O is finished.  I/O can then
>> > complete and write to disk stuff that was not meant to be written.
> I think the request is already completed when bdrv_aio_rw_vector returns this
> blockacb. I shouldn't override the return code anyway, but perhaps a nop
> bdrv_aio_cancel_em is better.

Note that the legacy bdrv_read/bdrv_write function calls actually are
AIO-friendly (they run in a coroutine, and can yield).

> > I think there is a pre-existing bug, which should be fixed with a "bool
> > *done" member similar to BlockDriverAIOCBCoroutine's.  But for the sake
> > of conversion to async cancellation, you can just empty bdrv_aio_cancel_em.
> 
> BTW, why is it "bool *done" instead of just "bool done"?

Because, until your patches to add reference counting, this would have
caused a dangling pointer in bdrv_aio_co_cancel_em:

    acb->done = true;
    qemu_bh_delete(acb->bh);
    qemu_aio_release(acb);

instead, using "bool *done" works because bdrv_co_em_bh writes into the
variable of bdrv_aio_co_cancel_em.  This assumes that bdrv_aio_cancel is
only called once (no reentrancy).

Paolo

[Qemu-devel] [PATCH v5 06/22] thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async

[Fam Zheng]

The .cancel_async shares the same the first half with .cancel: try to
steal the request if not submitted yet. In this case set the elem to
THREAD_DONE status and ret to -ECANCELED, which means
thread_pool_completion_bh will call the cb with -ECANCELED.

If the request is already submitted, do nothing, as we know the normal
completion will happen in the future.

Testing code update:

Before, done_cb is only called if the request is already submitted by
thread pool. Now done_cb is always called, even before it is submitted,
because we emulate bdrv_aio_cancel with bdrv_aio_cancel_async. So also
update the test criteria accordingly.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/test-thread-pool.c | 34 ++++++++++++++++++++++++++--------
 thread-pool.c            | 32 ++++++++++++++------------------
 2 files changed, 40 insertions(+), 26 deletions(-)

diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index f40b7fc..ed2b25b 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -33,7 +33,7 @@ static int long_cb(void *opaque)
 static void done_cb(void *opaque, int ret)
 {
     WorkerTestData *data = opaque;
-    g_assert_cmpint(data->ret, ==, -EINPROGRESS);
+    g_assert(data->ret == -EINPROGRESS || data->ret == -ECANCELED);
     data->ret = ret;
     data->aiocb = NULL;
 
@@ -132,7 +132,7 @@ static void test_submit_many(void)
     }
 }
 
-static void test_cancel(void)
+static void do_test_cancel(bool sync)
 {
     WorkerTestData data[100];
     int num_canceled;
@@ -170,18 +170,25 @@ static void test_cancel(void)
     for (i = 0; i < 100; i++) {
         if (atomic_cmpxchg(&data[i].n, 0, 3) == 0) {
             data[i].ret = -ECANCELED;
-            bdrv_aio_cancel(data[i].aiocb);
-            active--;
+            if (sync) {
+                bdrv_aio_cancel(data[i].aiocb);
+            } else {
+                bdrv_aio_cancel_async(data[i].aiocb);
+            }
             num_canceled++;
         }
     }
     g_assert_cmpint(active, >, 0);
     g_assert_cmpint(num_canceled, <, 100);
 
-    /* Canceling the others will be a blocking operation.  */
     for (i = 0; i < 100; i++) {
         if (data[i].aiocb && data[i].n != 3) {
-            bdrv_aio_cancel(data[i].aiocb);
+            if (sync) {
+                /* Canceling the others will be a blocking operation.  */
+                bdrv_aio_cancel(data[i].aiocb);
+            } else {
+                bdrv_aio_cancel_async(data[i].aiocb);
+            }
         }
     }
 
@@ -193,15 +200,25 @@ static void test_cancel(void)
     for (i = 0; i < 100; i++) {
         if (data[i].n == 3) {
             g_assert_cmpint(data[i].ret, ==, -ECANCELED);
-            g_assert(data[i].aiocb != NULL);
+            g_assert(data[i].aiocb == NULL);
         } else {
             g_assert_cmpint(data[i].n, ==, 2);
-            g_assert_cmpint(data[i].ret, ==, 0);
+            g_assert(data[i].ret == 0 || data[i].ret == -ECANCELED);
             g_assert(data[i].aiocb == NULL);
         }
     }
 }
 
+static void test_cancel(void)
+{
+    do_test_cancel(true);
+}
+
+static void test_cancel_async(void)
+{
+    do_test_cancel(false);
+}
+
 int main(int argc, char **argv)
 {
     int ret;
@@ -217,6 +234,7 @@ int main(int argc, char **argv)
     g_test_add_func("/thread-pool/submit-co", test_submit_co);
     g_test_add_func("/thread-pool/submit-many", test_submit_many);
     g_test_add_func("/thread-pool/cancel", test_cancel);
+    g_test_add_func("/thread-pool/cancel-async", test_cancel_async);
 
     ret = g_test_run();
 
diff --git a/thread-pool.c b/thread-pool.c
index 23888dc..6afd343 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -32,7 +32,6 @@ enum ThreadState {
     THREAD_QUEUED,
     THREAD_ACTIVE,
     THREAD_DONE,
-    THREAD_CANCELED,
 };
 
 struct ThreadPoolElement {
@@ -59,7 +58,6 @@ struct ThreadPool {
     AioContext *ctx;
     QEMUBH *completion_bh;
     QemuMutex lock;
-    QemuCond check_cancel;
     QemuCond worker_stopped;
     QemuSemaphore sem;
     int max_threads;
@@ -74,7 +72,6 @@ struct ThreadPool {
     int idle_threads;
     int new_threads;     /* backlog of threads we need to create */
     int pending_threads; /* threads created but not running yet */
-    int pending_cancellations; /* whether we need a cond_broadcast */
     bool stopping;
 };
 
@@ -114,9 +111,6 @@ static void *worker_thread(void *opaque)
         req->state = THREAD_DONE;
 
         qemu_mutex_lock(&pool->lock);
-        if (pool->pending_cancellations) {
-            qemu_cond_broadcast(&pool->check_cancel);
-        }
 
         qemu_bh_schedule(pool->completion_bh);
     }
@@ -174,7 +168,7 @@ static void thread_pool_completion_bh(void *opaque)
 
 restart:
     QLIST_FOREACH_SAFE(elem, &pool->head, all, next) {
-        if (elem->state != THREAD_CANCELED && elem->state != THREAD_DONE) {
+        if (elem->state != THREAD_DONE) {
             continue;
         }
         if (elem->state == THREAD_DONE) {
@@ -218,22 +212,26 @@ static void thread_pool_cancel(BlockDriverAIOCB *acb)
          */
         qemu_sem_timedwait(&pool->sem, 0) == 0) {
         QTAILQ_REMOVE(&pool->request_list, elem, reqs);
-        elem->state = THREAD_CANCELED;
         qemu_bh_schedule(pool->completion_bh);
-    } else {
-        pool->pending_cancellations++;
-        while (elem->state != THREAD_CANCELED && elem->state != THREAD_DONE) {
-            qemu_cond_wait(&pool->check_cancel, &pool->lock);
-        }
-        pool->pending_cancellations--;
+
+        elem->state = THREAD_DONE;
+        elem->ret = -ECANCELED;
     }
+
     qemu_mutex_unlock(&pool->lock);
-    thread_pool_completion_bh(pool);
+}
+
+static AioContext *thread_pool_get_aio_context(BlockDriverAIOCB *acb)
+{
+    ThreadPoolElement *elem = (ThreadPoolElement *)acb;
+    ThreadPool *pool = elem->pool;
+    return pool->ctx;
 }
 
 static const AIOCBInfo thread_pool_aiocb_info = {
     .aiocb_size         = sizeof(ThreadPoolElement),
-    .cancel             = thread_pool_cancel,
+    .cancel_async       = thread_pool_cancel,
+    .get_aio_context    = thread_pool_get_aio_context,
 };
 
 BlockDriverAIOCB *thread_pool_submit_aio(ThreadPool *pool,
@@ -300,7 +298,6 @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
     pool->ctx = ctx;
     pool->completion_bh = aio_bh_new(ctx, thread_pool_completion_bh, pool);
     qemu_mutex_init(&pool->lock);
-    qemu_cond_init(&pool->check_cancel);
     qemu_cond_init(&pool->worker_stopped);
     qemu_sem_init(&pool->sem, 0);
     pool->max_threads = 64;
@@ -343,7 +340,6 @@ void thread_pool_free(ThreadPool *pool)
 
     qemu_bh_delete(pool->completion_bh);
     qemu_sem_destroy(&pool->sem);
-    qemu_cond_destroy(&pool->check_cancel);
     qemu_cond_destroy(&pool->worker_stopped);
     qemu_mutex_destroy(&pool->lock);
     g_free(pool);
-- 
1.9.3

[Qemu-devel] [PATCH v5 07/22] linux-aio: Convert laio_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Just call io_cancel (2), if it fails, it means the request is not
canceled, so the event loop will eventually call
qemu_laio_process_completion.

In qemu_laio_process_completion, change to call the cb unconditionally.
It is required by bdrv_aio_cancel_async.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/linux-aio.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/block/linux-aio.c b/block/linux-aio.c
index 9aca758..b67f56c 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -85,9 +85,8 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s,
                 ret = -EINVAL;
             }
         }
-
-        laiocb->common.cb(laiocb->common.opaque, ret);
     }
+    laiocb->common.cb(laiocb->common.opaque, ret);
 
     qemu_aio_release(laiocb);
 }
@@ -153,35 +152,22 @@ static void laio_cancel(BlockDriverAIOCB *blockacb)
     struct io_event event;
     int ret;
 
-    if (laiocb->ret != -EINPROGRESS)
+    if (laiocb->ret != -EINPROGRESS) {
         return;
-
-    /*
-     * Note that as of Linux 2.6.31 neither the block device code nor any
-     * filesystem implements cancellation of AIO request.
-     * Thus the polling loop below is the normal code path.
-     */
+    }
     ret = io_cancel(laiocb->ctx->ctx, &laiocb->iocb, &event);
-    if (ret == 0) {
-        laiocb->ret = -ECANCELED;
+    laiocb->ret = -ECANCELED;
+    if (ret != 0) {
+        /* iocb is not cancelled, cb will be called by the event loop later */
         return;
     }
 
-    /*
-     * We have to wait for the iocb to finish.
-     *
-     * The only way to get the iocb status update is by polling the io context.
-     * We might be able to do this slightly more optimal by removing the
-     * O_NONBLOCK flag.
-     */
-    while (laiocb->ret == -EINPROGRESS) {
-        qemu_laio_completion_cb(&laiocb->ctx->e);
-    }
+    laiocb->common.cb(laiocb->common.opaque, laiocb->ret);
 }
 
 static const AIOCBInfo laio_aiocb_info = {
     .aiocb_size         = sizeof(struct qemu_laiocb),
-    .cancel             = laio_cancel,
+    .cancel_async       = laio_cancel,
 };
 
 static void ioq_init(LaioQueue *io_q)
-- 
1.9.3

[Qemu-devel] [PATCH v5 08/22] dma: Convert dma_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Just forward the request to bdrv_aio_cancel_async.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 dma-helpers.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/dma-helpers.c b/dma-helpers.c
index 499b52b..5a46e5c 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -73,7 +73,6 @@ typedef struct {
     QEMUSGList *sg;
     uint64_t sector_num;
     DMADirection dir;
-    bool in_cancel;
     int sg_cur_index;
     dma_addr_t sg_cur_byte;
     QEMUIOVector iov;
@@ -125,12 +124,7 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
         qemu_bh_delete(dbs->bh);
         dbs->bh = NULL;
     }
-    if (!dbs->in_cancel) {
-        /* Requests may complete while dma_aio_cancel is in progress.  In
-         * this case, the AIOCB should not be released because it is still
-         * referenced by dma_aio_cancel.  */
-        qemu_aio_release(dbs);
-    }
+    qemu_aio_release(dbs);
 }
 
 static void dma_bdrv_cb(void *opaque, int ret)
@@ -186,19 +180,14 @@ static void dma_aio_cancel(BlockDriverAIOCB *acb)
     trace_dma_aio_cancel(dbs);
 
     if (dbs->acb) {
-        BlockDriverAIOCB *acb = dbs->acb;
-        dbs->acb = NULL;
-        dbs->in_cancel = true;
-        bdrv_aio_cancel(acb);
-        dbs->in_cancel = false;
+        bdrv_aio_cancel_async(dbs->acb);
     }
-    dbs->common.cb = NULL;
-    dma_complete(dbs, 0);
 }
 
+
 static const AIOCBInfo dma_aiocb_info = {
     .aiocb_size         = sizeof(DMAAIOCB),
-    .cancel             = dma_aio_cancel,
+    .cancel_async       = dma_aio_cancel,
 };
 
 BlockDriverAIOCB *dma_bdrv_io(
@@ -217,7 +206,6 @@ BlockDriverAIOCB *dma_bdrv_io(
     dbs->sg_cur_index = 0;
     dbs->sg_cur_byte = 0;
     dbs->dir = dir;
-    dbs->in_cancel = false;
     dbs->io_func = io_func;
     dbs->bh = NULL;
     qemu_iovec_init(&dbs->iov, sg->nsg);
-- 
1.9.3

[Qemu-devel] [PATCH v5 09/22] iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Also drop the unused field "canceled".

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3e19202..a0aca5f 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -88,7 +88,6 @@ typedef struct IscsiAIOCB {
     struct scsi_task *task;
     uint8_t *buf;
     int status;
-    int canceled;
     int64_t sector_num;
     int nb_sectors;
 #ifdef __linux__
@@ -120,9 +119,7 @@ iscsi_bh_cb(void *p)
     g_free(acb->buf);
     acb->buf = NULL;
 
-    if (acb->canceled == 0) {
-        acb->common.cb(acb->common.opaque, acb->status);
-    }
+    acb->common.cb(acb->common.opaque, acb->status);
 
     if (acb->task != NULL) {
         scsi_free_scsi_task(acb->task);
@@ -240,20 +237,15 @@ iscsi_aio_cancel(BlockDriverAIOCB *blockacb)
         return;
     }
 
-    acb->canceled = 1;
-
     /* send a task mgmt call to the target to cancel the task on the target */
     iscsi_task_mgmt_abort_task_async(iscsilun->iscsi, acb->task,
                                      iscsi_abort_task_cb, acb);
 
-    while (acb->status == -EINPROGRESS) {
-        aio_poll(iscsilun->aio_context, true);
-    }
 }
 
 static const AIOCBInfo iscsi_aiocb_info = {
     .aiocb_size         = sizeof(IscsiAIOCB),
-    .cancel             = iscsi_aio_cancel,
+    .cancel_async       = iscsi_aio_cancel,
 };
 
 
@@ -638,10 +630,6 @@ iscsi_aio_ioctl_cb(struct iscsi_context *iscsi, int status,
     g_free(acb->buf);
     acb->buf = NULL;
 
-    if (acb->canceled != 0) {
-        return;
-    }
-
     acb->status = 0;
     if (status < 0) {
         error_report("Failed to ioctl(SG_IO) to iSCSI lun. %s",
@@ -683,7 +671,6 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     acb = qemu_aio_get(&iscsi_aiocb_info, bs, cb, opaque);
 
     acb->iscsilun = iscsilun;
-    acb->canceled    = 0;
     acb->bh          = NULL;
     acb->status      = -EINPROGRESS;
     acb->buf         = NULL;
-- 
1.9.3

[Qemu-devel] [PATCH v5 10/22] archipelago: Drop archipelago_aiocb_info.cancel

[Fam Zheng]

The cancelled flag is no longer useful. Later the request will complete
as before, and cb will be called.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/archipelago.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/block/archipelago.c b/block/archipelago.c
index 22a7daa..1fa0462 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -92,7 +92,6 @@ typedef struct ArchipelagoAIOCB {
     struct BDRVArchipelagoState *s;
     QEMUIOVector *qiov;
     ARCHIPCmd cmd;
-    bool cancelled;
     int status;
     int64_t size;
     int64_t ret;
@@ -318,9 +317,7 @@ static void qemu_archipelago_complete_aio(void *opaque)
     aio_cb->common.cb(aio_cb->common.opaque, aio_cb->ret);
     aio_cb->status = 0;
 
-    if (!aio_cb->cancelled) {
-        qemu_aio_release(aio_cb);
-    }
+    qemu_aio_release(aio_cb);
     g_free(reqdata);
 }
 
@@ -724,19 +721,8 @@ static int qemu_archipelago_create(const char *filename,
     return ret;
 }
 
-static void qemu_archipelago_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    ArchipelagoAIOCB *aio_cb = (ArchipelagoAIOCB *) blockacb;
-    aio_cb->cancelled = true;
-    while (aio_cb->status == -EINPROGRESS) {
-        aio_poll(bdrv_get_aio_context(aio_cb->common.bs), true);
-    }
-    qemu_aio_release(aio_cb);
-}
-
 static const AIOCBInfo archipelago_aiocb_info = {
     .aiocb_size = sizeof(ArchipelagoAIOCB),
-    .cancel = qemu_archipelago_aio_cancel,
 };
 
 static int archipelago_submit_request(BDRVArchipelagoState *s,
@@ -888,7 +874,6 @@ static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs,
 
     aio_cb->ret = 0;
     aio_cb->s = s;
-    aio_cb->cancelled = false;
     aio_cb->status = -EINPROGRESS;
 
     off = sector_num * BDRV_SECTOR_SIZE;
-- 
1.9.3

[Qemu-devel] [PATCH v5 11/22] blkdebug: Drop blkdebug_aiocb_info.cancel

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/blkdebug.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 69b330e..08131b3 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -52,11 +52,8 @@ typedef struct BlkdebugSuspendedReq {
     QLIST_ENTRY(BlkdebugSuspendedReq) next;
 } BlkdebugSuspendedReq;
 
-static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb);
-
 static const AIOCBInfo blkdebug_aiocb_info = {
-    .aiocb_size = sizeof(BlkdebugAIOCB),
-    .cancel     = blkdebug_aio_cancel,
+    .aiocb_size    = sizeof(BlkdebugAIOCB),
 };
 
 enum {
@@ -450,16 +447,6 @@ static void error_callback_bh(void *opaque)
     qemu_aio_release(acb);
 }
 
-static void blkdebug_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    BlkdebugAIOCB *acb = container_of(blockacb, BlkdebugAIOCB, common);
-    if (acb->bh) {
-        qemu_bh_delete(acb->bh);
-        acb->bh = NULL;
-    }
-    qemu_aio_release(acb);
-}
-
 static BlockDriverAIOCB *inject_error(BlockDriverState *bs,
     BlockDriverCompletionFunc *cb, void *opaque, BlkdebugRule *rule)
 {
-- 
1.9.3

[Qemu-devel] [PATCH v5 12/22] blkverify: Drop blkverify_aiocb_info.cancel

[Fam Zheng]

Also the finished pointer is not used any more.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/blkverify.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/block/blkverify.c b/block/blkverify.c
index 163064c..460393f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -29,7 +29,6 @@ struct BlkverifyAIOCB {
 
     int ret;                    /* first completed request's result */
     unsigned int done;          /* completion counter */
-    bool *finished;             /* completion signal for cancel */
 
     QEMUIOVector *qiov;         /* user I/O vector */
     QEMUIOVector raw_qiov;      /* cloned I/O vector for raw file */
@@ -38,22 +37,8 @@ struct BlkverifyAIOCB {
     void (*verify)(BlkverifyAIOCB *acb);
 };
 
-static void blkverify_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    BlkverifyAIOCB *acb = (BlkverifyAIOCB *)blockacb;
-    AioContext *aio_context = bdrv_get_aio_context(blockacb->bs);
-    bool finished = false;
-
-    /* Wait until request completes, invokes its callback, and frees itself */
-    acb->finished = &finished;
-    while (!finished) {
-        aio_poll(aio_context, true);
-    }
-}
-
 static const AIOCBInfo blkverify_aiocb_info = {
     .aiocb_size         = sizeof(BlkverifyAIOCB),
-    .cancel             = blkverify_aio_cancel,
 };
 
 static void GCC_FMT_ATTR(2, 3) blkverify_err(BlkverifyAIOCB *acb,
@@ -194,7 +179,6 @@ static BlkverifyAIOCB *blkverify_aio_get(BlockDriverState *bs, bool is_write,
     acb->qiov = qiov;
     acb->buf = NULL;
     acb->verify = NULL;
-    acb->finished = NULL;
     return acb;
 }
 
@@ -208,9 +192,6 @@ static void blkverify_aio_bh(void *opaque)
         qemu_vfree(acb->buf);
     }
     acb->common.cb(acb->common.opaque, acb->ret);
-    if (acb->finished) {
-        *acb->finished = true;
-    }
     qemu_aio_release(acb);
 }
 
-- 
1.9.3

[Qemu-devel] [PATCH v5 13/22] curl: Drop curl_aiocb_info.cancel

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/curl.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 938f9d9..6f5d6ae 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -613,14 +613,8 @@ out_noclean:
     return -EINVAL;
 }
 
-static void curl_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    // Do we have to implement canceling? Seems to work without...
-}
-
 static const AIOCBInfo curl_aiocb_info = {
     .aiocb_size         = sizeof(CURLAIOCB),
-    .cancel             = curl_aio_cancel,
 };
 
 
-- 
1.9.3

[Qemu-devel] [PATCH v5 14/22] qed: Drop qed_aiocb_info.cancel

[Fam Zheng]

Also drop the now unused ->finished field.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/qed.c | 21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index ba395af..07cdb47 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -18,22 +18,8 @@
 #include "qapi/qmp/qerror.h"
 #include "migration/migration.h"
 
-static void qed_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    QEDAIOCB *acb = (QEDAIOCB *)blockacb;
-    AioContext *aio_context = bdrv_get_aio_context(blockacb->bs);
-    bool finished = false;
-
-    /* Wait for the request to finish */
-    acb->finished = &finished;
-    while (!finished) {
-        aio_poll(aio_context, true);
-    }
-}
-
 static const AIOCBInfo qed_aiocb_info = {
     .aiocb_size         = sizeof(QEDAIOCB),
-    .cancel             = qed_aio_cancel,
 };
 
 static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
@@ -918,18 +904,12 @@ static void qed_aio_complete_bh(void *opaque)
     BlockDriverCompletionFunc *cb = acb->common.cb;
     void *user_opaque = acb->common.opaque;
     int ret = acb->bh_ret;
-    bool *finished = acb->finished;
 
     qemu_bh_delete(acb->bh);
     qemu_aio_release(acb);
 
     /* Invoke callback */
     cb(user_opaque, ret);
-
-    /* Signal cancel completion */
-    if (finished) {
-        *finished = true;
-    }
 }
 
 static void qed_aio_complete(QEDAIOCB *acb, int ret)
@@ -1396,7 +1376,6 @@ static BlockDriverAIOCB *qed_aio_setup(BlockDriverState *bs,
                         opaque, flags);
 
     acb->flags = flags;
-    acb->finished = NULL;
     acb->qiov = qiov;
     acb->qiov_offset = 0;
     acb->cur_pos = (uint64_t)sector_num * BDRV_SECTOR_SIZE;
-- 
1.9.3

[Qemu-devel] [PATCH v5 15/22] quorum: fix quorum_aio_cancel()

[Fam Zheng]

From: Liu Yuan <namei.unix@gmail.com>

For a fifo read pattern, we only have one running aio (possible other cases that
has less number than num_children in the future), so we need to check if
.acb is NULL against bdrv_aio_cancel() to avoid segfault.

Cc: Eric Blake <eblake@redhat.com>
Cc: Benoit Canet <benoit@irqsave.net>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Liu Yuan <namei.unix@gmail.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/quorum.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/quorum.c b/block/quorum.c
index 093382e..41c4249 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -138,7 +138,9 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
 
     /* cancel all callbacks */
     for (i = 0; i < s->num_children; i++) {
-        bdrv_aio_cancel(acb->qcrs[i].aiocb);
+        if (acb->qcrs[i].aiocb) {
+            bdrv_aio_cancel(acb->qcrs[i].aiocb);
+        }
     }
 
     g_free(acb->qcrs);
-- 
1.9.3

[Qemu-devel] [PATCH v5 16/22] quorum: Convert quorum_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Before, we cancel all the child requests with bdrv_aio_cancel, then free
the acb..

Now we just kick off asynchronous cancellation of child requests and
return, we know quorum_aio_cb will be called later, so in the end
quorum_aio_finalize will take care of calling the caller's cb.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/quorum.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index 41c4249..f343c04 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -139,17 +139,14 @@ static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
     /* cancel all callbacks */
     for (i = 0; i < s->num_children; i++) {
         if (acb->qcrs[i].aiocb) {
-            bdrv_aio_cancel(acb->qcrs[i].aiocb);
+            bdrv_aio_cancel_async(acb->qcrs[i].aiocb);
         }
     }
-
-    g_free(acb->qcrs);
-    qemu_aio_release(acb);
 }
 
 static AIOCBInfo quorum_aiocb_info = {
     .aiocb_size         = sizeof(QuorumAIOCB),
-    .cancel             = quorum_aio_cancel,
+    .cancel_async       = quorum_aio_cancel,
 };
 
 static void quorum_aio_finalize(QuorumAIOCB *acb)
-- 
1.9.3

[Qemu-devel] [PATCH v5 17/22] rbd: Drop rbd_aiocb_info.cancel

[Fam Zheng]

And also drop the now unused "cancelled" field.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/rbd.c | 23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index ea969e7..349d465 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -77,7 +77,6 @@ typedef struct RBDAIOCB {
     int64_t sector_num;
     int error;
     struct BDRVRBDState *s;
-    int cancelled;
     int status;
 } RBDAIOCB;
 
@@ -407,9 +406,7 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
     acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret));
     acb->status = 0;
 
-    if (!acb->cancelled) {
-        qemu_aio_release(acb);
-    }
+    qemu_aio_release(acb);
 }
 
 /* TODO Convert to fine grained options */
@@ -538,25 +535,8 @@ static void qemu_rbd_close(BlockDriverState *bs)
     rados_shutdown(s->cluster);
 }
 
-/*
- * Cancel aio. Since we don't reference acb in a non qemu threads,
- * it is safe to access it here.
- */
-static void qemu_rbd_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    RBDAIOCB *acb = (RBDAIOCB *) blockacb;
-    acb->cancelled = 1;
-
-    while (acb->status == -EINPROGRESS) {
-        aio_poll(bdrv_get_aio_context(acb->common.bs), true);
-    }
-
-    qemu_aio_release(acb);
-}
-
 static const AIOCBInfo rbd_aiocb_info = {
     .aiocb_size = sizeof(RBDAIOCB),
-    .cancel = qemu_rbd_aio_cancel,
 };
 
 static void rbd_finish_bh(void *opaque)
@@ -639,7 +619,6 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
     acb->ret = 0;
     acb->error = 0;
     acb->s = s;
-    acb->cancelled = 0;
     acb->bh = NULL;
     acb->status = -EINPROGRESS;
 
-- 
1.9.3

[Qemu-devel] [PATCH v5 18/22] sheepdog: Convert sd_aiocb_info.cancel to .cancel_async

[Fam Zheng]

Also drop the now unused SheepdogAIOCB.finished field. Note that this
aio is internal to sheepdog driver and has NULL cb and opaque, and
should be unused at all.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/sheepdog.c | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index f91afc3..a04ac4e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -315,7 +315,6 @@ struct SheepdogAIOCB {
     void (*aio_done_func)(SheepdogAIOCB *);
 
     bool cancelable;
-    bool *finished;
     int nr_pending;
 };
 
@@ -446,9 +445,6 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
 static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
 {
     qemu_coroutine_enter(acb->coroutine, NULL);
-    if (acb->finished) {
-        *acb->finished = true;
-    }
     qemu_aio_release(acb);
 }
 
@@ -482,36 +478,33 @@ static void sd_aio_cancel(BlockDriverAIOCB *blockacb)
     SheepdogAIOCB *acb = (SheepdogAIOCB *)blockacb;
     BDRVSheepdogState *s = acb->common.bs->opaque;
     AIOReq *aioreq, *next;
-    bool finished = false;
-
-    acb->finished = &finished;
-    while (!finished) {
-        if (sd_acb_cancelable(acb)) {
-            /* Remove outstanding requests from pending and failed queues.  */
-            QLIST_FOREACH_SAFE(aioreq, &s->pending_aio_head, aio_siblings,
-                               next) {
-                if (aioreq->aiocb == acb) {
-                    free_aio_req(s, aioreq);
-                }
+
+    if (sd_acb_cancelable(acb)) {
+        /* Remove outstanding requests from pending and failed queues.  */
+        QLIST_FOREACH_SAFE(aioreq, &s->pending_aio_head, aio_siblings,
+                           next) {
+            if (aioreq->aiocb == acb) {
+                free_aio_req(s, aioreq);
             }
-            QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings,
-                               next) {
-                if (aioreq->aiocb == acb) {
-                    free_aio_req(s, aioreq);
-                }
+        }
+        QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings,
+                           next) {
+            if (aioreq->aiocb == acb) {
+                free_aio_req(s, aioreq);
             }
+        }
 
-            assert(acb->nr_pending == 0);
-            sd_finish_aiocb(acb);
-            return;
+        assert(acb->nr_pending == 0);
+        if (acb->common.cb) {
+            acb->common.cb(acb->common.opaque, -ECANCELED);
         }
-        aio_poll(s->aio_context, true);
+        sd_finish_aiocb(acb);
     }
 }
 
 static const AIOCBInfo sd_aiocb_info = {
-    .aiocb_size = sizeof(SheepdogAIOCB),
-    .cancel = sd_aio_cancel,
+    .aiocb_size     = sizeof(SheepdogAIOCB),
+    .cancel_async   = sd_aio_cancel,
 };
 
 static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
@@ -528,7 +521,6 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
 
     acb->aio_done_func = NULL;
     acb->cancelable = true;
-    acb->finished = NULL;
     acb->coroutine = qemu_coroutine_self();
     acb->ret = 0;
     acb->nr_pending = 0;
-- 
1.9.3

[Qemu-devel] [PATCH v5 19/22] win32-aio: Drop win32_aiocb_info.cancel

[Fam Zheng]

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/win32-aio.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/block/win32-aio.c b/block/win32-aio.c
index 5030e32..eed86f7 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -106,22 +106,8 @@ static void win32_aio_completion_cb(EventNotifier *e)
     }
 }
 
-static void win32_aio_cancel(BlockDriverAIOCB *blockacb)
-{
-    QEMUWin32AIOCB *waiocb = (QEMUWin32AIOCB *)blockacb;
-
-    /*
-     * CancelIoEx is only supported in Vista and newer.  For now, just
-     * wait for completion.
-     */
-    while (!HasOverlappedIoCompleted(&waiocb->ov)) {
-        aio_poll(bdrv_get_aio_context(blockacb->bs), true);
-    }
-}
-
 static const AIOCBInfo win32_aiocb_info = {
     .aiocb_size         = sizeof(QEMUWin32AIOCB),
-    .cancel             = win32_aio_cancel,
 };
 
 BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
-- 
1.9.3

[Qemu-devel] [PATCH v5 20/22] ide: Convert trim_aiocb_info.cancel to .cancel_async

[Fam Zheng]

We know that either bh is scheduled or ide_issue_trim_cb will be called
again, so we just set i, j and ret to the right values. In both cases,
ide_trim_bh_cb will be called.

Also forward the cancellation to the iocb->aiocb which we get from
bdrv_aio_discard.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/ide/core.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 2f2d3b4..ee30309 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -372,23 +372,21 @@ static void trim_aio_cancel(BlockDriverAIOCB *acb)
 {
     TrimAIOCB *iocb = container_of(acb, TrimAIOCB, common);
 
-    /* Exit the loop in case bdrv_aio_cancel calls ide_issue_trim_cb again.  */
+    /* Exit the loop so ide_issue_trim_cb will not continue  */
     iocb->j = iocb->qiov->niov - 1;
     iocb->i = (iocb->qiov->iov[iocb->j].iov_len / 8) - 1;
 
-    /* Tell ide_issue_trim_cb not to trigger the completion, too.  */
-    qemu_bh_delete(iocb->bh);
-    iocb->bh = NULL;
+    iocb->ret = -ECANCELED;
 
     if (iocb->aiocb) {
-        bdrv_aio_cancel(iocb->aiocb);
+        bdrv_aio_cancel_async(iocb->aiocb);
+        iocb->aiocb = NULL;
     }
-    qemu_aio_release(iocb);
 }
 
 static const AIOCBInfo trim_aiocb_info = {
     .aiocb_size         = sizeof(TrimAIOCB),
-    .cancel             = trim_aio_cancel,
+    .cancel_async       = trim_aio_cancel,
 };
 
 static void ide_trim_bh_cb(void *opaque)
-- 
1.9.3

[Qemu-devel] [PATCH v5 21/22] block: Drop AIOCBInfo.cancel

[Fam Zheng]

Now that all the implementations are converted to asynchronous version
and we can emulate synchronous cancellation with it. Let's drop the
unused member.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c             | 24 ++++++++++--------------
 include/block/aio.h |  1 -
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index f594d56..2e542a1 100644
--- a/block.c
+++ b/block.c
@@ -4634,22 +4634,18 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
 
 void bdrv_aio_cancel(BlockDriverAIOCB *acb)
 {
-    if (acb->aiocb_info->cancel) {
-        acb->aiocb_info->cancel(acb);
-    } else {
-        qemu_aio_ref(acb);
-        bdrv_aio_cancel_async(acb);
-        while (acb->refcnt > 1) {
-            if (acb->aiocb_info->get_aio_context) {
-                aio_poll(acb->aiocb_info->get_aio_context(acb), true);
-            } else if (acb->bs) {
-                aio_poll(bdrv_get_aio_context(acb->bs), true);
-            } else {
-                abort();
-            }
+    qemu_aio_ref(acb);
+    bdrv_aio_cancel_async(acb);
+    while (acb->refcnt > 1) {
+        if (acb->aiocb_info->get_aio_context) {
+            aio_poll(acb->aiocb_info->get_aio_context(acb), true);
+        } else if (acb->bs) {
+            aio_poll(bdrv_get_aio_context(acb->bs), true);
+        } else {
+            abort();
         }
-        qemu_aio_release(acb);
     }
+    qemu_aio_release(acb);
 }
 
 /* Async version of aio cancel. The caller is not blocked if the acb implements
diff --git a/include/block/aio.h b/include/block/aio.h
index ad361e3..f2d0582 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -26,7 +26,6 @@ typedef struct BlockDriverAIOCB BlockDriverAIOCB;
 typedef void BlockDriverCompletionFunc(void *opaque, int ret);
 
 typedef struct AIOCBInfo {
-    void (*cancel)(BlockDriverAIOCB *acb);
     void (*cancel_async)(BlockDriverAIOCB *acb);
     AioContext *(*get_aio_context)(BlockDriverAIOCB *acb);
     size_t aiocb_size;
-- 
1.9.3

[Qemu-devel] [PATCH v5 22/22] block: Rename qemu_aio_release -> qemu_aio_unref

[Fam Zheng]

Suggested-by: Benoît Canet <benoit.canet@irqsave.net>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block.c             | 10 +++++-----
 block/archipelago.c |  4 ++--
 block/blkdebug.c    |  2 +-
 block/blkverify.c   |  2 +-
 block/curl.c        | 10 +++++-----
 block/iscsi.c       |  6 +++---
 block/linux-aio.c   |  4 ++--
 block/qed.c         |  2 +-
 block/quorum.c      |  2 +-
 block/rbd.c         |  4 ++--
 block/sheepdog.c    |  8 ++++----
 block/win32-aio.c   |  4 ++--
 dma-helpers.c       |  2 +-
 hw/ide/core.c       |  2 +-
 include/block/aio.h |  2 +-
 thread-pool.c       |  4 ++--
 16 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/block.c b/block.c
index 2e542a1..fdf6638 100644
--- a/block.c
+++ b/block.c
@@ -4645,7 +4645,7 @@ void bdrv_aio_cancel(BlockDriverAIOCB *acb)
             abort();
         }
     }
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 /* Async version of aio cancel. The caller is not blocked if the acb implements
@@ -4680,7 +4680,7 @@ static void bdrv_aio_cancel_em(BlockDriverAIOCB *blockacb)
     acb->common.cb(acb->common.opaque, acb->ret);
     qemu_bh_delete(acb->bh);
     acb->bh = NULL;
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 static const AIOCBInfo bdrv_em_aiocb_info = {
@@ -4699,7 +4699,7 @@ static void bdrv_aio_bh_cb(void *opaque)
     acb->common.cb(acb->common.opaque, acb->ret);
     qemu_bh_delete(acb->bh);
     acb->bh = NULL;
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 static BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
@@ -4767,7 +4767,7 @@ static void bdrv_co_em_bh(void *opaque)
     acb->common.cb(acb->common.opaque, acb->req.error);
 
     qemu_bh_delete(acb->bh);
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 /* Invoke bdrv_co_do_readv/bdrv_co_do_writev */
@@ -4898,7 +4898,7 @@ void qemu_aio_ref(void *p)
     acb->refcnt++;
 }
 
-void qemu_aio_release(void *p)
+void qemu_aio_unref(void *p)
 {
     BlockDriverAIOCB *acb = p;
     assert(acb->refcnt > 0);
diff --git a/block/archipelago.c b/block/archipelago.c
index 1fa0462..731ebf1 100644
--- a/block/archipelago.c
+++ b/block/archipelago.c
@@ -317,7 +317,7 @@ static void qemu_archipelago_complete_aio(void *opaque)
     aio_cb->common.cb(aio_cb->common.opaque, aio_cb->ret);
     aio_cb->status = 0;
 
-    qemu_aio_release(aio_cb);
+    qemu_aio_unref(aio_cb);
     g_free(reqdata);
 }
 
@@ -889,7 +889,7 @@ static BlockDriverAIOCB *qemu_archipelago_aio_rw(BlockDriverState *bs,
 
 err_exit:
     error_report("qemu_archipelago_aio_rw(): I/O Error\n");
-    qemu_aio_release(aio_cb);
+    qemu_aio_unref(aio_cb);
     return NULL;
 }
 
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 08131b3..ced0b60 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -444,7 +444,7 @@ static void error_callback_bh(void *opaque)
     struct BlkdebugAIOCB *acb = opaque;
     qemu_bh_delete(acb->bh);
     acb->common.cb(acb->common.opaque, acb->ret);
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 static BlockDriverAIOCB *inject_error(BlockDriverState *bs,
diff --git a/block/blkverify.c b/block/blkverify.c
index 460393f..7d64a23 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -192,7 +192,7 @@ static void blkverify_aio_bh(void *opaque)
         qemu_vfree(acb->buf);
     }
     acb->common.cb(acb->common.opaque, acb->ret);
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 static void blkverify_aio_cb(void *opaque, int ret)
diff --git a/block/curl.c b/block/curl.c
index 6f5d6ae..225407c 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -212,7 +212,7 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
             qemu_iovec_from_buf(acb->qiov, 0, s->orig_buf + acb->start,
                                 acb->end - acb->start);
             acb->common.cb(acb->common.opaque, 0);
-            qemu_aio_release(acb);
+            qemu_aio_unref(acb);
             s->acb[i] = NULL;
         }
     }
@@ -304,7 +304,7 @@ static void curl_multi_check_completion(BDRVCURLState *s)
                     }
 
                     acb->common.cb(acb->common.opaque, -EIO);
-                    qemu_aio_release(acb);
+                    qemu_aio_unref(acb);
                     state->acb[i] = NULL;
                 }
             }
@@ -636,7 +636,7 @@ static void curl_readv_bh_cb(void *p)
     // we can just call the callback and be done.
     switch (curl_find_buf(s, start, acb->nb_sectors * SECTOR_SIZE, acb)) {
         case FIND_RET_OK:
-            qemu_aio_release(acb);
+            qemu_aio_unref(acb);
             // fall through
         case FIND_RET_WAIT:
             return;
@@ -648,7 +648,7 @@ static void curl_readv_bh_cb(void *p)
     state = curl_init_state(acb->common.bs, s);
     if (!state) {
         acb->common.cb(acb->common.opaque, -EIO);
-        qemu_aio_release(acb);
+        qemu_aio_unref(acb);
         return;
     }
 
@@ -664,7 +664,7 @@ static void curl_readv_bh_cb(void *p)
     if (state->buf_len && state->orig_buf == NULL) {
         curl_clean_state(state);
         acb->common.cb(acb->common.opaque, -ENOMEM);
-        qemu_aio_release(acb);
+        qemu_aio_unref(acb);
         return;
     }
     state->acb[0] = acb;
diff --git a/block/iscsi.c b/block/iscsi.c
index a0aca5f..392c38a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -126,7 +126,7 @@ iscsi_bh_cb(void *p)
         acb->task = NULL;
     }
 
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 static void
@@ -680,7 +680,7 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
     if (acb->task == NULL) {
         error_report("iSCSI: Failed to allocate task for scsi command. %s",
                      iscsi_get_error(iscsi));
-        qemu_aio_release(acb);
+        qemu_aio_unref(acb);
         return NULL;
     }
     memset(acb->task, 0, sizeof(struct scsi_task));
@@ -718,7 +718,7 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
                                  (data.size > 0) ? &data : NULL,
                                  acb) != 0) {
         scsi_free_scsi_task(acb->task);
-        qemu_aio_release(acb);
+        qemu_aio_unref(acb);
         return NULL;
     }
 
diff --git a/block/linux-aio.c b/block/linux-aio.c
index b67f56c..f4baba2 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -88,7 +88,7 @@ static void qemu_laio_process_completion(struct qemu_laio_state *s,
     }
     laiocb->common.cb(laiocb->common.opaque, ret);
 
-    qemu_aio_release(laiocb);
+    qemu_aio_unref(laiocb);
 }
 
 /* The completion BH fetches completed I/O requests and invokes their
@@ -286,7 +286,7 @@ BlockDriverAIOCB *laio_submit(BlockDriverState *bs, void *aio_ctx, int fd,
     return &laiocb->common;
 
 out_free_aiocb:
-    qemu_aio_release(laiocb);
+    qemu_aio_unref(laiocb);
     return NULL;
 }
 
diff --git a/block/qed.c b/block/qed.c
index 07cdb47..e6ae142 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -906,7 +906,7 @@ static void qed_aio_complete_bh(void *opaque)
     int ret = acb->bh_ret;
 
     qemu_bh_delete(acb->bh);
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 
     /* Invoke callback */
     cb(user_opaque, ret);
diff --git a/block/quorum.c b/block/quorum.c
index f343c04..7687466 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -168,7 +168,7 @@ static void quorum_aio_finalize(QuorumAIOCB *acb)
     }
 
     g_free(acb->qcrs);
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 static bool quorum_sha256_compare(QuorumVoteValue *a, QuorumVoteValue *b)
diff --git a/block/rbd.c b/block/rbd.c
index 349d465..67d2004 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -406,7 +406,7 @@ static void qemu_rbd_complete_aio(RADOSCB *rcb)
     acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret));
     acb->status = 0;
 
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 /* TODO Convert to fine grained options */
@@ -670,7 +670,7 @@ failed_completion:
 failed:
     g_free(rcb);
     qemu_vfree(acb->bounce);
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
     return NULL;
 }
 
diff --git a/block/sheepdog.c b/block/sheepdog.c
index a04ac4e..02287e4 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -445,7 +445,7 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
 static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
 {
     qemu_coroutine_enter(acb->coroutine, NULL);
-    qemu_aio_release(acb);
+    qemu_aio_unref(acb);
 }
 
 /*
@@ -2129,7 +2129,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
 
     ret = sd_co_rw_vector(acb);
     if (ret <= 0) {
-        qemu_aio_release(acb);
+        qemu_aio_unref(acb);
         return ret;
     }
 
@@ -2150,7 +2150,7 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
 
     ret = sd_co_rw_vector(acb);
     if (ret <= 0) {
-        qemu_aio_release(acb);
+        qemu_aio_unref(acb);
         return ret;
     }
 
@@ -2509,7 +2509,7 @@ static coroutine_fn int sd_co_discard(BlockDriverState *bs, int64_t sector_num,
 
     ret = sd_co_rw_vector(acb);
     if (ret <= 0) {
-        qemu_aio_release(acb);
+        qemu_aio_unref(acb);
         return ret;
     }
 
diff --git a/block/win32-aio.c b/block/win32-aio.c
index eed86f7..9323c1a 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -88,7 +88,7 @@ static void win32_aio_process_completion(QEMUWin32AIOState *s,
 
 
     waiocb->common.cb(waiocb->common.opaque, ret);
-    qemu_aio_release(waiocb);
+    qemu_aio_unref(waiocb);
 }
 
 static void win32_aio_completion_cb(EventNotifier *e)
@@ -158,7 +158,7 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
 out_dec_count:
     aio->count--;
 out:
-    qemu_aio_release(waiocb);
+    qemu_aio_unref(waiocb);
     return NULL;
 }
 
diff --git a/dma-helpers.c b/dma-helpers.c
index 5a46e5c..165471f 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -124,7 +124,7 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
         qemu_bh_delete(dbs->bh);
         dbs->bh = NULL;
     }
-    qemu_aio_release(dbs);
+    qemu_aio_unref(dbs);
 }
 
 static void dma_bdrv_cb(void *opaque, int ret)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index ee30309..4f2229d 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -397,7 +397,7 @@ static void ide_trim_bh_cb(void *opaque)
 
     qemu_bh_delete(iocb->bh);
     iocb->bh = NULL;
-    qemu_aio_release(iocb);
+    qemu_aio_unref(iocb);
 }
 
 static void ide_issue_trim_cb(void *opaque, int ret)
diff --git a/include/block/aio.h b/include/block/aio.h
index f2d0582..67a75dd 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -41,7 +41,7 @@ struct BlockDriverAIOCB {
 
 void *qemu_aio_get(const AIOCBInfo *aiocb_info, BlockDriverState *bs,
                    BlockDriverCompletionFunc *cb, void *opaque);
-void qemu_aio_release(void *p);
+void qemu_aio_unref(void *p);
 void qemu_aio_ref(void *p);
 
 typedef struct AioHandler AioHandler;
diff --git a/thread-pool.c b/thread-pool.c
index 6afd343..4b39261 100644
--- a/thread-pool.c
+++ b/thread-pool.c
@@ -186,12 +186,12 @@ restart:
             qemu_bh_schedule(pool->completion_bh);
 
             elem->common.cb(elem->common.opaque, elem->ret);
-            qemu_aio_release(elem);
+            qemu_aio_unref(elem);
             goto restart;
         } else {
             /* remove the request */
             QLIST_REMOVE(elem, all);
-            qemu_aio_release(elem);
+            qemu_aio_unref(elem);
         }
     }
 }
-- 
1.9.3

Re: [Qemu-devel] [PATCH v5 00/22] block: Asynchronous request cancellation

[Bin Wu]

On 2014/9/10 13:59, Fam Zheng wrote:
> v5: Fix IDE callback. (Paolo)
>      Fix blkdebug. (Paolo)
>      Drop the DMA fix which is independent of this series. (Paolo)
>      Incorperate Yuan's patch on quorum_aio_cancel. (Benoît)
>      Commit message wording fix. (Benoît)
>      Rename qemu_aio_release to qemu_aio_unref. (Benoît)
>
> v4: Drop AIOCBInfo.cancel.
>
> This series adds a new block layer API:
>
>    void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);
>
> And use it to emulate bdrv_aio_cancel.
>
> The function is similar to bdrv_aio_cancel in that it cancels an AIO request,
> but different that it doesn't block until the request is completely cancelled
> or done.
>
> More importantly, the completion callback, BlockDriverAIOCB.cb, is guaranteed
> to be called, so that the cb can take care of resource releasing and status
> reporting to guest, etc.
>
> In the following work, scsi emulation code will be shifted to use the async
> cancelling.
>
> One major benefit would be that when guest tries to cancel a request, where the
> request cannot be cancelled easily, (due to throttled BlockDriverState, a lost
> connection, or a large request queue), we don't need to block the whole vm with
> a busy loop, which is how bdrv_aio_cancel is implemented now.

First, I think this series is really useful. However, I tested the v4 
series and found virtio-scsi disk(scsi-hd) was still blocked when the IO 
could not come back because of 
virtio_scsi_do_tmf->scsi_cancel_io->bdrv_aio_cancel. can we just change 
the bdrv_aio_cancel to bdrv_aio_cancel_async to solve this problem?

>
> A test case that is easy to reproduce is, throttle a scsi-disk to a very low
> limit, for example 50 bps, then stress the guest block device with dd or fio.
>
> Currently, the vm will quickly hang when it loses patience and send a tmf
> command to cancel the request, at which point we will busy wait in
> bdrv_aio_cancel, until the request is slowly spit out from throttled_reqs.
>
> Later, we will change scsi device code to make this asynchronous, on top of
> bdrv_aio_cancel_async.
>
> Fam
>
>
> Fam Zheng (21):
>    ide/ahci: Check for -ECANCELED in aio callbacks
>    block: Add refcnt in BlockDriverAIOCB
>    block: Add bdrv_aio_cancel_async
>    block: Drop bdrv_em_co_aiocb_info.cancel
>    block: Convert bdrv_em_aiocb_info.cancel to .cancel_async
>    thread-pool: Convert thread_pool_aiocb_info.cancel to cancel_async
>    linux-aio: Convert laio_aiocb_info.cancel to .cancel_async
>    dma: Convert dma_aiocb_info.cancel to .cancel_async
>    iscsi: Convert iscsi_aiocb_info.cancel to .cancel_async
>    archipelago: Drop archipelago_aiocb_info.cancel
>    blkdebug: Drop blkdebug_aiocb_info.cancel
>    blkverify: Drop blkverify_aiocb_info.cancel
>    curl: Drop curl_aiocb_info.cancel
>    qed: Drop qed_aiocb_info.cancel
>    quorum: Convert quorum_aiocb_info.cancel to .cancel_async
>    rbd: Drop rbd_aiocb_info.cancel
>    sheepdog: Convert sd_aiocb_info.cancel to .cancel_async
>    win32-aio: Drop win32_aiocb_info.cancel
>    ide: Convert trim_aiocb_info.cancel to .cancel_async
>    block: Drop AIOCBInfo.cancel
>    block: Rename qemu_aio_release -> qemu_aio_unref
>
> Liu Yuan (1):
>    quorum: fix quorum_aio_cancel()
>
>   block.c                  | 69 ++++++++++++++++++++++++++++--------------------
>   block/archipelago.c      | 19 ++-----------
>   block/blkdebug.c         | 17 ++----------
>   block/blkverify.c        | 21 +--------------
>   block/curl.c             | 16 ++++-------
>   block/iscsi.c            | 23 ++++------------
>   block/linux-aio.c        | 34 +++++++-----------------
>   block/qed.c              | 23 +---------------
>   block/quorum.c           | 11 ++++----
>   block/rbd.c              | 25 ++----------------
>   block/sheepdog.c         | 54 ++++++++++++++++---------------------
>   block/win32-aio.c        | 18 ++-----------
>   dma-helpers.c            | 20 +++-----------
>   hw/ide/ahci.c            |  3 +++
>   hw/ide/core.c            | 26 ++++++++++++------
>   include/block/aio.h      |  7 +++--
>   include/block/block.h    |  1 +
>   tests/test-thread-pool.c | 34 ++++++++++++++++++------
>   thread-pool.c            | 36 +++++++++++--------------
>   19 files changed, 172 insertions(+), 285 deletions(-)
>

Re: [Qemu-devel] [PATCH v5 00/22] block: Asynchronous request cancellation

[Fam Zheng]

On Wed, 09/10 17:09, Bin Wu wrote:
> On 2014/9/10 13:59, Fam Zheng wrote:
> >v5: Fix IDE callback. (Paolo)
> >     Fix blkdebug. (Paolo)
> >     Drop the DMA fix which is independent of this series. (Paolo)
> >     Incorperate Yuan's patch on quorum_aio_cancel. (Benoît)
> >     Commit message wording fix. (Benoît)
> >     Rename qemu_aio_release to qemu_aio_unref. (Benoît)
> >
> >v4: Drop AIOCBInfo.cancel.
> >
> >This series adds a new block layer API:
> >
> >   void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);
> >
> >And use it to emulate bdrv_aio_cancel.
> >
> >The function is similar to bdrv_aio_cancel in that it cancels an AIO request,
> >but different that it doesn't block until the request is completely cancelled
> >or done.
> >
> >More importantly, the completion callback, BlockDriverAIOCB.cb, is guaranteed
> >to be called, so that the cb can take care of resource releasing and status
> >reporting to guest, etc.
> >
> >In the following work, scsi emulation code will be shifted to use the async
> >cancelling.
> >
> >One major benefit would be that when guest tries to cancel a request, where the
> >request cannot be cancelled easily, (due to throttled BlockDriverState, a lost
> >connection, or a large request queue), we don't need to block the whole vm with
> >a busy loop, which is how bdrv_aio_cancel is implemented now.
> 
> First, I think this series is really useful. However, I tested the v4 series
> and found virtio-scsi disk(scsi-hd) was still blocked when the IO could not
> come back because of virtio_scsi_do_tmf->scsi_cancel_io->bdrv_aio_cancel.
> can we just change the bdrv_aio_cancel to bdrv_aio_cancel_async to solve
> this problem?

We can't just change it yet. We need to take care of the scsi_req_unref there,
if we unref right after bdrv_aio_cancel_async, in the cb it will access
dangling pointer.

It's not trivial enough to do in this series, I will work on that on top, as
said below. For now, the user visible behavior of io cancellation is still the
same.

Thanks for testing.
Fam

> >
> >Later, we will change scsi device code to make this asynchronous, on top of
> >bdrv_aio_cancel_async.