qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 00/16] job: replace AioContext lock with job_mutex


From: Paolo Bonzini
Subject: Re: [PATCH v3 00/16] job: replace AioContext lock with job_mutex
Date: Wed, 19 Jan 2022 12:15:46 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.4.0

On 1/5/22 15:01, Emanuele Giuseppe Esposito wrote:
In this series, we want to remove the AioContext lock and instead
use the already existent job_mutex to protect the job structures
and list. This is part of the work to get rid of AioContext lock
usage in favour of smaller granularity locks.

In order to simplify reviewer's job, job lock/unlock functions and
macros are added as empty prototypes (nop) in patch 1.
They are converted to use the actual job mutex only in the last
patch, 14. In this way we can freely create locking sections
without worrying about deadlocks with the aiocontext lock.

Oops, sorry -- I missed this explanation when first reading the cover letter. Good job, though it needs another iteration; especially for patch 14, and possibly to decide the right placement of patches 10-13.

Thanks,

Paolo

Patch 2 defines what fields in the job structure need protection,
and patches 3-4 categorize respectively locked and unlocked
functions in the job API.

Patch 5-9 are in preparation to the job locks, they try to reduce
the aiocontext critical sections and other minor fixes.

Patch 10-13 introduces the (nop) job lock into the job API and
its users, following the comments and categorizations done in
patch 2-3-4.

Patch 14 makes the prototypes in patch 1 use the job_mutex and
removes all aiocontext lock at the same time.

Tested this series by running unit tests, qemu-iotests and qtests
(x86_64).

This serie is based on my previous series "block layer: split
block APIs in global state and I/O".

Based-on: <20211124064418.3120601-1-eesposit@redhat.com>
---
v3:
* add "_locked" suffix to the functions called under job_mutex lock
* rename _job_lock in real_job_lock
* job_mutex is now public, and drivers like monitor use it directly
* introduce and protect job_get_aio_context
* remove mirror-specific APIs and just use WITH_JOB_GUARD
* more extensive use of WITH_JOB_GUARD and JOB_LOCK_GUARD

RFC v2:
* use JOB_LOCK_GUARD and WITH_JOB_LOCK_GUARD
* mu(u)ltiple typos in commit messages
* job API split patches are sent separately in another series
* use of empty job_{lock/unlock} and JOB_LOCK_GUARD/WITH_JOB_LOCK_GUARD
   to avoid deadlocks and simplify the reviewer job
* move patch 11 (block_job_query: remove atomic read) as last

Emanuele Giuseppe Esposito (16):
   job.c: make job_mutex and job_lock/unlock() public
   job.h: categorize fields in struct Job
   job.h: define locked functions
   job.h: define unlocked functions
   block/mirror.c: use of job helpers in drivers to avoid TOC/TOU
   job.c: make job_event_* functions static
   job.c: move inner aiocontext lock in callbacks
   aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED
   jobs: remove aiocontext locks since the functions are under BQL
   jobs: protect jobs with job_lock/unlock
   jobs: document all static functions and add _locked() suffix
   jobs: use job locks and helpers also in the unit tests
   jobs: add job lock in find_* functions
   job.c: use job_get_aio_context()
   job.c: enable job lock/unlock and remove Aiocontext locks
   block_job_query: remove atomic read

  block.c                          |  18 +-
  block/commit.c                   |   4 +-
  block/mirror.c                   |  21 +-
  block/replication.c              |  10 +-
  blockdev.c                       | 112 ++----
  blockjob.c                       | 122 +++---
  include/block/aio-wait.h         |  15 +-
  include/qemu/job.h               | 317 +++++++++++----
  job-qmp.c                        |  74 ++--
  job.c                            | 656 +++++++++++++++++++------------
  monitor/qmp-cmds.c               |   6 +-
  qemu-img.c                       |  41 +-
  tests/unit/test-bdrv-drain.c     |  46 +--
  tests/unit/test-block-iothread.c |  14 +-
  tests/unit/test-blockjob-txn.c   |  24 +-
  tests/unit/test-blockjob.c       |  98 ++---
  16 files changed, 947 insertions(+), 631 deletions(-)





reply via email to

[Prev in Thread] Current Thread [Next in Thread]