Re: [PATCH v2 00/11] block: Re-enable the graph lock

[Stefan Hajnoczi]

On Mon, Jun 05, 2023 at 10:57:00AM +0200, Kevin Wolf wrote:
> This series fixes the deadlock that was observed before commit ad128dff
> ('graph-lock: Disable locking for now'), which just disabled the graph
> lock completely as a workaround to get 8.0.1 stable.
> 
> In theory the problem is simple: We can't poll while still holding the
> lock of a different AioContext. So bdrv_graph_wrlock() just needs to
> drop that lock before it polls. However, there are a number of callers
> that don't even hold the AioContext lock they are supposed to hold, so
> temporarily unlocking tries to unlock a mutex that isn't locked,
> resulting in assertion failures.
> 
> Therefore, much of this series is just for fixing AioContext locking
> correctness. It is only the last two patches that actually fix the
> deadlock and reenable the graph locking.
> 
> v2:
> - Fixed patch 2 to actually lock the correct AioContext even if the
>   device doesn't support iothreads
> - Improved the commit message for patch 7 [Eric]
> - Fixed mismerge in patch 11 (v1 incorrectly left an #if 0 around)
> 
> Kevin Wolf (11):
>   iotests: Test active commit with iothread and background I/O
>   qdev-properties-system: Lock AioContext for blk_insert_bs()
>   test-block-iothread: Lock AioContext for blk_insert_bs()
>   block: Fix AioContext locking in bdrv_open_child()
>   block: Fix AioContext locking in bdrv_attach_child_common()
>   block: Fix AioContext locking in bdrv_reopen_parse_file_or_backing()
>   block: Fix AioContext locking in bdrv_open_inherit()
>   block: Fix AioContext locking in bdrv_open_backing_file()
>   blockjob: Fix AioContext locking in block_job_add_bdrv()
>   graph-lock: Unlock the AioContext while polling
>   Revert "graph-lock: Disable locking for now"
> 
>  include/block/graph-lock.h                    |   6 +-
>  block.c                                       | 103 ++++++++++++++++--
>  block/graph-lock.c                            |  42 ++++---
>  blockjob.c                                    |  17 ++-
>  hw/core/qdev-properties-system.c              |   8 +-
>  tests/unit/test-block-iothread.c              |   7 +-
>  .../tests/iothreads-commit-active             |  85 +++++++++++++++
>  .../tests/iothreads-commit-active.out         |  23 ++++
>  8 files changed, 250 insertions(+), 41 deletions(-)
>  create mode 100755 tests/qemu-iotests/tests/iothreads-commit-active
>  create mode 100644 tests/qemu-iotests/tests/iothreads-commit-active.out
> 
> -- 
> 2.40.1
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

signature.asc
Description: PGP signature