qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v12 13/21] jobs: protect job.aio_context with BQL and job_mut


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH v12 13/21] jobs: protect job.aio_context with BQL and job_mutex
Date: Mon, 26 Sep 2022 14:53:06 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0

On 9/26/22 12:32, Emanuele Giuseppe Esposito wrote:
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>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

---
  block/replication.c |  1 +
  blockjob.c          |  3 ++-

[..]

--- 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.

I'd also add "set only by job_set_aio_context()".

+ */
+void job_set_aio_context(Job *job, AioContext *ctx);
+
  #endif


--
Best regards,
Vladimir



reply via email to

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