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
- Re: [PATCH v5 04/20] job.c: move inner... Emanuele Giuseppe Esposito
- [PATCH v5 01/20] job.c: make job_mutex and job_... Emanuele Giuseppe Esposito
- Re: [PATCH v5 01/20] job.c: make job_mutex... Stefan Hajnoczi
- [PATCH v5 02/20] job.h: categorize fields in st... Emanuele Giuseppe Esposito
- Re: [PATCH v5 02/20] job.h: categorize fie... Stefan Hajnoczi
- Re: [PATCH v5 02/20] job.h: categorize... Emanuele Giuseppe Esposito
- Re: [PATCH v5 02/20] job.h: catego... Stefan Hajnoczi
- Re: [PATCH v5 02/20] job.h: ca... Emanuele Giuseppe Esposito
- [PATCH v5 09/20] jobs: add job lock in find_* f... Emanuele Giuseppe Esposito
- Re: [PATCH v5 09/20] jobs: add job lock in... Stefan Hajnoczi
- Re: [PATCH v5 09/20] jobs: add job loc... Emanuele Giuseppe Esposito
- [PATCH v5 05/20] aio-wait.h: introduce AIO_WAIT... Emanuele Giuseppe Esposito
- Re: [PATCH v5 05/20] aio-wait.h: introduce... Stefan Hajnoczi
- [PATCH v5 10/20] jobs: use job locks also in th... Emanuele Giuseppe Esposito
- [PATCH v5 11/20] block/mirror.c: use of job hel... Emanuele Giuseppe Esposito
- Re: [PATCH v5 11/20] block/mirror.c: use o... Stefan Hajnoczi
- [PATCH v5 12/20] jobs: rename static functions ... Emanuele Giuseppe Esposito
- [PATCH v5 18/20] jobs: protect job.aio_context ... Emanuele Giuseppe Esposito
- Re: [PATCH v5 18/20] jobs: protect job.aio... Stefan Hajnoczi
- Re: [PATCH v5 18/20] jobs: protect job... Emanuele Giuseppe Esposito
- [PATCH v5 15/20] job.h: define unlocked functio... Emanuele Giuseppe Esposito