[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 0/8] block: Asynchronous request cancellation
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v3 0/8] block: Asynchronous request cancellation |
Date: |
Tue, 2 Sep 2014 16:09:46 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Wed, Aug 27, 2014 at 10:49:08AM +0800, Fam Zheng wrote:
> v3: Drop "RFC".
> Improvements according to Paolo's comments:
> 05: Just use THREAD_DONE and ret = -ECANCELED in thread-pool.c
> 06: Don't check dbs->cancelled for twice.
> Don't set dbs->acb to NULL.
>
> v2: Drop the unfinished scsi part, which was broken in v1. (Paolo)
> Add refcnt in BlockDriverAIOCB to maintain invariant of acb availability
> in
> bdrv_aio_cancel_async. (Paolo)
> Drop blkdebug change. (Stefan)
>
> This series adds a new block layer API:
>
> void bdrv_aio_cancel_async(BlockDriverAIOCB *acb);
>
> which is similar to existing 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.
We need to get rid of .bdrv_aio_cancel(). Keeping both
.bdrv_aio_cancel() and .bdrv_aio_cancel_async() around is problematic
because they have slightly different semantics.
This patch series makes block driver cancellation more complex because
we support both approaches :(.
Could we do something like:
void bdrv_aio_cancel(BdrvAIOCB *acb)
{
bdrv_aiocb_ref(acb);
bdrv_aio_cancel_async(acb);
while (acb->ref > 1) {
aio_poll(bdrv_get_aio_context(acb->bs), true);
}
bdrv_aiocb_release(acb);
}
(pseudo-code)
Then .bdrv_aio_cancel() should be deleted.
Stefan
pgpKvPTRnzw47.pgp
Description: PGP signature
- Re: [Qemu-devel] [PATCH v3 0/8] block: Asynchronous request cancellation,
Stefan Hajnoczi <=