[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 09/20] jobs: add job lock in find_* functions
From: |
Emanuele Giuseppe Esposito |
Subject: |
Re: [PATCH v5 09/20] jobs: add job lock in find_* functions |
Date: |
Thu, 24 Feb 2022 13:36:12 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
On 17/02/2022 16:00, Stefan Hajnoczi wrote:
> On Tue, Feb 08, 2022 at 09:35:02AM -0500, Emanuele Giuseppe Esposito wrote:
>> diff --git a/blockdev.c b/blockdev.c
>> index c5fba4d157..08408cd44b 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -3311,7 +3311,10 @@ out:
>> aio_context_release(aio_context);
>> }
>>
>> -/* Get a block job using its ID and acquire its AioContext */
>> +/*
>> + * Get a block job using its ID and acquire its AioContext.
>> + * Returns with job_lock held on success.
>
> The caller needs to deal with unlocking anyway, so maybe ask the caller
> to acquire the lock too? That would make the function simpler to reason
> about.
Those aiocontext locks there are going to be removed when job
lock/unlock are enabled, so it is useless to modify the logic now.
It makes sense to apply this logic with job_lock/unlock though.
>
>> @@ -60,6 +65,7 @@ void qmp_job_cancel(const char *id, Error **errp)
>> trace_qmp_job_cancel(job);
>> job_user_cancel(job, true, errp);
>> aio_context_release(aio_context);
>> + job_unlock();
>> }
>
> Is job_mutex -> AioContext lock ordering correct? I thought the
> AioContext must be held before taking the job lock according to "jobs:
> remove aiocontext locks since the functions are under BQL"?
>
I think you got it at this point, but anyways: job_lock is nop and when
it will be enabled, aio_context_acquire will go away in the same patch.
Thank you,
Emanuele
- [PATCH v5 00/20] job: replace AioContext lock with job_mutex, Emanuele Giuseppe Esposito, 2022/02/08
- [PATCH v5 01/20] job.c: make job_mutex and job_lock/unlock() public, Emanuele Giuseppe Esposito, 2022/02/08
- [PATCH v5 09/20] jobs: add job lock in find_* functions, Emanuele Giuseppe Esposito, 2022/02/08
- [PATCH v5 03/20] job.c: API functions not used outside should be static, Emanuele Giuseppe Esposito, 2022/02/08
- [PATCH v5 04/20] job.c: move inner aiocontext lock in callbacks, Emanuele Giuseppe Esposito, 2022/02/08
- [PATCH v5 02/20] job.h: categorize fields in struct Job, Emanuele Giuseppe Esposito, 2022/02/08