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

[Emanuele Giuseppe Esposito]

Am 11/07/2022 um 16:33 schrieb 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 <eespo...@redhat.com>
>> Reviewed-by: Stefan Hajnoczi <stefa...@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?

I'll change it so that job_commit/clean/abort do not take the lock, but
just the aiocontext.

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