diff mbox series

[RFC,02/15] job.c: make job_lock/unlock public

Message ID 20211029163914.4044794-3-eesposit@redhat.com (mailing list archive)
State New, archived
Headers show
Series job: replace AioContext lock with job_mutex | expand

Commit Message

Emanuele Giuseppe Esposito Oct. 29, 2021, 4:39 p.m. UTC
job mutex will be used to protect the job struct elements and list,
replacing AioContext locks.

Right now use a shared lock for all jobs, in order to keep things
simple. Once the AioContext lock is gone, we can introduce per-job
locks.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 include/qemu/job-common.h | 18 ++++++++++++++++++
 job.c                     | 12 +++++-------
 2 files changed, 23 insertions(+), 7 deletions(-)

Comments

Stefan Hajnoczi Nov. 2, 2021, 10:10 a.m. UTC | #1
On Fri, Oct 29, 2021 at 12:39:01PM -0400, Emanuele Giuseppe Esposito wrote:
> job mutex will be used to protect the job struct elements and list,
> replacing AioContext locks.
> 
> Right now use a shared lock for all jobs, in order to keep things
> simple. Once the AioContext lock is gone, we can introduce per-job
> locks.
> 
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>  include/qemu/job-common.h | 18 ++++++++++++++++++
>  job.c                     | 12 +++++-------
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/include/qemu/job-common.h b/include/qemu/job-common.h
> index c115028e33..dcc24fba48 100644
> --- a/include/qemu/job-common.h
> +++ b/include/qemu/job-common.h
> @@ -297,4 +297,22 @@ typedef enum JobCreateFlags {
>      JOB_MANUAL_DISMISS = 0x04,
>  } JobCreateFlags;
>  
> +/**
> + * job_lock:
> + *
> + * Take the mutex protecting the list of jobs and their status.
> + * Most functions called by the monitor need to call job_lock
> + * and job_unlock manually.  On the other hand, function called

s/function/functions/

> diff --git a/job.c b/job.c
> index 94b142684f..e003f136f0 100644
> --- a/job.c
> +++ b/job.c
> @@ -32,6 +32,9 @@
>  #include "trace/trace-root.h"
>  #include "qapi/qapi-events-job.h"
>  
> +/* job_mutex protects the jobs list, but also makes the job API thread-safe. */

"but also makes the job API thread-safe" is vague. Are the jobs list and
the Job objects all protected by job_mutex? Stating that would be
clearer.

Otherwise:

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff mbox series

Patch

diff --git a/include/qemu/job-common.h b/include/qemu/job-common.h
index c115028e33..dcc24fba48 100644
--- a/include/qemu/job-common.h
+++ b/include/qemu/job-common.h
@@ -297,4 +297,22 @@  typedef enum JobCreateFlags {
     JOB_MANUAL_DISMISS = 0x04,
 } JobCreateFlags;
 
+/**
+ * job_lock:
+ *
+ * Take the mutex protecting the list of jobs and their status.
+ * Most functions called by the monitor need to call job_lock
+ * and job_unlock manually.  On the other hand, function called
+ * by the block jobs themselves and by the block layer will take the
+ * lock for you.
+ */
+void job_lock(void);
+
+/**
+ * job_unlock:
+ *
+ * Release the mutex protecting the list of jobs and their status.
+ */
+void job_unlock(void);
+
 #endif
diff --git a/job.c b/job.c
index 94b142684f..e003f136f0 100644
--- a/job.c
+++ b/job.c
@@ -32,6 +32,9 @@ 
 #include "trace/trace-root.h"
 #include "qapi/qapi-events-job.h"
 
+/* job_mutex protects the jobs list, but also makes the job API thread-safe. */
+static QemuMutex job_mutex;
+
 static QLIST_HEAD(, Job) jobs = QLIST_HEAD_INITIALIZER(jobs);
 
 /* Job State Transition Table */
@@ -74,17 +77,12 @@  struct JobTxn {
     int refcnt;
 };
 
-/* Right now, this mutex is only needed to synchronize accesses to job->busy
- * and job->sleep_timer, such as concurrent calls to job_do_yield and
- * job_enter. */
-static QemuMutex job_mutex;
-
-static void job_lock(void)
+void job_lock(void)
 {
     qemu_mutex_lock(&job_mutex);
 }
 
-static void job_unlock(void)
+void job_unlock(void)
 {
     qemu_mutex_unlock(&job_mutex);
 }