Message ID | 20220105140208.365608-1-eesposit@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | job: replace AioContext lock with job_mutex | expand |
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(-) >