[RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

[Emanuele Giuseppe Esposito]

This is a new attempt to replace the need to take the AioContext lock to
protect graph modifications. In particular, we aim to remove
(or better, substitute) the AioContext around bdrv_replace_child_noperm,
since this function changes BlockDriverState's ->parents and ->children
lists.

In the previous version, we decided to discard using subtree_drains to
protect the nodes, for different reasons: for those unfamiliar with it,
please see https://patchew.org/QEMU/20220301142113.163174-1-eesposit@redhat.com/

In this new attempt, we introduce a custom rwlock implementation.
This is inspired from cpus-common.c implementation, and aims to
reduce cacheline bouncing by having per-aiocontext counter of readers.
All details and implementation of the lock are in patch 3.

We distinguish between writer (main loop, under BQL) that modifies the
graph, and readers (all other coroutines running in various AioContext),
that go through the graph edges, reading ->parents and->children.
The writer (main loop)  has an "exclusive" access, so it first waits for
current read to finish, and then prevents incoming ones from
entering while it has the exclusive access.
The readers (coroutines in multiple AioContext) are free to
access the graph as long the writer is not modifying the graph.
In case it is, they go in a CoQueue and sleep until the writer
is done.

Patch 1 and 2 are in preparation for the lock, and fix bugs or missing
cases that popped out while implementing this lock.
Patch 3-7 implement and use the graph and assertions.
Note that unfortunately assert_graph_readable is not very efficient,
because it iterates through the list of all AioContexts to get the total number 
of readers.
For now we will use it only for debugging purposes and to be sure all
places are properly protected, but eventually we might want to disable it.

Patch 8 is an example of usage: as you can see, it is not essential that
the right coroutine takes the rdlock, but just that it is taken
somewhere. Otherwise the writer is not aware of any readers, and can write
while others are reading.

Next step is the most complex one: figure where to put the rdlocks.
While wrlock is easy, rdlock should ideally be:
- not recursive, otherwise it is not very intuitive
- not everywhere, prefereably only on the top caller of recursion trees
- still manage to protect ->parents and ->children reads.

Luckly, most of the cases where we recursively go through a graph are
the BlockDriverState callback functions in block_int-common.h
In order to understand what to protect, I categorized the callbacks in
block_int-common.h depending on the type of function that calls them:

1) If the caller is a generated_co_wrapper, this function must be
   protected by rdlock. The reason is that generated_co_wrapper create
   coroutines that run in the given bs AioContext, so it doesn't matter
   if we are running in the main loop or not, the coroutine might run
   in an iothread.
2) If the caller calls it directly, and has the GLOBAL_STATE_CODE() macro,
   then the function is safe. The main loop is the writer and thus won't
   read and write at the same time.
3) If the caller calls it directly, but has not the GLOBAL_STATE_CODE()
   macro, then we need to check the callers and see case-by-case if the
   caller is in the main loop, if it needs to take the lock, or delegate
   this duty to its caller (to reduce the places where to take it).

I used the vrc script (https://github.com/bonzini/vrc) to get help finding
all the callers of a callback. Using its filter function, I can
omit all functions protected by the added lock to avoid having duplicates
when querying for new callbacks.


Emanuele Giuseppe Esposito (8):
  aio_wait_kick: add missing memory barrier
  coroutine-lock: release lock when restarting all coroutines
  block: introduce a lock to protect graph operations
  async: register/unregister aiocontext in graph lock list
  block.c: wrlock in bdrv_replace_child_noperm
  block: assert that graph read and writes are performed correctly
  graph-lock: implement WITH_GRAPH_RDLOCK_GUARD and GRAPH_RDLOCK_GUARD
    macros
  mirror: protect drains in coroutine with rdlock

 block.c                                |  39 ++++-
 block/block-backend.c                  |   6 +-
 block/graph-lock.c                     | 227 +++++++++++++++++++++++++
 block/meson.build                      |   1 +
 block/mirror.c                         |   6 +
 include/block/aio.h                    |   9 +
 include/block/block_int-common.h       |   8 +-
 include/block/block_int-global-state.h |  17 --
 include/block/block_int.h              |   1 +
 include/block/graph-lock.h             | 101 +++++++++++
 include/qemu/coroutine.h               |  10 ++
 util/aio-wait.c                        |   3 +-
 util/async.c                           |   4 +
 util/meson.build                       |   1 +
 util/qemu-coroutine-lock.c             |  26 ++-
 15 files changed, 413 insertions(+), 46 deletions(-)
 create mode 100644 block/graph-lock.c
 create mode 100644 include/block/graph-lock.h

-- 
2.31.1