On Tue, Feb 08, 2022 at 09:34:59AM -0500, 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 must be taken inside the AioContext > lock, and taking it outside would cause deadlocks. > > Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com> > --- > blockdev.c | 4 ---- > job-qmp.c | 4 ---- > 2 files changed, 8 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index 8cac3d739c..e315466914 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -3713,15 +3713,11 @@ BlockJobInfoList *qmp_query_block_jobs(Error **errp) > > for (job = block_job_next(NULL); job; job = block_job_next(job)) {
I'm confused. block_job_next() is supposed to be called with job_mutex held since it iterates the jobs list. The patch series might fix this later on but it's hard to review patches with broken invariants. Does this mean git-bisect(1) is broken since intermediate commits are not thread-safe? > BlockJobInfo *value; > - AioContext *aio_context; > > if (block_job_is_internal(job)) { > continue; > } > - aio_context = block_job_get_aio_context(job); > - aio_context_acquire(aio_context); > value = block_job_query(job, errp); This function accesses fields that are protected by job_mutex, which we don't hold.
signature.asc
Description: PGP signature