Message ID | 20211029163914.4044794-3-eesposit@redhat.com |
---|---|
State | New |
Headers | show |
Series | job: replace AioContext lock with job_mutex | expand |
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 --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); }
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(-)