From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56387) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6irA-0002Z9-OV for qemu-devel@nongnu.org; Tue, 06 Aug 2013 11:07:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V6ir1-0004Ox-4Q for qemu-devel@nongnu.org; Tue, 06 Aug 2013 11:07:28 -0400 Received: from mail-wi0-x231.google.com ([2a00:1450:400c:c05::231]:65448) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6ir0-0004Oi-SW for qemu-devel@nongnu.org; Tue, 06 Aug 2013 11:07:19 -0400 Received: by mail-wi0-f177.google.com with SMTP id hq12so534201wib.16 for ; Tue, 06 Aug 2013 08:07:18 -0700 (PDT) Date: Tue, 6 Aug 2013 17:07:15 +0200 From: Stefan Hajnoczi Message-ID: <20130806150715.GB9327@stefanha-thinkpad.redhat.com> References: <1374765505-14356-1-git-send-email-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1374765505-14356-1-git-send-email-stefanha@redhat.com> Subject: Re: [Qemu-devel] [PATCH v6 00/18] aio: drop io_flush() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Ping Fan Liu , Stefan Hajnoczi , qemu-devel@nongnu.org, Michael Roth , alex@alex.org.uk, Paolo Bonzini 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?