qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Fam Zheng <fam@euphon.net>,
	Wen Congyang <wencongyang2@huawei.com>,
	Xie Changlong <xiechanglong.d@gmail.com>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org, Hanna Reitz <hreitz@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [RFC PATCH 11/15] jobs: remove aiocontext locks since the functions are under BQL
Date: Wed, 3 Nov 2021 16:56:56 +0100	[thread overview]
Message-ID: <2bc317aa-098e-67c6-013d-e661d292d1d1@redhat.com> (raw)
In-Reply-To: <a19af72b-3b81-7181-15ed-2bbaa50779fd@virtuozzo.com>



On 02/11/2021 13:41, Vladimir Sementsov-Ogievskiy wrote:
> Hmm. subject looks like it's safe to remove aiocontext locks from any 
> qmp command?
> 
> 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..
> 
> 

In this specific case the aiocontext is useless there. I am not sure 
what it was used for before, but if you look at what it protects in this 
patch we have:

- block_job_query: not called by anyone else, and internally calls:
	* block_job_is_internal = checks struct job id, that is only written at 
job initialization
	* progress_get_snapshot = we made it thread safe, as you might remember
	* accesses struct job fields, that we protect anyways with job_mutex. 
Anyways the aiocontext is not used to protect jobs fields.

- job_query_single: same as block_job_query, calls progress_get_snapshot 
and accesses job fields.

So yes, I am positive that here removing the aiocontext lock does not 
break anything.

Thank you,
Emanuele



  reply	other threads:[~2021-11-03 15:59 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-29 16:38 [RFC PATCH 00/15] job: replace AioContext lock with job_mutex Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 01/15] jobs: add job-common.h Emanuele Giuseppe Esposito
2021-11-02 10:07   ` Stefan Hajnoczi
2021-10-29 16:39 ` [RFC PATCH 02/15] job.c: make job_lock/unlock public Emanuele Giuseppe Esposito
2021-11-02 10:10   ` Stefan Hajnoczi
2021-10-29 16:39 ` [RFC PATCH 03/15] job-common.h: categorize fields in struct Job Emanuele Giuseppe Esposito
2021-11-02 10:19   ` Stefan Hajnoczi
2021-10-29 16:39 ` [RFC PATCH 04/15] jobs: add job-monitor.h Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 05/15] job-monitor.h: define the job monitor API Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 06/15] jobs: add job-driver.h Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 07/15] job-driver.h: add helper functions Emanuele Giuseppe Esposito
2021-11-02 10:54   ` Vladimir Sementsov-Ogievskiy
2021-10-29 16:39 ` [RFC PATCH 08/15] job.c: minor adjustments in preparation to job-driver Emanuele Giuseppe Esposito
2021-11-02 10:51   ` Vladimir Sementsov-Ogievskiy
2021-10-29 16:39 ` [RFC PATCH 09/15] job.c: move inner aiocontext lock in callbacks Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 10/15] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 11/15] jobs: remove aiocontext locks since the functions are under BQL Emanuele Giuseppe Esposito
2021-11-02 12:41   ` Vladimir Sementsov-Ogievskiy
2021-11-03 15:56     ` Emanuele Giuseppe Esposito [this message]
2021-10-29 16:39 ` [RFC PATCH 12/15] jobs: protect jobs with job_lock/unlock Emanuele Giuseppe Esposito
2021-11-02 12:53   ` Vladimir Sementsov-Ogievskiy
2021-10-29 16:39 ` [RFC PATCH 13/15] jobs: use job locks and helpers also in the unit tests Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 14/15] jobs: add missing job locks to replace aiocontext lock Emanuele Giuseppe Esposito
2021-10-29 16:39 ` [RFC PATCH 15/15] jobs: remove all unnecessary AioContext locks Emanuele Giuseppe Esposito
2021-11-02 10:06 ` [RFC PATCH 00/15] job: replace AioContext lock with job_mutex Stefan Hajnoczi
2021-11-02 13:08 ` Vladimir Sementsov-Ogievskiy
2021-11-02 14:13   ` Emanuele Giuseppe Esposito
2021-11-02 14:58     ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2bc317aa-098e-67c6-013d-e661d292d1d1@redhat.com \
    --to=eesposit@redhat.com \
    --cc=armbru@redhat.com \
    --cc=fam@euphon.net \
    --cc=hreitz@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    --cc=wencongyang2@huawei.com \
    --cc=xiechanglong.d@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).