Re: [PATCH v7 10/18] jobs: rename static functions called with job_mutex held

Am 24/06/2022 um 17:28 schrieb Paolo Bonzini: > On 6/24/22 16:29, Kevin Wolf wrote: >> Yes, I think Vladimir is having the same difficulties with reading the >> series as I had. And I believe his suggestion would make the >> intermediate states less impossible to review. The question is how much

Re: [PATCH v7 10/18] jobs: rename static functions called with job_mutex held

Am 23/06/2022 um 13:10 schrieb Vladimir Sementsov-Ogievskiy: > On 6/23/22 12:08, Emanuele Giuseppe Esposito wrote: >> >> >> Am 22/06/2022 um 20:38 schrieb Vladimir Sementsov-Ogievskiy: >>> On 6/22/22 17:26, Emanuele Giuseppe Esposito wrote: >>>> >>

Re: [PATCH v7 10/18] jobs: rename static functions called with job_mutex held

Am 22/06/2022 um 20:38 schrieb Vladimir Sementsov-Ogievskiy: > On 6/22/22 17:26, Emanuele Giuseppe Esposito wrote: >> >> >> Am 21/06/2022 um 19:26 schrieb Vladimir Sementsov-Ogievskiy: >>> On 6/16/22 16:18, Emanuele Giuseppe Esposito wrote: >>>> With th

Re: [PATCH v7 05/18] job.h: add _locked duplicates for job API functions called with and without job_mutex

Am 21/06/2022 um 17:03 schrieb Vladimir Sementsov-Ogievskiy: > On 6/16/22 16:18, Emanuele Giuseppe Esposito wrote: >> In preparation to the job_lock/unlock usage, create _locked >> duplicates of some functions, since they will be sometimes called with >> job_mutex held

Re: [PATCH v7 10/18] jobs: rename static functions called with job_mutex held

Am 21/06/2022 um 19:26 schrieb Vladimir Sementsov-Ogievskiy: > On 6/16/22 16:18, Emanuele Giuseppe Esposito wrote: >> With the*nop*  job_lock/unlock placed, rename the static >> functions that are always under job_mutex, adding "_locked" suffix. >> >> L

[PATCH v7 13/18] job.h: define unlocked functions

in job.c). Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 20 include/qemu/job.h | 36 +--- job.c

[PATCH v7 11/18] job.h: rename job API functions called with job_mutex held

miss Note that "locked" refers to the *nop* job_lock/unlock, and not real_job_lock/unlock. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 +- block/mirror.c | 2 +- block/replication.

[PATCH v7 09/18] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU

Once job lock is used and aiocontext is removed, mirror has to perform job operations under the same critical section, using the helpers prepared in previous commit. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by

[PATCH v7 10/18] jobs: rename static functions called with job_mutex held

lock. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 247 +- 1 file changed, 141 insertions(+), 106 deletions(-) diff --git a/job.c b/job.c index 55b92b2332..4f4b387625 100644 --- a/job.c +++ b/jo

[PATCH v7 05/18] job.h: add _locked duplicates for job API functions called with and without job_mutex

) job_cancel_requested (_locked version is static) Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 25 +--- job.c | 48

[PATCH v7 03/18] job.c: API functions not used outside should be static

job_event_* functions can all be static, as they are not used outside job.c. Same applies for job_txn_add_job(). Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 18 -- job.c | 22 +++--- 2 files

[PATCH v7 15/18] job: detect change of aiocontext within job coroutine

From: Paolo Bonzini We want to make sure access of job->aio_context is always done under either BQL or job_mutex. The problem is that using aio_co_enter(job->aiocontext, job->co) in job_start and job_enter_cond makes the coroutine immediately resume, so we can't hold the job lock. And caching it

[PATCH v7 16/18] jobs: protect job.aio_context with BQL and job_mutex

introduce job_set_aio_context and make sure that the context is set under BQL, job_mutex and drain. Also make sure all other places where the aiocontext is read are protected. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Suggested-by: Paolo Bonzini Signed-off-by: Emanuel

[PATCH v7 00/18] job: replace AioContext lock with job_mutex

ssages * job API split patches are sent separately in another series * use of empty job_{lock/unlock} and JOB_LOCK_GUARD/WITH_JOB_LOCK_GUARD to avoid deadlocks and simplify the reviewer job * move patch 11 (block_job_query: remove atomic read) as last Emanuele Giuseppe Esposito (17): job.c: ma

[PATCH v7 17/18] job.c: enable job lock/unlock and remove Aiocontext locks

ned-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 72 - include/qemu/job.h | 19 +++ job-qmp.c| 44 +++ job.c| 92 ++-- tests/unit/test-b

[PATCH v7 08/18] jobs: use job locks also in the unit tests

Add missing job synchronization in the unit tests, with explicit locks. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 76 - tests/unit/test-block-iothread.c

[PATCH v7 06/18] jobs: protect jobs with job_lock/unlock

macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 18 --- block/replication.c | 8 ++- blockdev.c | 17 -- blockjob.c | 56 +--- job-qmp.c | 2 + job.c | 125

[PATCH v7 18/18] block_job_query: remove atomic read

Not sure what the atomic here was supposed to do, since job.busy is protected by the job lock. Since the whole function is called under job_mutex, just remove the atomic. Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 2 +- 1 file changed, 1 insertion

[PATCH v7 01/18] job.c: make job_mutex and job_lock/unlock() public

e can change the nop into an actual mutex and remove the aiocontext lock. Since job_mutex is already being used, add static real_job_{lock/unlock} for the existing usage. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- include/qemu/job.h

[PATCH v7 12/18] block_job: rename block_job functions called with job_mutex held

Just as for the job API, rename block_job functions that are always called under job lock. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 3 ++- block/backup.c | 4 ++-- blockdev.c | 12 +++- blockjob.c

[PATCH v7 14/18] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext

We are always using the given bs AioContext, so there is no need to take the job ones (which is identical anyways). This also reduces the point we need to check when protecting job.aio_context field. Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- block/commit.c | 4

[PATCH v7 04/18] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED

Same as AIO_WAIT_WHILE macro, but if we are in the Main loop do not release and then acquire ctx_ 's aiocontext. Once all Aiocontext locks go away, this macro will replace AIO_WAIT_WHILE. Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- include/block/aio-wait.h

[PATCH v7 07/18] jobs: add job lock in find_* functions

*. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 46 +++--- job-qmp.c | 37 + 2 files changed, 64 insertions(+), 19 deletions(-) diff --git a/blockdev.c b/blockdev.c index b1099e678c..6f83783f10 100644 --- a

[PATCH v7 02/18] job.h: categorize fields in struct Job

Categorize the fields in struct Job to understand which ones need to be protected by the job mutex and which don't. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 61 +++--- 1 file changed, 36 insertions(+), 25 deletions(-) diff

[PATCH 8/8] virtio-blk: remove unnecessary AioContext lock from function already safe

. Signed-off-by: Emanuele Giuseppe Esposito --- hw/block/virtio-blk.c | 18 ++ 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 88c61457e1..ce8efd8381 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c

[PATCH 3/8] virtio_blk_process_queued_requests: always run in a bh

blk_set_aio_context fails, and we still need to process the requests. Since the logic of the bh is exactly the same as virtio_blk_dma_restart, so rename the function and make it public so that we can utilize it here too. Signed-off-by: Emanuele Giuseppe Esposito --- hw/block/dataplane/virtio-blk.c | 10

[PATCH 0/8] virtio-blk: removal of AioContext lock

if we know that the main loop is not interfering, we are safe. However, if we want to consider multiple iothreads concurrently running, we need to take additional precautions. A good example of the above is in patch 7. Signed-off-by: Emanuele Giuseppe Esposito Emanuele Giuseppe Esposito (8

[PATCH 5/8] virtio-blk: mark GLOBAL_STATE_CODE functions

Just as done in the block API, mark functions in virtio-blk that are always called in the main loop with BQL held. We know such functions are GS because they all are callbacks from virtio.c API that has already classified them as GS. Signed-off-by: Emanuele Giuseppe Esposito --- hw/block

[PATCH 6/8] virtio-blk: mark IO_CODE functions

, itself is categorized as IO_CODE too). Signed-off-by: Emanuele Giuseppe Esposito --- hw/block/dataplane/virtio-blk.c | 4 hw/block/virtio-blk.c | 35 + 2 files changed, 39 insertions(+) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block

[PATCH 4/8] virtio: categorize callbacks in GS

yet. Signed-off-by: Emanuele Giuseppe Esposito --- hw/block/virtio-blk.c | 2 ++ hw/virtio/virtio-bus.c | 5 + hw/virtio/virtio-pci.c | 2 ++ hw/virtio/virtio.c | 8 4 files changed, 17 insertions(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 29a9c53ebc

[PATCH 1/8] virtio_queue_aio_attach_host_notifier: remove AioContext lock

scheduled (thus serialized) by the main loop. Therefore removing the AioContext lock is safe, but unfortunately we can't do it right now since bdrv_set_aio_context() and aio_wait_bh_oneshot() still need to have it. Signed-off-by: Emanuele Giuseppe Esposito --- hw/block/dataplane/virtio-blk.c

[PATCH 1/2] linux-aio: use LinuxAioState from the running thread

From: Paolo Bonzini Remove usage of aio_context_acquire by always submitting asynchronous AIO to the current thread's LinuxAioState. Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/file-posix.c | 3 ++- block/linux-aio.c | 13 ++--- include/

[PATCH 7/8] VirtIOBlock: protect rq with its own lock

st. Signed-off-by: Emanuele Giuseppe Esposito --- hw/block/virtio-blk.c | 38 ++ include/hw/virtio/virtio-blk.h | 5 - 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index e1aaa606ba..88

[PATCH 2/8] block-backend: enable_write_cache should be atomic

It is read from IO_CODE and written with BQL held, so setting it as atomic should be enough. Also remove the aiocontext lock that was sporadically taken around the set. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 6 +++--- hw/block/virtio-blk.c | 4 2 files

[PATCH 2/2] thread-pool: use ThreadPool from the running thread

Remove usage of aio_context_acquire by always submitting work items to the current thread's ThreadPool. Signed-off-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/file-posix.c| 19 +-- block/file-win32.c| 2 +- block/qcow2-threads.c | 2 +-

[PATCH 0/2] AioContext removal: LinuxAioState and ThreadPool

Just remove some AioContext lock in LinuxAioState and ThreadPool. Not related to anything specific, so I decided to send it as a separate patch. These patches are taken from Paolo's old draft series. Emanuele Giuseppe Esposito (1): thread-pool: use ThreadPool from the running thread

[PATCH] main loop: add missing documentation links to GS/IO macros

hat refers to such documentation. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/main-loop.h | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h index 5518845299..c50d1b7e3a 100644 --- a/include

Re: [PATCH v6 02/18] job.h: categorize fields in struct Job

Am 07/06/2022 um 17:41 schrieb Paolo Bonzini: > On 6/7/22 15:20, Emanuele Giuseppe Esposito wrote: >> >> >> Am 03/06/2022 um 18:00 schrieb Kevin Wolf: >>> Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben: >>>> Categorize the fie

Re: [PATCH v6 15/18] job: detect change of aiocontext within job coroutine

Am 03/06/2022 um 18:59 schrieb Kevin Wolf: > Am 14.03.2022 um 14:37 hat Emanuele Giuseppe Esposito geschrieben: >> From: Paolo Bonzini >> >> We want to make sure access of job->aio_context is always done >> under either BQL or job_mutex. The problem is

Re: [PATCH v6 02/18] job.h: categorize fields in struct Job

Am 03/06/2022 um 18:00 schrieb Kevin Wolf: > Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben: >> Categorize the fields in struct Job to understand which ones >> need to be protected by the job mutex and which don't. >> >> Signed-off-by: Em

Re: [PATCH v6 05/18] job.h: add _locked duplicates for job API functions called with and without job_mutex

Am 03/06/2022 um 18:17 schrieb Kevin Wolf: > Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben: >> In preparation to the job_lock/unlock usage, create _locked >> duplicates of some functions, since they will be sometimes called with >> job_mutex held

Re: [PATCH v6 06/18] jobs: protect jobs with job_lock/unlock

Am 03/06/2022 um 18:40 schrieb Kevin Wolf: > Am 14.03.2022 um 14:36 hat Emanuele Giuseppe Esposito geschrieben: >> Introduce the job locking mechanism through the whole job API, >> following the comments in job.h and requirements of job-monitor >> (like the functions in j

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 24/05/2022 um 14:10 schrieb Kevin Wolf: > Am 18.05.2022 um 14:28 hat Emanuele Giuseppe Esposito geschrieben: >> label: // read till the end to see why I wrote this here >> >> I was hoping someone from the "No" party would answer to your question, >> bec

Re: aio_wait_bh_oneshot() thread-safety question

Am 24/05/2022 um 09:08 schrieb Paolo Bonzini: > On 5/23/22 18:04, Vladimir Sementsov-Ogievskiy wrote: >> >> I have a doubt about how aio_wait_bh_oneshot() works. Exactly, I see >> that data->done is not accessed atomically, and doesn't have any >> barrier protecting it.. >> >> Is following possi

[PATCH] aio_wait_kick: add missing memory barrier

ation. Suggested-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- include/block/aio-wait.h | 2 ++ util/aio-wait.c | 16 +++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h index b39eefb38d..54840

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 23/05/2022 um 15:15 schrieb Stefan Hajnoczi: > On Mon, May 23, 2022 at 10:48:48AM +0200, Emanuele Giuseppe Esposito wrote: >> >> >> Am 22/05/2022 um 17:06 schrieb Stefan Hajnoczi: >>> On Wed, May 18, 2022 at 06:14:17PM +0200, Kevin Wolf wrote: >>>>

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 22/05/2022 um 17:06 schrieb Stefan Hajnoczi: > On Wed, May 18, 2022 at 06:14:17PM +0200, Kevin Wolf wrote: >> Am 18.05.2022 um 14:43 hat Paolo Bonzini geschrieben: >>> On 5/18/22 14:28, Emanuele Giuseppe Esposito wrote: >>>> For example, all callers of bdrv_ope

Re: [PATCH] block: drop unused bdrv_co_drain() API

drv_drained_end() instead. They are "mixed" > functions that can be called from coroutine context. Unlike > bdrv_co_drain(), these functions provide control of the length of the > drained section, which is usually the right thing. > > Signed-off-by: Stefan Hajnoczi R

Re: [PATCH 06/18] block: Implement blk_{pread, pwrite}() using generated_co_wrapper

n the next series? So that I can follow too. Thank you, Emanuele >From 84fcea52c09024adcfe24bb0d6d2ec6842c6826b Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Tue, 17 May 2022 13:35:54 -0400 Subject: [PATCH] block-coroutine-wrapper: remove includes from coroutines.h These incl

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 17/05/2022 um 12:59 schrieb Stefan Hajnoczi: > On Wed, May 04, 2022 at 02:39:05PM +0100, Stefan Hajnoczi wrote: >> On Tue, Apr 26, 2022 at 04:51:06AM -0400, Emanuele Giuseppe Esposito wrote: >>> This is a new attempt to replace the need to take the AioContext lock to

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 30/04/2022 um 07:17 schrieb Stefan Hajnoczi: > On Thu, Apr 28, 2022 at 11:56:09PM +0200, Emanuele Giuseppe Esposito wrote: >> >> >> Am 28/04/2022 um 12:45 schrieb Stefan Hajnoczi: >>> On Wed, Apr 27, 2022 at 08:55:35AM +0200, Emanuele Giuseppe Esposito wrote: &

Re: [RFC PATCH v2 3/8] block: introduce a lock to protect graph operations

Am 30/04/2022 um 07:48 schrieb Stefan Hajnoczi: > On Fri, Apr 29, 2022 at 10:37:54AM +0200, Emanuele Giuseppe Esposito wrote: >> Am 28/04/2022 um 15:45 schrieb Stefan Hajnoczi: >>> On Tue, Apr 26, 2022 at 04:51:09AM -0400, Emanuele Giuseppe Esposito wrote: >>&

Re: [RFC PATCH v2 2/8] coroutine-lock: release lock when restarting all coroutines

Am 29/04/2022 um 00:14 schrieb Paolo Bonzini: > On 4/28/22 13:21, Stefan Hajnoczi wrote: >> It's unclear whether this patch fixes a bug or introduces a new API that >> will be used in later patches. >> >> The commit message is a bit misleading: existing functions are not >> changed to release th

Re: [RFC PATCH v2 5/8] block.c: wrlock in bdrv_replace_child_noperm

Am 28/04/2022 um 15:55 schrieb Stefan Hajnoczi: > On Tue, Apr 26, 2022 at 04:51:11AM -0400, Emanuele Giuseppe Esposito wrote: >> The only problem here is ->attach and ->detach callbacks >> could call bdrv_{un}apply_subtree_drain(), which itself >> will use a rdlock to

Re: [RFC PATCH v2 4/8] async: register/unregister aiocontext in graph lock list

Am 29/04/2022 um 00:19 schrieb Paolo Bonzini: > On 4/28/22 15:46, Stefan Hajnoczi wrote: >>>     if have_block >>>     util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c')) >>> +  util_ss.add(files('../block/graph-lock.c')) >> Why is it in block/ if it needs to be built into libqemuutil? > Mayb

Re: [RFC PATCH v2 3/8] block: introduce a lock to protect graph operations

Am 28/04/2022 um 15:45 schrieb Stefan Hajnoczi: > On Tue, Apr 26, 2022 at 04:51:09AM -0400, Emanuele Giuseppe Esposito wrote: >> block layer graph operations are always run under BQL in the >> main loop. This is proved by the assertion qemu_in_main_thread() >>

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 28/04/2022 um 12:34 schrieb Stefan Hajnoczi: > On Tue, Apr 26, 2022 at 04:51:06AM -0400, Emanuele Giuseppe Esposito wrote: >> Next step is the most complex one: figure where to put the rdlocks. > > I got a little lost at this step but will hopefully understand more whe

Re: [RFC PATCH v2 1/8] aio_wait_kick: add missing memory barrier

Am 28/04/2022 um 13:09 schrieb Stefan Hajnoczi: > On Tue, Apr 26, 2022 at 04:51:07AM -0400, Emanuele Giuseppe Esposito wrote: >> It seems that aio_wait_kick always required a memory barrier >> or atomic operation in the caller, but almost nobody actually >> took care of

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 28/04/2022 um 12:45 schrieb Stefan Hajnoczi: > On Wed, Apr 27, 2022 at 08:55:35AM +0200, Emanuele Giuseppe Esposito wrote: >> >> >> Am 26/04/2022 um 10:51 schrieb Emanuele Giuseppe Esposito: >>> Luckly, most of the cases where we recursively go through a grap

Re: [RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

Am 26/04/2022 um 10:51 schrieb Emanuele Giuseppe Esposito: > Luckly, most of the cases where we recursively go through a graph are > the BlockDriverState callback functions in block_int-common.h > In order to understand what to protect, I categorized the callbacks in > block_

[RFC PATCH v2 8/8] mirror: protect drains in coroutine with rdlock

Drain performed in coroutine schedules a bh in the main loop. However, drain itself still is a read, and we need to signal the writer that we are going through the graph. Signed-off-by: Emanuele Giuseppe Esposito --- block/mirror.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/block

[RFC PATCH v2 5/8] block.c: wrlock in bdrv_replace_child_noperm

ection. Therefore change ->attach and ->detach to return true if they are modifying the lock, so that we don't take it twice or release temporarly. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 31 +++ block/block-backend.

[RFC PATCH v2 3/8] block: introduce a lock to protect graph operations

re is the reader, but only if there is one. If instead the original AioContext gets deleted, we need to transfer the current amount of readers in a global shared counter, so that the writer is always aware of all readers. Signed-off-by: Emanuele Giuseppe Esposito --- block/graph-lock

[RFC PATCH v2 4/8] async: register/unregister aiocontext in graph lock list

Add/remove the AioContext in aio_context_list in graph-lock.c only when it is being effectively created/destroyed. Signed-off-by: Emanuele Giuseppe Esposito --- util/async.c | 4 util/meson.build | 1 + 2 files changed, 5 insertions(+) diff --git a/util/async.c b/util/async.c index

[RFC PATCH v2 0/8] Removal of AioContext lock, bs->parents and ->children: new rwlock

, I can omit all functions protected by the added lock to avoid having duplicates when querying for new callbacks. Emanuele Giuseppe Esposito (8): aio_wait_kick: add missing memory barrier coroutine-lock: release lock when restarting all coroutines block: introduce a lock to protect gr

[RFC PATCH v2 6/8] block: assert that graph read and writes are performed correctly

Remove the old assert_bdrv_graph_writable, and replace it with the new version using graph-lock API. See the function documentation for more information. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 8 block/graph-lock.c

[RFC PATCH v2 1/8] aio_wait_kick: add missing memory barrier

It seems that aio_wait_kick always required a memory barrier or atomic operation in the caller, but almost nobody actually took care of doing it. Let's put the barrier in the function instead. Signed-off-by: Emanuele Giuseppe Esposito --- util/aio-wait.c | 3 ++- 1 file changed, 2 inser

[RFC PATCH v2 7/8] graph-lock: implement WITH_GRAPH_RDLOCK_GUARD and GRAPH_RDLOCK_GUARD macros

Similar to the implementation in lockable.h, implement macros to automatically take and release the rdlock. Create the empty GraphLockable struct only to use it as a type for G_DEFINE_AUTOPTR_CLEANUP_FUNC. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/graph-lock.h | 30

[RFC PATCH v2 2/8] coroutine-lock: release lock when restarting all coroutines

same functionality that we want. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/coroutine.h | 10 ++ util/qemu-coroutine-lock.c | 26 ++ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/include/qemu/coroutine.h b/include/qemu

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 13/04/2022 um 17:14 schrieb Emanuele Giuseppe Esposito: > > > Am 13/04/2022 um 16:51 schrieb Kevin Wolf: >> Am 13.04.2022 um 15:43 hat Emanuele Giuseppe Esposito geschrieben: >>> So this is a more concrete and up-to-date header. >>> >>> Few

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 13/04/2022 um 16:51 schrieb Kevin Wolf: > Am 13.04.2022 um 15:43 hat Emanuele Giuseppe Esposito geschrieben: >> So this is a more concrete and up-to-date header. >> >> Few things to notice: >> - we have a list of AioContext. They are registered once an aiocontext

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

So this is a more concrete and up-to-date header. Few things to notice: - we have a list of AioContext. They are registered once an aiocontext is created, and deleted when it is destroyed. This list is helpful because each aiocontext can only modify its own number of readers, avoiding unnecessary

Re: [PATCH] block/stream: Drain subtree around graph change

Am 05/04/2022 um 19:53 schrieb Emanuele Giuseppe Esposito: > > > Am 05/04/2022 um 17:04 schrieb Kevin Wolf: >> Am 05.04.2022 um 15:09 hat Emanuele Giuseppe Esposito geschrieben: >>> Am 05/04/2022 um 12:14 schrieb Kevin Wolf: >>>> I think all of this is

Re: [PATCH] block/stream: Drain subtree around graph change

Am 05/04/2022 um 17:04 schrieb Kevin Wolf: > Am 05.04.2022 um 15:09 hat Emanuele Giuseppe Esposito geschrieben: >> Am 05/04/2022 um 12:14 schrieb Kevin Wolf: >>> I think all of this is really relevant for Emanuele's work, which >>> involves adding AIO_WAIT_

Re: [PATCH] block/stream: Drain subtree around graph change

Am 05/04/2022 um 12:14 schrieb Kevin Wolf: > I think all of this is really relevant for Emanuele's work, which > involves adding AIO_WAIT_WHILE() deep inside graph update functions. I > fully expect that we would see very similar problems, and just stacking > drain sections over drain sections t

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 04/04/2022 um 11:41 schrieb Paolo Bonzini: > > > On Mon, Apr 4, 2022 at 11:25 AM Stefan Hajnoczi > wrote: > > - The new API doesn't stop more I/O requests from being submitted, it >   just blocks the current coroutine so request processing is deferred.

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 31/03/2022 um 18:40 schrieb Paolo Bonzini: > On 3/31/22 15:51, Emanuele Giuseppe Esposito wrote: >> >> bdrv_graph_list_wrlock <-> start_exclusive >> bdrv_graph_list_wrunlock <-> end_exclusive >> bdrv_graph_list_rdlock <-> cpu_exec_start >>

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 31/03/2022 um 11:59 schrieb Paolo Bonzini: > On 3/30/22 18:02, Paolo Bonzini wrote: >> On 3/30/22 12:53, Hanna Reitz wrote: Seems a good compromise between drains and rwlock. What do you think? >>> >>> Well, sounds complicated.  So I’m asking myself whether this would be >>> noticea

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 30/03/2022 um 12:53 schrieb Hanna Reitz: > On 17.03.22 17:23, Emanuele Giuseppe Esposito wrote: >> >> Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: >>>>> * Drains allow the caller (either main loop or iothread running >>>>> the

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 30/03/2022 um 11:52 schrieb Vladimir Sementsov-Ogievskiy: > 30.03.2022 12:09, Emanuele Giuseppe Esposito wrote: >>> >>> Ah seems I understand what you mean. >>> >>> One of my arguments is that "drain" - is not a lock against other >>&

Re: [PATCH v2] block/stream: Drain subtree around graph change

Am 29/03/2022 um 14:15 schrieb Hanna Reitz: > On 29.03.22 11:55, Vladimir Sementsov-Ogievskiy wrote: >> 29.03.2022 11:54, Hanna Reitz wrote: >>> On 28.03.22 12:24, Vladimir Sementsov-Ogievskiy wrote: 28.03.2022 11:09, Hanna Reitz wrote: > On 28.03.22 09:44, Hanna Reitz wrote: >> On

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

> > Ah seems I understand what you mean. > > One of my arguments is that "drain" - is not a lock against other > clients who want to modify the graph. Because, drained section allows > nested drained sections. > > And you try to solve it, by draining more things, this way, we'll drain > also the jo

Re: [PATCH for-7.0] main-loop: Disable GLOBAL_STATE_CODE() assertions

/* assert(qemu_in_main_thread()); */\ > } while (0) > > /* Mark and check that the function is part of the I/O API. */ > Reviewed-by: Emanuele Giuseppe Esposito

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: >>> * Drains allow the caller (either main loop or iothread running >>> the context) to wait all in_flights requests and operations >>> of a BDS: normal drains target a given node and is parents, while >&g

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: >> Next, I have a problem in mind, that in past lead to a lot of iotest 30 >> failures. Next there were different fixes and improvements, but the core >> problem (as far as I understand) is still here: nothing protects

Re: [PULL 21/50] block/block-backend.c: assertions for block-backend

Am 16/03/2022 um 13:53 schrieb Philippe Mathieu-Daudé: > On 16/3/22 13:44, Philippe Mathieu-Daudé wrote: >> Hi, >> >> On 4/3/22 17:46, Kevin Wolf wrote: >>> From: Emanuele Giuseppe Esposito >>> >>> All the global state (GS) API functions will c

Re: [PULL 21/50] block/block-backend.c: assertions for block-backend

ome invariants. Thank you, Emanuele Am 16/03/2022 um 13:44 schrieb Philippe Mathieu-Daudé: > Hi, > > On 4/3/22 17:46, Kevin Wolf wrote: >> From: Emanuele Giuseppe Esposito >> >> All the global state (GS) API functions will check that >> qemu_in_main_thread() return

Re: [PATCH v2 05/10] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child

next); and QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent); Please ignore it. This patch could eventually go in the subtree_drain serie, if we decide to go in that direction. Emanuele Am 14/03/2022 um 14:18 schrieb Emanuele Giuseppe Esposito: > Doing the opposite can make adding the

Re: [PATCH v2 04/10] block.c: bdrv_replace_child_noperm: first remove the child, and then call ->detach()

ild, next); Please ignore it. This patch could eventually go in the subtree_drain serie, if we decide to go in that direction. Emanuele Am 14/03/2022 um 14:18 schrieb Emanuele Giuseppe Esposito: > Doing the opposite can make ->detach() (more precisely > bdrv_unapply_subtre

[PATCH v6 17/18] job.c: enable job lock/unlock and remove Aiocontext locks

ned-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 72 - include/qemu/job.h | 19 --- job-qmp.c| 44 +++ job.c| 92 ++-- tests/unit/test-b

[PATCH v6 18/18] block_job_query: remove atomic read

Not sure what the atomic here was supposed to do, since job.busy is protected by the job lock. Since the whole function is called under job_mutex, just remove the atomic. Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 2 +- 1 file changed, 1 insertion

[PATCH v6 06/18] jobs: protect jobs with job_lock/unlock

macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 18 --- block/replication.c | 8 ++- blockdev.c | 17 -- blockjob.c | 56 +--- job-qmp.c | 2 + job.c | 125

[PATCH v6 12/18] block_job: rename block_job functions called with job_mutex held

Just as for the job API, rename block_job functions that are always called under job lock. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 3 ++- block/backup.c | 4 ++-- blockdev.c | 12 +++- blockjob.c

[PATCH v6 05/18] job.h: add _locked duplicates for job API functions called with and without job_mutex

) job_cancel_requested (_locked version is static) Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 25 +--- job.c | 48 -- 2

[PATCH v6 02/18] job.h: categorize fields in struct Job

Categorize the fields in struct Job to understand which ones need to be protected by the job mutex and which don't. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 59 ++ 1 file changed, 34 insertions(+), 25 deletions(-) diff

[PATCH v6 00/18] job: replace AioContext lock with job_mutex

API split patches are sent separately in another series * use of empty job_{lock/unlock} and JOB_LOCK_GUARD/WITH_JOB_LOCK_GUARD to avoid deadlocks and simplify the reviewer job * move patch 11 (block_job_query: remove atomic read) as last Emanuele Giuseppe Esposito (17): job.c: make job_m

[PATCH v6 03/18] job.c: API functions not used outside should be static

job_event_* functions can all be static, as they are not used outside job.c. Same applies for job_txn_add_job(). Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 18 -- job.c | 22 +++--- 2 files changed, 19 insertions(+), 21

[PATCH v6 14/18] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext

We are always using the given bs AioContext, so there is no need to take the job ones (which is identical anyways). This also reduces the point we need to check when protecting job.aio_context field. Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- block/commit.c | 4

[PATCH v6 08/18] jobs: use job locks also in the unit tests

Add missing job synchronization in the unit tests, with explicit locks. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 76 - tests/unit/test-block-iothread.c

[PATCH v2 08/10] tests/unit/test-bdrv-drain.c: graph setup functions can't run in coroutines

that we are not in a coroutine. Move the initialization phase of test_drv_cb and test_quiesce_common outside the coroutine logic. Signed-off-by: Emanuele Giuseppe Esposito Message-Id: <20211213104014.69858-2-eespo...@redhat.com> --- tests/unit/test-bdrv-drain.c

<    1   2   3   4   5   6   7   8   9   10   >