Re: [PATCH v9 15/21] job.c: enable job lock/unlock and remove Aiocontext locks

[Vladimir Sementsov-Ogievskiy]

On 7/6/22 23:15, Emanuele Giuseppe Esposito wrote:
> Change the job_{lock/unlock} and macros to use job_mutex.
>
> Now that they are not nop anymore, remove the aiocontext
> to avoid deadlocks.
>
> Therefore:
> - when possible, remove completely the aiocontext lock/unlock pair
> - if it is used by some other function too, reduce the locking
>    section as much as possible, leaving the job API outside.
> - change AIO_WAIT_WHILE in AIO_WAIT_WHILE_UNLOCKED, since we
>    are not using the aiocontext lock anymore
>
> There is only one JobDriver callback, ->free() that assumes that
> the aiocontext lock is held (because it calls bdrv_unref), so for
> now keep that under aiocontext lock.
>
> Also remove real_job_{lock/unlock}, as they are replaced by the
> public functions.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---

[..]

>   static int job_finalize_single_locked(Job *job)
>   {
>       int job_ret;
> +    AioContext *ctx = job->aio_context;
>   
>       assert(job_is_completed_locked(job));
>   
>       /* Ensure abort is called for late-transactional failures */
>       job_update_rc_locked(job);
>   
> +    aio_context_acquire(ctx);

so here we acquire aio-context under job_mutex lock? Wouldn't that dead-lock?

> +
>       if (!job->ret) {
>           job_commit_locked(job);
>       } else {



--
Best regards,
Vladimir