[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 11/15] jobs: remove aiocontext locks since the functions

From: Vladimir Sementsov-Ogievskiy
Subject: Re: [RFC PATCH 11/15] jobs: remove aiocontext locks since the functions are under BQL
Date: Tue, 2 Nov 2021 15:41:17 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.1.0

Hmm. subject looks like it's safe to remove aiocontext locks from any qmp 

For example, commit 91005a495e228eb added aiocontext locks back to qmp bitmap 
add/remove commands because otherwise they crashed.

So, I'm not sure that being under BQL is enough to drop aiocontext locking..

29.10.2021 19:39, 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.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
  blockdev.c | 4 ----
  job-qmp.c  | 4 ----
  2 files changed, 8 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index d9bf842a81..67b55eec11 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3719,15 +3719,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)) {
-        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) {
              return NULL;
diff --git a/job-qmp.c b/job-qmp.c
index 829a28aa70..a6774aaaa5 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -173,15 +173,11 @@ JobInfoList *qmp_query_jobs(Error **errp)
for (job = job_next(NULL); job; job = job_next(job)) {
          JobInfo *value;
-        AioContext *aio_context;
if (job_is_internal(job)) {
-        aio_context = job->aio_context;
-        aio_context_acquire(aio_context);
          value = job_query_single(job, errp);
-        aio_context_release(aio_context);
          if (!value) {
              return NULL;

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]