Am 17/08/2022 um 20:54 schrieb Vladimir Sementsov-Ogievskiy:
> On 8/16/22 15:52, Emanuele Giuseppe Esposito wrote:
>>>> }
>>>> @@ -501,8 +481,12 @@ void job_unref_locked(Job *job)>
>>>> assert(!job->txn);
>>>> if (job->driver->free) {
>>>> + AioContext *aio_context = job->aio_context;
>>>> job_unlock();
>>>> + /* FIXME: aiocontext lock is required because cb calls
>>>> blk_unref */
>>>> + aio_context_acquire(aio_context);
>>>> job->driver->free(job);
>>>> + aio_context_release(aio_context);
>>> So, job_unref_locked() must be called with aio_context not locked,
>>> otherwise we dead-lock here? That should be documented in function
>>> declaration comment.
>>>
>>> For example in qemu-img.c in run_block_job() we do exactly that: call
>>> job_unref_locked() inside aio-context lock critical seaction..
>> No, job_unref_locked has also status and refcnt and all the other fields
>> that need to be protectd. Only free must be called without job lock held.
>>
>
> I mean, don't we try here to acquire aio_context twice, when we call
> job_unref_locked() with aio_context _already_ acquired?
>
You're right, so why do we need the AioContext lock in qemu-img then?
All jobs API don't need it, aio_poll seems not to need it either, and
progress API has its own lock.
I guess I could remove it?
Emanuele
