These functions don't need a _locked() counterpart, since they are all called outside job.c and take the lock only internally.
Update also the comments in blockjob.c (and move them in job.c). Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com> --- blockjob.c | 20 -------------------- include/qemu/job.h | 37 ++++++++++++++++++++++++++++++++++--- job.c | 15 +++++++++++++++ 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/blockjob.c b/blockjob.c index 4868453d74..7da59a1f1c 100644 --- a/blockjob.c +++ b/blockjob.c @@ -36,21 +36,6 @@ #include "qemu/main-loop.h" #include "qemu/timer.h" -/* - * The block job API is composed of two categories of functions. - * - * The first includes functions used by the monitor. The monitor is - * peculiar in that it accesses the block job list with block_job_get, and - * therefore needs consistency across block_job_get and the actual operation - * (e.g. block_job_set_speed). The consistency is achieved with - * aio_context_acquire/release. These functions are declared in blockjob.h. - * - * The second includes functions used by the block job drivers and sometimes - * by the core block layer. These do not care about locking, because the - * whole coroutine runs under the AioContext lock, and are declared in - * blockjob_int.h. - */ - static bool is_block_job(Job *job) { return job_type(job) == JOB_TYPE_BACKUP || @@ -433,11 +418,6 @@ static void block_job_event_ready(Notifier *n, void *opaque) } -/* - * API for block job drivers and the block layer. These functions are - * declared in blockjob_int.h. - */ - void *block_job_create(const char *job_id, const BlockJobDriver *driver, JobTxn *txn, BlockDriverState *bs, uint64_t perm, uint64_t shared_perm, int64_t speed, int flags, diff --git a/include/qemu/job.h b/include/qemu/job.h index 99960cc9a3..b714236c1a 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -363,6 +363,7 @@ void job_txn_unref_locked(JobTxn *txn); /** * Create a new long-running job and return it. + * Called with job_mutex *not* held. * * @job_id: The id of the newly-created job, or %NULL for internal jobs * @driver: The class object for the newly-created job. @@ -400,6 +401,8 @@ void job_unref_locked(Job *job); * @done: How much progress the job made since the last call * * Updates the progress counter of the job. + * + * Progress API is thread safe. */ void job_progress_update(Job *job, uint64_t done); @@ -410,6 +413,8 @@ void job_progress_update(Job *job, uint64_t done); * * Sets the expected end value of the progress counter of a job so that a * completion percentage can be calculated when the progress is updated. + * + * Progress API is thread safe. */ void job_progress_set_remaining(Job *job, uint64_t remaining); @@ -425,6 +430,8 @@ void job_progress_set_remaining(Job *job, uint64_t remaining); * length before, and job_progress_update() afterwards. * (So the operation acts as a parenthesis in regards to the main job * operation running in background.) + * + * Progress API is thread safe. */ void job_progress_increase_remaining(Job *job, uint64_t delta); @@ -443,6 +450,8 @@ void job_enter_cond_locked(Job *job, bool(*fn)(Job *job)); * * Begins execution of a job. * Takes ownership of one reference to the job object. + * + * Called with job_mutex *not* held. */ void job_start(Job *job); @@ -450,6 +459,7 @@ void job_start(Job *job); * @job: The job to enter. * * Continue the specified job by entering the coroutine. + * Called with job_mutex *not* held. */ void job_enter(Job *job); @@ -458,6 +468,9 @@ void job_enter(Job *job); * * Pause now if job_pause() has been called. Jobs that perform lots of I/O * must call this between requests so that the job can be paused. + * + * Called with job_mutex *not* held (we don't want the coroutine + * to yield with the lock held!). */ void coroutine_fn job_pause_point(Job *job); @@ -465,6 +478,8 @@ void coroutine_fn job_pause_point(Job *job); * @job: The job that calls the function. * * Yield the job coroutine. + * Called with job_mutex *not* held (we don't want the coroutine + * to yield with the lock held!). */ void job_yield(Job *job); @@ -475,6 +490,9 @@ void job_yield(Job *job); * Put the job to sleep (assuming that it wasn't canceled) for @ns * %QEMU_CLOCK_REALTIME nanoseconds. Canceling the job will immediately * interrupt the wait. + * + * Called with job_mutex *not* held (we don't want the coroutine + * to yield with the lock held!). */ void coroutine_fn job_sleep_ns(Job *job, int64_t ns); @@ -496,6 +514,7 @@ bool job_is_cancelled_locked(Job *job); /** * Returns whether the job is scheduled for cancellation (at an * indefinite point). + * Called with job_mutex *not* held. */ bool job_cancel_requested(Job *job); @@ -582,10 +601,16 @@ int job_apply_verb(Job *job, JobVerb verb, Error **errp); /* Same as job_apply_verb, but called with job lock held. */ int job_apply_verb_locked(Job *job, JobVerb verb, Error **errp); -/** The @job could not be started, free it. */ +/** + * The @job could not be started, free it. + * Called with job_mutex *not* held. + */ void job_early_fail(Job *job); -/** Moves the @job from RUNNING to READY */ +/** + * Moves the @job from RUNNING to READY. + * Called with job_mutex *not* held. + */ void job_transition_to_ready(Job *job); /** Asynchronously complete the specified @job. */ @@ -628,7 +653,13 @@ int job_cancel_sync(Job *job, bool force); /* Same as job_cancel_sync, but called with job lock held. */ int job_cancel_sync_locked(Job *job, bool force); -/** Synchronously force-cancels all jobs using job_cancel_sync(). */ +/** + * Synchronously force-cancels all jobs using job_cancel_sync_locked(). + * + * Called with job_lock *not* held, unlike most other APIs consumed + * by the monitor! This is primarly to avoid adding unnecessary lock-unlock + * patterns in the caller. + */ void job_cancel_sync_all(void); /** diff --git a/job.c b/job.c index dd44fac8dd..7a3cc93f66 100644 --- a/job.c +++ b/job.c @@ -32,12 +32,27 @@ #include "trace/trace-root.h" #include "qapi/qapi-events-job.h" +/* + * The job API is composed of two categories of functions. + * + * The first includes functions used by the monitor. The monitor is + * peculiar in that it accesses the block job list with job_get, and + * therefore needs consistency across job_get and the actual operation + * (e.g. job_user_cancel). To achieve this consistency, the caller + * calls job_lock/job_unlock itself around the whole operation. + * + * + * The second includes functions used by the block job drivers and sometimes + * by the core block layer. These delegate the locking to the callee instead. + */ + /* * job_mutex protects the jobs list, but also makes the * struct job fields thread-safe. */ QemuMutex job_mutex; +/* Protected by job_mutex */ static QLIST_HEAD(, Job) jobs = QLIST_HEAD_INITIALIZER(jobs); /* Job State Transition Table */ -- 2.31.1