[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 2/6] job: _locked functions and public job_lock/unlock fo
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC PATCH 2/6] job: _locked functions and public job_lock/unlock for next patch |
Date: |
Thu, 8 Jul 2021 11:50:16 +0100 |
On Wed, Jul 07, 2021 at 06:58:09PM +0200, Emanuele Giuseppe Esposito wrote:
> diff --git a/job.c b/job.c
> index 872bbebb01..96fb8e9730 100644
> --- a/job.c
> +++ b/job.c
> @@ -32,6 +32,10 @@
> #include "trace/trace-root.h"
> #include "qapi/qapi-events-job.h"
>
> +/* job_mutex protexts the jobs list, but also the job operations. */
> +static QemuMutex job_mutex;
It's unclear what protecting "job operations" means. I would prefer a
fine-grained per-job lock that protects the job's fields instead of a
global lock with an unclear scope.
> +
> +/* Protected by job_mutex */
> static QLIST_HEAD(, Job) jobs = QLIST_HEAD_INITIALIZER(jobs);
>
> /* Job State Transition Table */
> @@ -64,27 +68,22 @@ bool JobVerbTable[JOB_VERB__MAX][JOB_STATUS__MAX] = {
> /* Transactional group of jobs */
> struct JobTxn {
>
> - /* Is this txn being cancelled? */
> + /* Is this txn being cancelled? Atomic.*/
> bool aborting;
The comment says atomic but this field is not accessed using atomic
operations (at least at this point in the patch series)?
>
> - /* List of jobs */
> + /* List of jobs. Protected by job_mutex. */
> QLIST_HEAD(, Job) jobs;
>
> - /* Reference count */
> + /* Reference count. Atomic. */
> int refcnt;
Same.
signature.asc
Description: PGP signature