| Message ID | 20220105140208.365608-6-eesposit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | job: replace AioContext lock with job_mutex | expand |
On 1/5/22 15:01, Emanuele Giuseppe Esposito wrote: > > + WITH_JOB_LOCK_GUARD() { > + abort = job->ret < 0; > + } > + > if (s->prepared) { > return 0; > } At this point I think job->ret is stable and can be accessed without guard. The question however is what serializes calls to job_prepare. Is it the BQL? Can we say that job->ret is only written under BQL, just like job->aio_context. > @@ -1161,8 +1165,10 @@ static void mirror_complete(Job *job, Error **errp) > s->should_complete = true; > > /* If the job is paused, it will be re-entered when it is resumed */ > - if (!job->paused) { > - job_enter(job); > + WITH_JOB_LOCK_GUARD() { > + if (!job->paused) { > + job_enter_cond_locked(job, NULL); > + } > } I don't want to open a can of worms, but does it ever make sense to call job_enter while the job is paused? Should this condition be moved to job_enter_cond_locked? Paolo
diff --git a/block/mirror.c b/block/mirror.c index 00089e519b..41450df55c 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -653,9 +653,13 @@ static int mirror_exit_common(Job *job) BlockDriverState *target_bs; BlockDriverState *mirror_top_bs; Error *local_err = NULL; - bool abort = job->ret < 0; + bool abort; int ret = 0; + WITH_JOB_LOCK_GUARD() { + abort = job->ret < 0; + } + if (s->prepared) { return 0; } @@ -1161,8 +1165,10 @@ static void mirror_complete(Job *job, Error **errp) s->should_complete = true; /* If the job is paused, it will be re-entered when it is resumed */ - if (!job->paused) { - job_enter(job); + WITH_JOB_LOCK_GUARD() { + if (!job->paused) { + job_enter_cond_locked(job, NULL); + } } } @@ -1182,8 +1188,11 @@ static bool mirror_drained_poll(BlockJob *job) * from one of our own drain sections, to avoid a deadlock waiting for * ourselves. */ - if (!s->common.job.paused && !job_is_cancelled(&job->job) && !s->in_drain) { - return true; + WITH_JOB_LOCK_GUARD() { + if (!s->common.job.paused && !job_is_cancelled_locked(&job->job) + && !s->in_drain) { + return true; + } } return !!s->in_flight;
Once job lock is used and aiocontext is removed, mirror has to perform job operations under the same critical section, using the helpers prepared in previous commit. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- block/mirror.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)