Re: [Qemu-devel] [PATCH v3 00/18] aio_context_acquire/release pushdown, part 2

[Stefan Hajnoczi]

On Wed, Feb 01, 2017 at 04:05:15AM -0800, Paolo Bonzini wrote:
> This series pushes down aio_context_acquire/release to the point
> where we can actually reason on using different fine-grained mutexes.
> 
> The main infrastructure is introduced in patch 2.  The new API aio_co_wake
> starts a coroutine with aio_context_acquire/release protection, which
> requires tracking each coroutine's "home" AioContext.  aio_co_schedule
> instead takes care of moving a sleeping coroutine to a different
> AioContext, also ensuring that it runs under aio_context_acquire/release.
> This is useful to implement bdrv_set_aio_context, as a simpler alternative
> to bottom halves.  Even though one-shot BHs are already simpler than
> what we had before, after this patch aio_co_wake and aio_co_schedule
> save you from having to do aio_context_acquire/release explicitly.
> 
> Ultimately, only a small number of aio_context_acquire/release pairs are
> added after the pushdown.  These are mostly in drivers that use external
> libraries (and actually they could already be replaced by QemuMutex)
> and in device models that support multithreaded operation (aka iothread
> aka dataplane).
> 
> Notably, coroutines need not care about aio_context_acquire/release.
> The device models ensure that the first creation of the coroutine has
> the AioContext, while aio_co_wake/aio_co_schedule do the same after
> they yield.  Therefore, most of the files only need to use those two
> functions instead of, respectively, qemu_coroutine_enter and
> aio_bh_schedule_oneshot.
> 
> However, this is only an intermediate step which is needed because the
> block layer and qemu-coroutine locks are thread-unsafe.  So the next
> part will add separate locking, independent of AioContext, to block.c and
> mostly block/io.c---this includes making CoMutex thread-safe.  Patch 17
> therefore already documents the current locking policies block.h to
> prepare for the next series.
> 
> v2->v3:
>         moved patch 4 ("block: move AioContext and QEMUTimer to libqemuutil")
>                 earlier due to rebase
>         add coroutine_fn annotation in test-aio-multithread.c [Stefan]
>         don't remove yield_until_fd_readable, will resubmit to trivial 
> [Stefan]
>         add comments about wake/yield sequence [Stefan]
>         new patch 9 to fix an incorrect (in the future) use of aio_co_schedule
>         
> v1->v2:
>       new patch (4) removing block-obj-y -> io-obj-y dependency
>       removed QIOChannelRestart from
>               "io: add methods to set I/O handlers on AioContext"
>       improved qio_channel_set_aio_context docs, renamed to
>               qio_channel_attach_aio_context
>       fixed pasto in "io: make qio_channel_yield aware of AioContexts"
>       document restrictions in bdrv_aio_cancel due to qemu_aio_ref
>       converted NBD server to qio_channel_yield too
>       allow NULL s->read_reply_co
> 
> 
> Paolo Bonzini (18):
>   block: move AioContext and QEMUTimer to libqemuutil
>   aio: introduce aio_co_schedule and aio_co_wake
>   block-backend: allow blk_prw from coroutine context
>   test-thread-pool: use generic AioContext infrastructure
>   io: add methods to set I/O handlers on AioContext
>   io: make qio_channel_yield aware of AioContexts
>   nbd: convert to use qio_channel_yield
>   coroutine-lock: reschedule coroutine on the AioContext it was running
>     on
>   blkdebug: reschedule coroutine on the AioContext it is running on
>   qed: introduce qed_aio_start_io and qed_aio_next_io_cb
>   aio: push aio_context_acquire/release down to dispatching
>   block: explicitly acquire aiocontext in timers that need it
>   block: explicitly acquire aiocontext in callbacks that need it
>   block: explicitly acquire aiocontext in bottom halves that need it
>   block: explicitly acquire aiocontext in aio callbacks that need it
>   aio-posix: partially inline aio_dispatch into aio_poll
>   async: remove unnecessary inc/dec pairs
>   block: document fields protected by AioContext lock
> 
>  Makefile.objs                       |   4 -
>  block/blkdebug.c                    |   9 +-
>  block/blkreplay.c                   |   2 +-
>  block/block-backend.c               |  13 ++-
>  block/curl.c                        |  44 ++++++--
>  block/gluster.c                     |   9 +-
>  block/io.c                          |  38 ++-----
>  block/iscsi.c                       |  15 ++-
>  block/linux-aio.c                   |  10 +-
>  block/mirror.c                      |  12 +-
>  block/nbd-client.c                  | 117 +++++++++-----------
>  block/nbd-client.h                  |   2 +-
>  block/nfs.c                         |   9 +-
>  block/qed-cluster.c                 |   2 +
>  block/qed-table.c                   |  12 +-
>  block/qed.c                         |  58 +++++++---
>  block/qed.h                         |   3 +
>  block/sheepdog.c                    |  29 ++---
>  block/ssh.c                         |  29 ++---
>  block/throttle-groups.c             |   2 +
>  block/win32-aio.c                   |   9 +-
>  dma-helpers.c                       |   2 +
>  hw/block/virtio-blk.c               |  19 +++-
>  hw/scsi/scsi-bus.c                  |   2 +
>  hw/scsi/scsi-disk.c                 |  15 +++
>  hw/scsi/scsi-generic.c              |  20 +++-
>  hw/scsi/virtio-scsi.c               |   6 +
>  include/block/aio.h                 |  38 ++++++-
>  include/block/block_int.h           |  64 ++++++-----
>  include/io/channel.h                |  72 +++++++++++-
>  include/qemu/coroutine_int.h        |  11 +-
>  include/sysemu/block-backend.h      |  14 ++-
>  io/channel-command.c                |  13 +++
>  io/channel-file.c                   |  11 ++
>  io/channel-socket.c                 |  16 ++-
>  io/channel-tls.c                    |  12 ++
>  io/channel-watch.c                  |   6 +
>  io/channel.c                        |  97 ++++++++++++----
>  nbd/client.c                        |   2 +-
>  nbd/common.c                        |   9 +-
>  nbd/server.c                        |  94 +++++-----------
>  stubs/Makefile.objs                 |   1 +
>  stubs/linux-aio.c                   |  32 ++++++
>  stubs/set-fd-handler.c              |  11 --
>  tests/Makefile.include              |  18 +--
>  tests/iothread.c                    |  91 +++++++++++++++
>  tests/iothread.h                    |  25 +++++
>  tests/test-aio-multithread.c        | 213 
> ++++++++++++++++++++++++++++++++++++
>  tests/test-thread-pool.c            |  12 +-
>  trace-events                        |   4 +
>  util/Makefile.objs                  |   6 +-
>  aio-posix.c => util/aio-posix.c     |  60 +++-------
>  aio-win32.c => util/aio-win32.c     |  30 ++---
>  util/aiocb.c                        |  55 ++++++++++
>  async.c => util/async.c             |  84 ++++++++++++--
>  iohandler.c => util/iohandler.c     |   0
>  main-loop.c => util/main-loop.c     |   0
>  util/qemu-coroutine-lock.c          |   5 +-
>  util/qemu-coroutine-sleep.c         |   2 +-
>  util/qemu-coroutine.c               |   8 ++
>  qemu-timer.c => util/qemu-timer.c   |   0
>  thread-pool.c => util/thread-pool.c |   6 +-
>  util/trace-events                   |   1 -
>  63 files changed, 1166 insertions(+), 454 deletions(-)
>  create mode 100644 stubs/linux-aio.c
>  create mode 100644 tests/iothread.c
>  create mode 100644 tests/iothread.h
>  create mode 100644 tests/test-aio-multithread.c
>  rename aio-posix.c => util/aio-posix.c (94%)
>  rename aio-win32.c => util/aio-win32.c (95%)
>  create mode 100644 util/aiocb.c
>  rename async.c => util/async.c (82%)
>  rename iohandler.c => util/iohandler.c (100%)
>  rename main-loop.c => util/main-loop.c (100%)
>  rename qemu-timer.c => util/qemu-timer.c (100%)
>  rename thread-pool.c => util/thread-pool.c (98%)

Please rebase.

Reviewed-by: Stefan Hajnoczi <address@hidden>

signature.asc
Description: PGP signature