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(-)