[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v12 13/21] jobs: protect job.aio_context with BQL and job_mutex
From: |
Emanuele Giuseppe Esposito |
Subject: |
[PATCH v12 13/21] jobs: protect job.aio_context with BQL and job_mutex |
Date: |
Mon, 26 Sep 2022 05:32:06 -0400 |
In order to make it thread safe, implement a "fake rwlock",
where we allow reads under BQL *or* job_mutex held, but
writes only under BQL *and* job_mutex.
The only write we have is in child_job_set_aio_ctx, which always
happens under drain (so the job is paused).
For this reason, introduce job_set_aio_context and make sure that
the context is set under BQL, job_mutex and drain.
Also make sure all other places where the aiocontext is read
are protected.
The reads in commit.c and mirror.c are actually safe, because always
done under BQL.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
block/replication.c | 1 +
blockjob.c | 3 ++-
include/qemu/job.h | 23 ++++++++++++++++++++---
job.c | 12 ++++++++++++
4 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/block/replication.c b/block/replication.c
index 55c8f894aa..5977f7a833 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -142,6 +142,7 @@ static void replication_close(BlockDriverState *bs)
{
BDRVReplicationState *s = bs->opaque;
Job *commit_job;
+ GLOBAL_STATE_CODE();
if (s->stage == BLOCK_REPLICATION_RUNNING) {
replication_stop(s->rs, false, NULL);
diff --git a/blockjob.c b/blockjob.c
index 96fb9d9f73..c8919cef9b 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -162,12 +162,13 @@ static void child_job_set_aio_ctx(BdrvChild *c,
AioContext *ctx,
bdrv_set_aio_context_ignore(sibling->bs, ctx, ignore);
}
- job->job.aio_context = ctx;
+ job_set_aio_context(&job->job, ctx);
}
static AioContext *child_job_get_parent_aio_context(BdrvChild *c)
{
BlockJob *job = c->opaque;
+ GLOBAL_STATE_CODE();
return job->job.aio_context;
}
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 5709e8d4a8..50a4c06b93 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -74,11 +74,17 @@ typedef struct Job {
/* ProgressMeter API is thread-safe */
ProgressMeter progress;
+ /**
+ * AioContext to run the job coroutine in.
+ * The job Aiocontext can be read when holding *either*
+ * the BQL (so we are in the main loop) or the job_mutex.
+ * It can only be written when we hold *both* BQL
+ * and the job_mutex.
+ */
+ AioContext *aio_context;
- /** Protected by AioContext lock */
- /** AioContext to run the job coroutine in */
- AioContext *aio_context;
+ /** Protected by AioContext lock */
/** Reference count of the block job */
int refcnt;
@@ -741,4 +747,15 @@ int job_finish_sync(Job *job, void (*finish)(Job *, Error
**errp),
int job_finish_sync_locked(Job *job, void (*finish)(Job *, Error **errp),
Error **errp);
+/**
+ * Sets the @job->aio_context.
+ * Called with job_mutex *not* held.
+ *
+ * This function must run in the main thread to protect against
+ * concurrent read in job_finish_sync_locked(), takes the job_mutex
+ * lock to protect against the read in job_do_yield_locked(), and must
+ * be called when the job is quiescent.
+ */
+void job_set_aio_context(Job *job, AioContext *ctx);
+
#endif
diff --git a/job.c b/job.c
index 85ae843f03..c4ac363f08 100644
--- a/job.c
+++ b/job.c
@@ -396,6 +396,17 @@ Job *job_get(const char *id)
return job_get_locked(id);
}
+void job_set_aio_context(Job *job, AioContext *ctx)
+{
+ /* protect against read in job_finish_sync_locked and job_start */
+ GLOBAL_STATE_CODE();
+ /* protect against read in job_do_yield_locked */
+ JOB_LOCK_GUARD();
+ /* ensure the job is quiescent while the AioContext is changed */
+ assert(job->paused || job_is_completed_locked(job));
+ job->aio_context = ctx;
+}
+
/* Called with job_mutex *not* held. */
static void job_sleep_timer_cb(void *opaque)
{
@@ -1379,6 +1390,7 @@ int job_finish_sync_locked(Job *job,
{
Error *local_err = NULL;
int ret;
+ GLOBAL_STATE_CODE();
job_ref_locked(job);
--
2.31.1
- [PATCH v12 10/21] block/mirror.c: use of job helpers in drivers, (continued)
- [PATCH v12 10/21] block/mirror.c: use of job helpers in drivers, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 03/21] job.c: API functions not used outside should be static, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 02/21] job.h: categorize fields in struct Job, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 09/21] jobs: use job locks also in the unit tests, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 17/21] job.h: categorize JobDriver callbacks that need the AioContext lock, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 16/21] blockjob: protect iostatus field in BlockJob struct, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 19/21] block_job_query: remove atomic read, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 14/21] blockjob.h: categorize fields in struct BlockJob, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 15/21] blockjob: rename notifier callbacks as _locked, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 05/21] job.c: add job_lock/unlock while keeping job.h intact, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 13/21] jobs: protect job.aio_context with BQL and job_mutex,
Emanuele Giuseppe Esposito <=
- [PATCH v12 11/21] jobs: group together API calls under the same job lock, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 12/21] job: detect change of aiocontext within job coroutine, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 20/21] blockjob: remove unused functions, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 21/21] job: remove unused functions, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 18/21] job.c: enable job lock/unlock and remove Aiocontext locks, Emanuele Giuseppe Esposito, 2022/09/26
- [PATCH v12 07/21] blockjob: introduce block_job _locked() APIs, Emanuele Giuseppe Esposito, 2022/09/26