[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[RFC PATCH 14/15] jobs: add missing job locks to replace aiocontext lock
From: |
Emanuele Giuseppe Esposito |
Subject: |
[RFC PATCH 14/15] jobs: add missing job locks to replace aiocontext lock |
Date: |
Fri, 29 Oct 2021 12:39:13 -0400 |
find_block_job() and find_job() cannot be handled like
all other functions in the previous commit: in order
to avoid unneccessary job lock/unlock, replace the
aiocontext lock with the job_lock.
However, once we start dropping these aiocontex locks
we also break the assumptions of the callees in the job
API, many of which assume the aiocontext lock is held.
To keep this commit simple, handle the rest of the aiocontext
removal in the next commit.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
blockdev.c | 52 ++++++++++++++++++----------------------------------
job-qmp.c | 46 ++++++++++++++++------------------------------
2 files changed, 34 insertions(+), 64 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index c5a835d9ed..592ed4a0fc 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3318,48 +3318,43 @@ out:
aio_context_release(aio_context);
}
-/* Get a block job using its ID and acquire its AioContext */
-static BlockJob *find_block_job(const char *id, AioContext **aio_context,
- Error **errp)
+/* Get a block job using its ID. Returns with job_lock held on success */
+static BlockJob *find_block_job(const char *id, Error **errp)
{
BlockJob *job;
assert(id != NULL);
- *aio_context = NULL;
+ job_lock();
job = block_job_get(id);
if (!job) {
error_set(errp, ERROR_CLASS_DEVICE_NOT_ACTIVE,
"Block job '%s' not found", id);
+ job_unlock();
return NULL;
}
- *aio_context = blk_get_aio_context(job->blk);
- aio_context_acquire(*aio_context);
-
return job;
}
void qmp_block_job_set_speed(const char *device, int64_t speed, Error **errp)
{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
+ BlockJob *job = find_block_job(device, errp);
if (!job) {
return;
}
block_job_set_speed(job, speed, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_block_job_cancel(const char *device,
bool has_force, bool force, Error **errp)
{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
+ BlockJob *job = find_block_job(device, errp);
if (!job) {
return;
@@ -3378,13 +3373,12 @@ void qmp_block_job_cancel(const char *device,
trace_qmp_block_job_cancel(job);
job_user_cancel(&job->job, force, errp);
out:
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_block_job_pause(const char *device, Error **errp)
{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
+ BlockJob *job = find_block_job(device, errp);
if (!job) {
return;
@@ -3392,13 +3386,12 @@ void qmp_block_job_pause(const char *device, Error
**errp)
trace_qmp_block_job_pause(job);
job_user_pause(&job->job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_block_job_resume(const char *device, Error **errp)
{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
+ BlockJob *job = find_block_job(device, errp);
if (!job) {
return;
@@ -3406,13 +3399,12 @@ void qmp_block_job_resume(const char *device, Error
**errp)
trace_qmp_block_job_resume(job);
job_user_resume(&job->job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_block_job_complete(const char *device, Error **errp)
{
- AioContext *aio_context;
- BlockJob *job = find_block_job(device, &aio_context, errp);
+ BlockJob *job = find_block_job(device, errp);
if (!job) {
return;
@@ -3420,13 +3412,12 @@ void qmp_block_job_complete(const char *device, Error
**errp)
trace_qmp_block_job_complete(job);
job_complete(&job->job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_block_job_finalize(const char *id, Error **errp)
{
- AioContext *aio_context;
- BlockJob *job = find_block_job(id, &aio_context, errp);
+ BlockJob *job = find_block_job(id, errp);
if (!job) {
return;
@@ -3436,20 +3427,13 @@ void qmp_block_job_finalize(const char *id, Error
**errp)
job_ref(&job->job);
job_finalize(&job->job, errp);
- /*
- * Job's context might have changed via job_finalize (and job_txn_apply
- * automatically acquires the new one), so make sure we release the correct
- * one.
- */
- aio_context = blk_get_aio_context(job->blk);
job_unref(&job->job);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_block_job_dismiss(const char *id, Error **errp)
{
- AioContext *aio_context;
- BlockJob *bjob = find_block_job(id, &aio_context, errp);
+ BlockJob *bjob = find_block_job(id, errp);
Job *job;
if (!bjob) {
@@ -3459,7 +3443,7 @@ void qmp_block_job_dismiss(const char *id, Error **errp)
trace_qmp_block_job_dismiss(bjob);
job = &bjob->job;
job_dismiss(&job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_change_backing_file(const char *device,
diff --git a/job-qmp.c b/job-qmp.c
index a355dc2954..d592780953 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -29,29 +29,26 @@
#include "qapi/error.h"
#include "trace/trace-root.h"
-/* Get a job using its ID and acquire its AioContext */
-static Job *find_job(const char *id, AioContext **aio_context, Error **errp)
+/* Get a job using its ID. Returns with job_lock held on success. */
+static Job *find_job(const char *id, Error **errp)
{
Job *job;
- *aio_context = NULL;
+ job_lock();
job = job_get(id);
if (!job) {
error_setg(errp, "Job not found");
+ job_unlock();
return NULL;
}
- *aio_context = job->aio_context;
- aio_context_acquire(*aio_context);
-
return job;
}
void qmp_job_cancel(const char *id, Error **errp)
{
- AioContext *aio_context;
- Job *job = find_job(id, &aio_context, errp);
+ Job *job = find_job(id, errp);
if (!job) {
return;
@@ -59,13 +56,12 @@ void qmp_job_cancel(const char *id, Error **errp)
trace_qmp_job_cancel(job);
job_user_cancel(job, true, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_job_pause(const char *id, Error **errp)
{
- AioContext *aio_context;
- Job *job = find_job(id, &aio_context, errp);
+ Job *job = find_job(id, errp);
if (!job) {
return;
@@ -73,13 +69,12 @@ void qmp_job_pause(const char *id, Error **errp)
trace_qmp_job_pause(job);
job_user_pause(job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_job_resume(const char *id, Error **errp)
{
- AioContext *aio_context;
- Job *job = find_job(id, &aio_context, errp);
+ Job *job = find_job(id, errp);
if (!job) {
return;
@@ -87,13 +82,12 @@ void qmp_job_resume(const char *id, Error **errp)
trace_qmp_job_resume(job);
job_user_resume(job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_job_complete(const char *id, Error **errp)
{
- AioContext *aio_context;
- Job *job = find_job(id, &aio_context, errp);
+ Job *job = find_job(id, errp);
if (!job) {
return;
@@ -101,13 +95,12 @@ void qmp_job_complete(const char *id, Error **errp)
trace_qmp_job_complete(job);
job_complete(job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_job_finalize(const char *id, Error **errp)
{
- AioContext *aio_context;
- Job *job = find_job(id, &aio_context, errp);
+ Job *job = find_job(id, errp);
if (!job) {
return;
@@ -117,20 +110,13 @@ void qmp_job_finalize(const char *id, Error **errp)
job_ref(job);
job_finalize(job, errp);
- /*
- * Job's context might have changed via job_finalize (and job_txn_apply
- * automatically acquires the new one), so make sure we release the correct
- * one.
- */
- aio_context = job->aio_context;
job_unref(job);
- aio_context_release(aio_context);
+ job_unlock();
}
void qmp_job_dismiss(const char *id, Error **errp)
{
- AioContext *aio_context;
- Job *job = find_job(id, &aio_context, errp);
+ Job *job = find_job(id, errp);
if (!job) {
return;
@@ -138,7 +124,7 @@ void qmp_job_dismiss(const char *id, Error **errp)
trace_qmp_job_dismiss(job);
job_dismiss(&job, errp);
- aio_context_release(aio_context);
+ job_unlock();
}
static JobInfo *job_query_single(Job *job, Error **errp)
--
2.27.0
- [RFC PATCH 00/15] job: replace AioContext lock with job_mutex, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 01/15] jobs: add job-common.h, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 02/15] job.c: make job_lock/unlock public, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 03/15] job-common.h: categorize fields in struct Job, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 04/15] jobs: add job-monitor.h, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 05/15] job-monitor.h: define the job monitor API, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 06/15] jobs: add job-driver.h, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 09/15] job.c: move inner aiocontext lock in callbacks, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 07/15] job-driver.h: add helper functions, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 14/15] jobs: add missing job locks to replace aiocontext lock,
Emanuele Giuseppe Esposito <=
- [RFC PATCH 15/15] jobs: remove all unnecessary AioContext locks, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 10/15] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 13/15] jobs: use job locks and helpers also in the unit tests, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 12/15] jobs: protect jobs with job_lock/unlock, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 08/15] job.c: minor adjustments in preparation to job-driver, Emanuele Giuseppe Esposito, 2021/10/29
- [RFC PATCH 11/15] jobs: remove aiocontext locks since the functions are under BQL, Emanuele Giuseppe Esposito, 2021/10/29