Re: [Qemu-devel] [PATCH v6 00/18] aio: drop io_flush()

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-8-6 23:07, Stefan Hajnoczi 写道:
> On Thu, Jul 25, 2013 at 05:18:07PM +0200, Stefan Hajnoczi wrote:
>> v6:
>>   * Fix block/stream.c:close_unused_images() dangling pointer in Patch 2
>>   * Rebase onto qemu.git/master
>>
>> v5:
>>   * Split out bdrv_delete() drain fix [bonzini]
>>   * Fix commit message  [bonzini]
>>
>> v4:
>>   * Ensure pending BHs are processed in bdrv_drain_all() [bonzini]
>>
>> v3:
>>   * I forgot about this series, time to push it again!
>>   * Rebase onto qemu.git/master
>>   * Drop now-unused AioFlushHandler typedef [bonzini]
>>
>> This series gets rid of aio's .io_flush() callback.  It's based on Paolo's
>> insight that bdrv_drain_all() can be implemented using the bs->tracked_requests
>> list.  io_flush() is redundant since the block layer already knows if requests
>> are pending.
>>
>> The point of this effort is to simplify our event loop(s).  If we can reduce
>> custom features like io_flush() it becomes possible to unify AioContext and
>> main-loop.c, maybe even to replace it with glib's main loop.
>>
>> This is also important to me for dataplane, since bdrv_drain_all() is one of
>> the synchronization points between threads.  QEMU monitor commands invoke
>> bdrv_drain_all() while the block device is accessed from a dataplane thread.
>>
>> Background on io_flush() semantics:
>>
>> The io_flush() handler must return 1 if this aio fd handler is active.  That
>> is, requests are pending and we'd like to make progress by monitoring the fd.
>>
>> If io_flush() returns 0, the aio event loop skips monitoring this fd.  This is
>> critical for block drivers like iscsi where we have an idle TCP socket which we
>> want to block on *only* when there are pending requests.
>>
>> The series works as follows:
>>
>> Part 1 - stop relying on .io_flush()
>>
>> The first patches change aio_poll() callers to check their termination
>> condition themselves instead of relying on .io_flush():
>>
>>    bcfa688 block: ensure bdrv_drain_all() works during bdrv_delete()
>>    c1351f5 block: stop relying on io_flush() in bdrv_drain_all()
>>    b75d9e5 dataplane/virtio-blk: check exit conditions before aio_poll()
>>    18fc4d6 tests: adjust test-aio to new aio_poll() semantics
>>    4c52a7c tests: adjust test-thread-pool to new aio_poll() semantics
>>
>> Part 2 - stop calling .io_flush() from aio_poll()
>>
>> The semantic change to aio_poll() is made here:
>>
>>    bacae7a aio: stop using .io_flush()
>>
>> Part 3 - drop io_flush() handlers, just pass NULL for the io_flush argument
>>
>> Remove the now dead code from all .io_flush() users:
>>
>>    720e0ad block/curl: drop curl_aio_flush()
>>    2830cf3 block/gluster: drop qemu_gluster_aio_flush_cb()
>>    c683e8e block/iscsi: drop iscsi_process_flush()
>>    5019686 block/linux-aio: drop qemu_laio_completion_cb()
>>    b862bcf block/nbd: drop nbd_have_request()
>>    516e517 block/rbd: drop qemu_rbd_aio_flush_cb()
>>    177090b block/sheepdog: drop have_co_req() and aio_flush_request()
>>    e2e9e85 block/ssh: drop return_true()
>>    7cc7bac dataplane/virtio-blk: drop flush_true() and flush_io()
>>    db3cb18 thread-pool: drop thread_pool_active()
>>    10a783b tests: drop event_active_cb()
>>
>> Part 4 - remove io_flush arguments from aio functions
>>
>> The big, mechanical patch that drops the io_flush argument:
>>
>>    4a8c36b aio: drop io_flush argument
>>
>> Note that I split Part 3 from Part 4 so it's easy to review individual block
>> drivers.
>>
>> Stefan Hajnoczi (18):
>>    block: ensure bdrv_drain_all() works during bdrv_delete()
>>    block: stop relying on io_flush() in bdrv_drain_all()
>>    dataplane/virtio-blk: check exit conditions before aio_poll()
>>    tests: adjust test-aio to new aio_poll() semantics
>>    tests: adjust test-thread-pool to new aio_poll() semantics
>>    aio: stop using .io_flush()
>>    block/curl: drop curl_aio_flush()
>>    block/gluster: drop qemu_gluster_aio_flush_cb()
>>    block/iscsi: drop iscsi_process_flush()
>>    block/linux-aio: drop qemu_laio_completion_cb()
>>    block/nbd: drop nbd_have_request()
>>    block/rbd: drop qemu_rbd_aio_flush_cb()
>>    block/sheepdog: drop have_co_req() and aio_flush_request()
>>    block/ssh: drop return_true()
>>    dataplane/virtio-blk: drop flush_true() and flush_io()
>>    thread-pool: drop thread_pool_active()
>>    tests: drop event_active_cb()
>>    aio: drop io_flush argument
>>
>>   aio-posix.c                     | 36 ++++++------------
>>   aio-win32.c                     | 37 ++++++++-----------
>>   async.c                         |  4 +-
>>   block.c                         | 50 ++++++++++++++++++-------
>>   block/curl.c                    | 25 ++-----------
>>   block/gluster.c                 | 21 ++---------
>>   block/iscsi.c                   | 10 +----
>>   block/linux-aio.c               | 18 +--------
>>   block/nbd.c                     | 18 ++-------
>>   block/rbd.c                     | 16 +-------
>>   block/sheepdog.c                | 33 ++++-------------
>>   block/ssh.c                     | 12 +-----
>>   block/stream.c                  |  6 ++-
>>   hw/block/dataplane/virtio-blk.c | 25 +++----------
>>   include/block/aio.h             | 14 +------
>>   main-loop.c                     |  9 ++---
>>   tests/test-aio.c                | 82 +++++++++++++++++++++--------------------
>>   tests/test-thread-pool.c        | 24 ++++++------
>>   thread-pool.c                   | 11 +-----
>>   19 files changed, 163 insertions(+), 288 deletions(-)
>
> Ping?
>
   I tried to apply this series to do more work above it, but seems
code conflict with upstream.

-- 
Best Regards

Wenchao Xia