All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 00/20] job: replace AioContext lock with job_mutex
@ 2022-06-29 14:15 Emanuele Giuseppe Esposito
  2022-06-29 14:15 ` [PATCH v8 01/20] job.c: make job_mutex and job_lock/unlock() public Emanuele Giuseppe Esposito
                   ` (20 more replies)
  0 siblings, 21 replies; 59+ messages in thread
From: Emanuele Giuseppe Esposito @ 2022-06-29 14:15 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Hanna Reitz, Paolo Bonzini, John Snow,
	Vladimir Sementsov-Ogievskiy, Wen Congyang, Xie Changlong,
	Markus Armbruster, Stefan Hajnoczi, Fam Zheng, qemu-devel,
	Emanuele Giuseppe Esposito

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. In this way we can freely create locking sections
without worrying about deadlocks with the aiocontext lock.

Patch 2 defines what fields in the job structure need protection.
Patches 3-6 are in preparation to the job locks, moving functions
from global to static and introducing helpers.

Patch 7-9 introduce the (nop) job lock into the job API and
its users, and patches 10-13 categorize respectively locked and
unlocked functions in the job API.

Patches 14-17 take care of protecting job->aio_context, and
finally patch 18 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).

---
v8:
* reorganize patch ordering according with Vladimir proposal
* minor nitpicks

v7:
* s/temporary/temporarly
* double identical locking comment to the same function
* patch 2: add "Protected by AioContext lock" to better categorize fields in
  job.h
* use same comment style in all function headers ("Just like {funct}, but
  called between job_lock and job_unlock")

v6:
* patch 4 and 6 squashed with patch 19 (enable job lock and
  reduce/remove AioContext lock)
* patch 19: job_unref_locked read the aiocontext inside the
  job lock.

v5:
* just restructured patches a little bit better, as there were
  functions used before they were defined.
* rebased on kwolf/block branch and API split serie

v4:
* move "protected by job_mutex" from patch 2 to 15, where the job_mutex is
  actually added.
* s/aio_co_enter/aio_co_schedule in job.c, and adjust tests accordingly.
* remove job_get_aio_context, add job_set_aio_context. Use "fake rwlock"
  to protect job->aiocontext.
* get rid of useless getters method, namely:
  job_get_status
  job_get_pause_count
  job_get_paused
  job_get_busy
  They are all used only by tests, and such getter is pretty useless.
  Replace with job_lock(); assert(); job_unlock();
* use job lock macros instead of job lock/unlock in unit tests.
* convert also blockjob functions to have _locked
* put the job_lock/unlock patches before the _locked ones
* replace aio_co_enter in job.c and detect change of context

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 (19):
  job.c: make job_mutex and job_lock/unlock() public
  job.h: categorize fields in struct Job
  job.c: API functions not used outside should be static
  aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED
  job.c: add job_lock/unlock while keeping job.h intact
  job.h: define functions called without job lock held
  job.h: add _locked public functions
  blockjob.h: introduce block_job  _locked() APIs
  blockjob: rename notifier callbacks as _locked
  jobs: add job lock in find_* functions
  jobs: use job locks also in the unit tests
  block/mirror.c: use of job helpers in drivers to avoid TOC/TOU
  jobs: group together API calls under the same job lock
  commit and mirror: create new nodes using bdrv_get_aio_context, and
    not the job aiocontext
  jobs: protect job.aio_context with BQL and job_mutex
  job.c: enable job lock/unlock and remove Aiocontext locks
  block_job_query: remove atomic read
  blockjob: remove unused functions
  job: remove unused functions

Paolo Bonzini (1):
  job: detect change of aiocontext within job coroutine

 block.c                          |  20 +-
 block/commit.c                   |   4 +-
 block/mirror.c                   |  21 +-
 block/replication.c              |   6 +-
 blockdev.c                       | 129 +++---
 blockjob.c                       | 126 +++---
 include/block/aio-wait.h         |  17 +-
 include/block/blockjob.h         |  28 +-
 include/qemu/job.h               | 268 +++++++++----
 job-qmp.c                        |  87 ++--
 job.c                            | 654 +++++++++++++++++++------------
 monitor/qmp-cmds.c               |   7 +-
 qemu-img.c                       |  41 +-
 tests/unit/test-bdrv-drain.c     |  80 ++--
 tests/unit/test-block-iothread.c |   8 +-
 tests/unit/test-blockjob-txn.c   |  24 +-
 tests/unit/test-blockjob.c       | 107 +++--
 17 files changed, 999 insertions(+), 628 deletions(-)

-- 
2.31.1



^ permalink raw reply	[flat|nested] 59+ messages in thread

end of thread, other threads:[~2022-07-06 22:09 UTC | newest]

Thread overview: 59+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-29 14:15 [PATCH v8 00/20] job: replace AioContext lock with job_mutex Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 01/20] job.c: make job_mutex and job_lock/unlock() public Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 02/20] job.h: categorize fields in struct Job Emanuele Giuseppe Esposito
2022-07-05  7:29   ` Stefan Hajnoczi
2022-06-29 14:15 ` [PATCH v8 03/20] job.c: API functions not used outside should be static Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 04/20] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 05/20] job.c: add job_lock/unlock while keeping job.h intact Emanuele Giuseppe Esposito
2022-07-05  7:39   ` Stefan Hajnoczi
2022-07-05  8:07     ` Emanuele Giuseppe Esposito
2022-07-06 10:09       ` Stefan Hajnoczi
2022-07-05  7:39   ` Stefan Hajnoczi
2022-07-05 10:23   ` Vladimir Sementsov-Ogievskiy
2022-06-29 14:15 ` [PATCH v8 06/20] job.h: define functions called without job lock held Emanuele Giuseppe Esposito
2022-07-05  7:43   ` Stefan Hajnoczi
2022-07-05 10:53   ` Vladimir Sementsov-Ogievskiy
2022-07-06  8:22     ` Emanuele Giuseppe Esposito
2022-07-06  9:48       ` Vladimir Sementsov-Ogievskiy
2022-07-05 10:54   ` Vladimir Sementsov-Ogievskiy
2022-07-06  8:23     ` Emanuele Giuseppe Esposito
2022-07-06  9:51       ` Vladimir Sementsov-Ogievskiy
2022-06-29 14:15 ` [PATCH v8 07/20] job.h: add _locked public functions Emanuele Giuseppe Esposito
2022-07-05  7:44   ` Stefan Hajnoczi
2022-07-05 10:58   ` Vladimir Sementsov-Ogievskiy
2022-06-29 14:15 ` [PATCH v8 08/20] blockjob.h: introduce block_job _locked() APIs Emanuele Giuseppe Esposito
2022-07-05  7:58   ` Stefan Hajnoczi
2022-07-05  8:12     ` Emanuele Giuseppe Esposito
2022-07-05 15:01   ` Vladimir Sementsov-Ogievskiy
2022-07-06 12:05     ` Emanuele Giuseppe Esposito
2022-07-06 12:23       ` Vladimir Sementsov-Ogievskiy
2022-07-06 12:36         ` Emanuele Giuseppe Esposito
2022-07-06 12:59           ` Emanuele Giuseppe Esposito
2022-07-06 17:23             ` Vladimir Sementsov-Ogievskiy
2022-06-29 14:15 ` [PATCH v8 09/20] blockjob: rename notifier callbacks as _locked Emanuele Giuseppe Esposito
2022-07-05  8:00   ` Stefan Hajnoczi
2022-06-29 14:15 ` [PATCH v8 10/20] jobs: add job lock in find_* functions Emanuele Giuseppe Esposito
2022-07-05  8:03   ` Stefan Hajnoczi
2022-06-29 14:15 ` [PATCH v8 11/20] jobs: use job locks also in the unit tests Emanuele Giuseppe Esposito
2022-07-05  8:04   ` Stefan Hajnoczi
2022-06-29 14:15 ` [PATCH v8 12/20] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 13/20] jobs: group together API calls under the same job lock Emanuele Giuseppe Esposito
2022-07-05  8:14   ` Stefan Hajnoczi
2022-07-05  8:17     ` Emanuele Giuseppe Esposito
2022-07-05 13:01       ` Emanuele Giuseppe Esposito
2022-07-05 13:22         ` Vladimir Sementsov-Ogievskiy
2022-07-06 10:13           ` Stefan Hajnoczi
2022-07-05 14:55   ` Vladimir Sementsov-Ogievskiy
2022-06-29 14:15 ` [PATCH v8 14/20] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 15/20] job: detect change of aiocontext within job coroutine Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 16/20] jobs: protect job.aio_context with BQL and job_mutex Emanuele Giuseppe Esposito
2022-07-05 12:44   ` Stefan Hajnoczi
2022-06-29 14:15 ` [PATCH v8 17/20] job.c: enable job lock/unlock and remove Aiocontext locks Emanuele Giuseppe Esposito
2022-07-05 13:07   ` Stefan Hajnoczi
2022-07-06 21:29     ` Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 18/20] block_job_query: remove atomic read Emanuele Giuseppe Esposito
2022-06-29 14:15 ` [PATCH v8 19/20] blockjob: remove unused functions Emanuele Giuseppe Esposito
2022-07-05 13:10   ` Stefan Hajnoczi
2022-06-29 14:15 ` [PATCH v8 20/20] job: " Emanuele Giuseppe Esposito
2022-07-05 13:11   ` Stefan Hajnoczi
2022-07-05 13:12 ` [PATCH v8 00/20] job: replace AioContext lock with job_mutex Stefan Hajnoczi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.