[PATCH v2 00/14] aio: remove AioContext lock

Stefan Hajnoczi posted 14 patches 4 months, 3 weeks ago
Failed in applying to current master (apply log)
There is a newer version of this series
docs/devel/multiple-iothreads.txt    |  45 ++--
include/block/aio-wait.h             |  16 +-
include/block/aio.h                  |  17 --
include/block/block-common.h         |   3 -
include/block/block-global-state.h   |  23 +-
include/block/block-io.h             |  12 +-
include/block/block_int-common.h     |   2 -
include/block/graph-lock.h           |  21 +-
include/block/snapshot.h             |   2 -
include/hw/virtio/virtio-scsi.h      |  17 +-
include/qemu/job.h                   |  20 --
block.c                              | 363 ++++-----------------------
block/backup.c                       |   4 +-
block/blklogwrites.c                 |   8 +-
block/blkverify.c                    |   4 +-
block/block-backend.c                |  33 +--
block/commit.c                       |  16 +-
block/copy-before-write.c            |  22 +-
block/export/export.c                |  22 +-
block/export/vhost-user-blk-server.c |   4 -
block/graph-lock.c                   |  44 +---
block/io.c                           |  45 +---
block/mirror.c                       |  41 +--
block/monitor/bitmap-qmp-cmds.c      |  20 +-
block/monitor/block-hmp-cmds.c       |  29 ---
block/qapi-sysemu.c                  |  27 +-
block/qapi.c                         |  18 +-
block/qcow2.c                        |   4 +-
block/quorum.c                       |   8 +-
block/raw-format.c                   |   5 -
block/replication.c                  |  72 +-----
block/snapshot.c                     |  26 +-
block/stream.c                       |  12 +-
block/vmdk.c                         |  20 +-
block/write-threshold.c              |   6 -
blockdev.c                           | 319 +++++------------------
blockjob.c                           |  30 +--
hw/block/dataplane/virtio-blk.c      |  10 -
hw/block/dataplane/xen-block.c       |  17 +-
hw/block/virtio-blk.c                |  45 +---
hw/core/qdev-properties-system.c     |   9 -
hw/scsi/scsi-bus.c                   |   2 -
hw/scsi/scsi-disk.c                  |  46 ++--
hw/scsi/virtio-scsi.c                |  80 +++---
job.c                                |  16 --
migration/block.c                    |  33 +--
migration/migration-hmp-cmds.c       |   3 -
migration/savevm.c                   |  22 --
net/colo-compare.c                   |   2 -
qemu-img.c                           |   4 -
qemu-io.c                            |  10 +-
qemu-nbd.c                           |   2 -
replay/replay-debugging.c            |   4 -
system/dma-helpers.c                 |   3 +
tests/unit/test-aio.c                |  67 +----
tests/unit/test-bdrv-drain.c         |  91 ++-----
tests/unit/test-bdrv-graph-mod.c     |  26 +-
tests/unit/test-block-iothread.c     |  31 ---
tests/unit/test-blockjob.c           | 137 ----------
tests/unit/test-replication.c        |  11 -
util/async.c                         |  14 --
util/vhost-user-server.c             |   3 -
scripts/block-coroutine-wrapper.py   |   7 +-
tests/qemu-iotests/202               |   2 +-
tests/qemu-iotests/203               |   3 +-
tests/tsan/suppressions.tsan         |   1 -
66 files changed, 340 insertions(+), 1741 deletions(-)
[PATCH v2 00/14] aio: remove AioContext lock
Posted by Stefan Hajnoczi 4 months, 3 weeks ago
v2:
- Add Patch 2 "scsi: assert that callbacks run in the correct AioContext" [Kevin]
- Add Patch 7 "block: remove bdrv_co_lock()" [Eric and Kevin]
- Remove stray goto label in Patch 8 [Kevin]
- Fix "eeked" -> "eked" typo in Patch 10 [Eric]

This series removes the AioContext locking APIs from QEMU.
aio_context_acquire() and aio_context_release() are currently only needed to
support the locking discipline required by AIO_POLL_WHILE() (except for a stray
user that I converted in Patch 1). AIO_POLL_WHILE() doesn't really need the
AioContext lock anymore, so it's possible to remove the API. This is a nice
simplification because the AioContext locking rules were sometimes tricky or
underspecified, leading to many bugs of the years.

This patch series removes these APIs across the codebase and cleans up the
documentation/comments that refers to them.

Patch 1 is a AioContext lock user I forgot to convert in my earlier SCSI
conversion series.

Patch 2 adds an assertion to the SCSI code to ensure that callbacks are invoked
in the correct AioContext.

Patch 3 removes tests for the AioContext lock because they will no longer be
needed when the lock is gone.

Patches 4-10 remove the AioContext lock. These can be reviewed by categorizing
the call sites into 1. places that take the lock because they call an API that
requires the lock (ultimately AIO_POLL_WHILE()) and 2. places that take the
lock to protect state. There should be no instances of case 2 left. If you see
one, you've found a bug in this patch series!

Patches 11-14 remove comments.

Based-on: 20231204164259.1515217-1-stefanha@redhat.com ("[PATCH v2 0/4] scsi: eliminate AioContext lock")
Since SCSI needs to stop relying on the AioContext lock before we can remove
the lock.

Stefan Hajnoczi (14):
  virtio-scsi: replace AioContext lock with tmf_bh_lock
  scsi: assert that callbacks run in the correct AioContext
  tests: remove aio_context_acquire() tests
  aio: make aio_context_acquire()/aio_context_release() a no-op
  graph-lock: remove AioContext locking
  block: remove AioContext locking
  block: remove bdrv_co_lock()
  scsi: remove AioContext locking
  aio-wait: draw equivalence between AIO_WAIT_WHILE() and
    AIO_WAIT_WHILE_UNLOCKED()
  aio: remove aio_context_acquire()/aio_context_release() API
  docs: remove AioContext lock from IOThread docs
  scsi: remove outdated AioContext lock comment
  job: remove outdated AioContext locking comments
  block: remove outdated AioContext locking comments

 docs/devel/multiple-iothreads.txt    |  45 ++--
 include/block/aio-wait.h             |  16 +-
 include/block/aio.h                  |  17 --
 include/block/block-common.h         |   3 -
 include/block/block-global-state.h   |  23 +-
 include/block/block-io.h             |  12 +-
 include/block/block_int-common.h     |   2 -
 include/block/graph-lock.h           |  21 +-
 include/block/snapshot.h             |   2 -
 include/hw/virtio/virtio-scsi.h      |  17 +-
 include/qemu/job.h                   |  20 --
 block.c                              | 363 ++++-----------------------
 block/backup.c                       |   4 +-
 block/blklogwrites.c                 |   8 +-
 block/blkverify.c                    |   4 +-
 block/block-backend.c                |  33 +--
 block/commit.c                       |  16 +-
 block/copy-before-write.c            |  22 +-
 block/export/export.c                |  22 +-
 block/export/vhost-user-blk-server.c |   4 -
 block/graph-lock.c                   |  44 +---
 block/io.c                           |  45 +---
 block/mirror.c                       |  41 +--
 block/monitor/bitmap-qmp-cmds.c      |  20 +-
 block/monitor/block-hmp-cmds.c       |  29 ---
 block/qapi-sysemu.c                  |  27 +-
 block/qapi.c                         |  18 +-
 block/qcow2.c                        |   4 +-
 block/quorum.c                       |   8 +-
 block/raw-format.c                   |   5 -
 block/replication.c                  |  72 +-----
 block/snapshot.c                     |  26 +-
 block/stream.c                       |  12 +-
 block/vmdk.c                         |  20 +-
 block/write-threshold.c              |   6 -
 blockdev.c                           | 319 +++++------------------
 blockjob.c                           |  30 +--
 hw/block/dataplane/virtio-blk.c      |  10 -
 hw/block/dataplane/xen-block.c       |  17 +-
 hw/block/virtio-blk.c                |  45 +---
 hw/core/qdev-properties-system.c     |   9 -
 hw/scsi/scsi-bus.c                   |   2 -
 hw/scsi/scsi-disk.c                  |  46 ++--
 hw/scsi/virtio-scsi.c                |  80 +++---
 job.c                                |  16 --
 migration/block.c                    |  33 +--
 migration/migration-hmp-cmds.c       |   3 -
 migration/savevm.c                   |  22 --
 net/colo-compare.c                   |   2 -
 qemu-img.c                           |   4 -
 qemu-io.c                            |  10 +-
 qemu-nbd.c                           |   2 -
 replay/replay-debugging.c            |   4 -
 system/dma-helpers.c                 |   3 +
 tests/unit/test-aio.c                |  67 +----
 tests/unit/test-bdrv-drain.c         |  91 ++-----
 tests/unit/test-bdrv-graph-mod.c     |  26 +-
 tests/unit/test-block-iothread.c     |  31 ---
 tests/unit/test-blockjob.c           | 137 ----------
 tests/unit/test-replication.c        |  11 -
 util/async.c                         |  14 --
 util/vhost-user-server.c             |   3 -
 scripts/block-coroutine-wrapper.py   |   7 +-
 tests/qemu-iotests/202               |   2 +-
 tests/qemu-iotests/203               |   3 +-
 tests/tsan/suppressions.tsan         |   1 -
 66 files changed, 340 insertions(+), 1741 deletions(-)

-- 
2.43.0
Re: [PATCH v2 00/14] aio: remove AioContext lock
Posted by Kevin Wolf 4 months, 1 week ago
Am 05.12.2023 um 19:19 hat Stefan Hajnoczi geschrieben:
> v2:
> - Add Patch 2 "scsi: assert that callbacks run in the correct AioContext" [Kevin]
> - Add Patch 7 "block: remove bdrv_co_lock()" [Eric and Kevin]
> - Remove stray goto label in Patch 8 [Kevin]
> - Fix "eeked" -> "eked" typo in Patch 10 [Eric]
> 
> This series removes the AioContext locking APIs from QEMU.
> aio_context_acquire() and aio_context_release() are currently only needed to
> support the locking discipline required by AIO_POLL_WHILE() (except for a stray
> user that I converted in Patch 1). AIO_POLL_WHILE() doesn't really need the
> AioContext lock anymore, so it's possible to remove the API. This is a nice
> simplification because the AioContext locking rules were sometimes tricky or
> underspecified, leading to many bugs of the years.
> 
> This patch series removes these APIs across the codebase and cleans up the
> documentation/comments that refers to them.
> 
> Patch 1 is a AioContext lock user I forgot to convert in my earlier SCSI
> conversion series.
> 
> Patch 2 adds an assertion to the SCSI code to ensure that callbacks are invoked
> in the correct AioContext.
> 
> Patch 3 removes tests for the AioContext lock because they will no longer be
> needed when the lock is gone.
> 
> Patches 4-10 remove the AioContext lock. These can be reviewed by categorizing
> the call sites into 1. places that take the lock because they call an API that
> requires the lock (ultimately AIO_POLL_WHILE()) and 2. places that take the
> lock to protect state. There should be no instances of case 2 left. If you see
> one, you've found a bug in this patch series!
> 
> Patches 11-14 remove comments.

Thanks, applied to the block branch.

Kevin