All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.