Re: [PATCH v3 09/16] jobs: remove aiocontext locks since the functions are under BQL

[Paolo Bonzini]

On 1/5/22 15:02, Emanuele Giuseppe Esposito wrote:
> In preparation to the job_lock/unlock patch, remove these
> aiocontext locks.
> The main reason these two locks are removed here is because
> they are inside a loop iterating on the jobs list. Once the
> job_lock is added, it will have to protect the whole loop,
> wrapping also the aiocontext acquire/release.
>
> We don't want this, as job_lock can only be *wrapped by*
> the aiocontext lock, and not vice-versa, to avoid deadlocks.

Better to avoid the passive: "must be taken inside the AioContext lock, 
and taking it outside would cause deadlocks".  Also add a note about the 
lock hierarchy to patch 1.

> @@ -3707,15 +3707,11 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp)
>   
>       for (job = block_job_next(NULL); job; job = block_job_next(job)) {
>           BlockJobInfo *value;
> -        AioContext *aio_context;
>   
>           if (block_job_is_internal(job)) {
>               continue;
>           }

block_job_next, block_job_query, etc. do not have the _locked suffix. 
Is this because all block_job_ functions need the job_mutex held, or 
just laziness? :)

Paolo

> -        aio_context = blk_get_aio_context(job->blk);
> -        aio_context_acquire(aio_context);
>           value = block_job_query(job, errp);
> -        aio_context_release(aio_context);
>           if (!value) {
>               qapi_free_BlockJobInfoList(head);
>               return NULL;
> diff --git a/job-qmp.c b/job-qmp.c
> index de4120a1d4..f6f9840436 100644
> --- a/job-qmp.c
> +++ b/job-qmp.c
> @@ -173,15 +173,11 @@ JobInfoList *qmp_query_jobs(Error **errp)
>   
>       for (job = job_next_locked(NULL); job; job = job_next_locked(job)) {
>           JobInfo *value;
> -        AioContext *aio_context;
>   
>           if (job_is_internal(job)) {
>               continue;
>           }
> -        aio_context = job->aio_context;
> -        aio_context_acquire(aio_context);
>           value = job_query_single(job, errp);
> -        aio_context_release(aio_context);
>           if (!value) {
>               qapi_free_JobInfoList(head);
>               return NULL;