[PATCH v8 30/31] job.h: split function pointers in JobDriver

The job API will be handled separately in another serie. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 22 ++ 1 file changed, 22 insertions(+) diff --git a/include/qemu/job.h b/include/qemu/job.h index 6e67b6977f..c105b31076 100644 --- a/include/qemu

[PATCH v8 31/31] job.h: assertions in the callers of JobDriver function pointers

Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/job.c b/job.c index 54db80df66..075c6f3a20 100644 --- a/job.c +++ b/job.c @@ -381,6 +381,8 @@ void job_ref(Job *job) void job_unref(Job *job) { +GLOBAL_STATE_CODE

[PATCH v8 00/31] block layer: split block APIs in global state and I/O

ing in preparation to the API split" Based-on: <20220209105452.1694545-1-eespo...@redhat.com> Signed-off-by: Emanuele Giuseppe Esposito --- v8: bdrv_get_full_backing_filename to GLOBAL_STATE_CODE blk_iostatus_is_enabled in IO_CODE blk_iostatus_set_err in IO_CODE bdrv_apply_auto_read_onl

[PATCH v8 08/31] block/block-backend.c: assertions for block-backend

All the global state (GS) API functions will check that qemu_in_main_thread() returns true. If not, it means that the safety of BQL cannot be guaranteed, and they need to be moved to I/O. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 78

[PATCH v8 16/31] GS and IO CODE macros for blockjob_int.h

Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 5 + 1 file changed, 5 insertions(+) diff --git a/blockjob.c b/blockjob.c index 10815a89fe..d79a52d204 100644 --- a/blockjob.c +++ b/blockjob.c @@ -84,6 +84,7 @@ BlockJob *block_job_get(const char *id) void block_job_free(Job *job

[PATCH v8 19/31] assertions for blockjob.h global state API

Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/blockjob.c b/blockjob.c index d79a52d204..4868453d74 100644 --- a/blockjob.c +++ b/blockjob.c @@ -62,6 +62,7 @@ static bool is_block_job(Job *job) BlockJob *block_job_next

[PATCH v8 23/31] block/copy-before-write.h: global state API + assertions

copy-before-write functions always run under BQL. Signed-off-by: Emanuele Giuseppe Esposito --- block/copy-before-write.c | 2 ++ block/copy-before-write.h | 7 +++ 2 files changed, 9 insertions(+) diff --git a/block/copy-before-write.c b/block/copy-before-write.c index c30a5ff8de

[PATCH v8 20/31] include/sysemu/blockdev.h: global state API

blockdev functions run always under the BQL lock. Signed-off-by: Emanuele Giuseppe Esposito --- include/sysemu/blockdev.h | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index f9fb54d437..3211b16513 100644

[PATCH v8 18/31] include/block/blockjob.h: global state API

blockjob functions run always under the BQL lock. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/blockjob.h | 29 ++--- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/include/block/blockjob.h b/include/block/blockjob.h index 87fbb3985f

[PATCH v8 25/31] block_int-common.h: split function pointers in BlockDriver

Similar to the header split, also the function pointers in BlockDriver can be split in I/O and global state. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/block_int-common.h | 445 --- 1 file changed, 237 insertions(+), 208 deletions(-) diff --git

[PATCH v8 28/31] block_int-common.h: assertions in the callers of BdrvChildClass function pointers

Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index 5afdbe3060..01811d6519 100644 --- a/block.c +++ b/block.c @@ -1497,7 +1497,7 @@ const BdrvChildClass child_of_bds = { AioContext

[PATCH v8 15/31] include/block/blockjob_int.h: split header into I/O and GS API

Since the I/O functions are not many, keep a single file. Also split the function pointers in BlockJobDriver. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- include/block/blockjob_int.h | 28 1 file changed, 28 insertions(+) diff --git

[PATCH v8 21/31] assertions for blockdev.h global state API

Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 3 +++ blockdev.c| 16 2 files changed, 19 insertions(+) diff --git a/block/block-backend.c b/block/block-backend.c index 2ab1274dfe..bf77c4a8fa 100644 --- a/block/block-backend.c +++ b/block

[PATCH v8 26/31] block_int-common.h: assertions in the callers of BlockDriver function pointers

Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 17 + block/create.c | 2 ++ 2 files changed, 19 insertions(+) diff --git a/block.c b/block.c index 4a3447b2a0..5afdbe3060 100644 --- a/block.c +++ b/block.c @@ -529,6 +529,7 @@ static void coroutine_fn

[PATCH v8 29/31] block-backend-common.h: split function pointers in BlockDevOps

Assertions in the callers of the function pointrs are already added by previous patches. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi Reviewed-by: Philippe Mathieu-Daudé --- include/sysemu/block-backend-common.h | 28 ++- 1 file changed, 23

[PATCH v8 27/31] block_int-common.h: split function pointers in BdrvChildClass

Signed-off-by: Emanuele Giuseppe Esposito --- include/block/block_int-common.h | 81 ++-- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index f05ebb0da3..5a04c778e4 100644

[PATCH v8 02/31] main loop: macros to mark GS and I/O functions

in all callers. They will also visually help understanding in which category each function is, without looking at the header. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/main-loop.h | 18 ++ 1 file changed, 18 insertions(+) diff --git a/include/qemu/main-loop.h b

[PATCH v8 11/31] include/block/block_int: split header into I/O and global state API

-by: Emanuele Giuseppe Esposito --- blockdev.c |5 + include/block/block_int-common.h | 1180 +++ include/block/block_int-global-state.h | 312 + include/block/block_int-io.h | 179 +++ include/block/block_int.h | 1489

[PATCH v8 01/31] main-loop.h: introduce qemu_in_main_thread()

in next patches fail despite the AioContext is still the main loop. See the comment in the function header for more information. Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/main-loop.h | 24 softmmu/cpus.c | 5 + stubs/iothread-lock.c| 5

[PATCH v8 12/31] assertions for block_int global state API

Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 15 +++ block/backup.c | 1 + block/block-backend.c | 3 +++ block/commit.c | 2 ++ block/dirty-bitmap.c| 1 + block/io.c

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

On 11/02/2022 13:34, Kevin Wolf wrote: > Am 08.02.2022 um 16:36 hat Emanuele Giuseppe Esposito geschrieben: >> Doing the opposite can make adding the child node to a non-drained node, >> as apply_subtree_drain is only done in ->attach() and thus make >> assert_

Re: [PATCH 5/6] test-bdrv-drain.c: remove test_detach_by_parent_cb()

On 11/02/2022 16:38, Kevin Wolf wrote: > Am 08.02.2022 um 16:36 hat Emanuele Giuseppe Esposito geschrieben: >> This test uses a callback of an I/O function (blk_aio_preadv) >> to modify the graph, using bdrv_attach_child. >> This is simply not allowed anymore. I/O ca

Re: [PATCH 1/6] block/io.c: fix bdrv_child_cb_drained_begin invocations from a coroutine

On 11/02/2022 12:54, Kevin Wolf wrote: > Am 08.02.2022 um 16:36 hat Emanuele Giuseppe Esposito geschrieben: >> Using bdrv_do_drained_begin_quiesce() in bdrv_child_cb_drained_begin() >> is not a good idea: the callback might be called when running >> a

[PATCH v7 07/31] include/sysemu/block-backend: split header into I/O and global state (GS) API

ared between the two headers, and the functions that can't be categorized as I/O or global state. Assertions are added in the next patch. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 9 +- include/sysemu/block-backend-common.h | 84 ++

[PATCH v7 09/31] IO_CODE and IO_OR_GS_CODE for block-backend I/O API

Mark all I/O functions with IO_CODE, and all "I/O OR GS" with IO_OR_GS_CODE. Signed-off-by: Emanuele Giuseppe Esposito --- block/block-backend.c | 57 +++ include/sysemu/block-backend-io.h | 2 ++ 2 files changed, 59 insertions(+) diff --g

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

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

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: [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 >>>>>

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 >>>

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 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: [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

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: [PATCH 10/12] block.c: add subtree_drains where needed

On 01/02/2022 15:47, Vladimir Sementsov-Ogievskiy wrote: > 18.01.2022 19:27, Emanuele Giuseppe Esposito wrote: >> Protect bdrv_replace_child_noperm, as it modifies the >> graph by adding/removing elements to .children and .parents >> list of a bs. Use

Re: [PATCH 10/12] block.c: add subtree_drains where needed

On 02/02/2022 18:38, Paolo Bonzini wrote: > On 2/2/22 16:37, Emanuele Giuseppe Esposito wrote: >> So we have disk B with backing file C, and new disk A that wants to have >> backing file C. >> >> I think I understand what you mean, so in theory the operation wou

Re: [PATCH 09/12] jobs: ensure sleep in job_sleep_ns is fully performed

On 26/01/2022 12:21, Stefan Hajnoczi wrote: > On Tue, Jan 18, 2022 at 11:27:35AM -0500, Emanuele Giuseppe Esposito wrote: >> If a drain happens while a job is sleeping, the timeout >> gets cancelled and the job continues once the drain ends. >> This is especially bad fo

Re: [PATCH 05/12] test-bdrv-drain.c: adapt test to the coming subtree drains

On 19/01/2022 10:18, Paolo Bonzini wrote: > On 1/18/22 17:27, Emanuele Giuseppe Esposito wrote: >> - First of all, inconsistency between block_job_create under >> aiocontext lock that internally calls blk_insert_bs and therefore >> bdrv_replace_child_noperm, and blk_insert_

Re: [PATCH 01/12] introduce BDRV_POLL_WHILE_UNLOCKED

On 26/01/2022 11:49, Stefan Hajnoczi wrote: > On Tue, Jan 18, 2022 at 11:27:27AM -0500, Emanuele Giuseppe Esposito wrote: >> Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED. >> >> Signed-off-by: Emanuele Giuseppe Esposito >> --- >> include/block/blo

Re: [PATCH 10/12] block.c: add subtree_drains where needed

On 19/01/2022 10:47, Paolo Bonzini wrote: > > About this: > >> + * TODO: this is called by job_unref with lock held, because >> + * afterwards it calls bdrv_try_set_aio_context. >> + * Once all of this is fixed, take care of removing >> + * the aiocontext lock

Re: [PATCH 10/12] block.c: add subtree_drains where needed

>> >>  From the other thread: >>> So you mean that backing_hd is modified as its parent is changed, do >>> I understand correctly? >> >> Yes >> >>> >>> As we started to discuss in a thread started with my "[PATCH] block: >>> bdrv_set_backing_hd(): use drained section", I think draining the whole

[PATCH v4 13/19] 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 --- block

[PATCH v4 10/19] job.h: define locked functions

} and job lock guard macros are *nop*. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 5 +- block/backup.c | 4 +- block/replication.c | 4 +- blockdev.c | 44 + blockjob.c | 28

[PATCH v4 11/19] jobs: document all static functions and add _locked() suffix

Now that we added the job_lock/unlock pairs, we can also rename all static functions in job.c that are called with the job mutex held as _locked(), and add a little comment on top. No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 29

[PATCH v4 06/19] jobs: remove aiocontext locks since the functions are under BQL

/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 --- blockdev.c | 4 job-qmp.c | 4 2 files changed, 8 deletions(-) diff --git a/blockdev.c b/blockdev.c index 800e95ba54

[PATCH v4 05/19] 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. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/aio-wait.h | 15 +++ 1 file

[PATCH v4 04/19] job.c: move inner aiocontext lock in callbacks

. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/job.c b/job.c index e97b159396..2b88cef19d 100644 --- a/job.c +++ b/job.c @@ -149,7 +149,6 @@ static void job_txn_del_job(Job *job) static int job_txn_apply

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

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 --- include/qemu/job.h | 24 job.c | 35

[PATCH v4 14/19] blockjob: block_job_get_aio_context is a GS function

Move block_job_get_aio_context under the GS category in blockjob.h, as it is always called with the BQL held. This is done also to simplify the coming job->aiocontext protection. For more infos about the GS API, see include/block/block-global-state.h Signed-off-by: Emanuele Giuseppe Espos

[PATCH v4 16/19] 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 v4 19/19] 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. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git

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

ned-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 65 --- include/qemu/job.h | 22 - job-qmp.c| 41 - job.c| 76 +++- tests/unit/t

[PATCH v4 07/19] 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 | 60 +--- job-qmp.c | 2 + job.c | 110

[PATCH v4 12/19] job.h: define unlocked functions

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

[PATCH v4 17/19] 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. Suggested-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- block/replication.c | 2 +- blockjob.c

[PATCH v4 15/19] 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. Signed-off-by: Emanuele Giuseppe Esposito --- block/commit.c | 4 ++-- block/mirror.c | 2 +- 2

[PATCH v4 02/19] 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 --git

[PATCH v4 03/19] job.c: make job_event_* functions static

job_event_* functions can all be static, as they are not used outside job.c. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- include/qemu/job.h | 6 -- job.c | 12 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include

[PATCH v4 00/19] job: replace AioContext lock with job_mutex

_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 (18): job.c: make job_mutex and job_lock/unlock() public job.h: categorize fields in struct Job job.c: make job_event_* functions sta

[PATCH v4 08/19] jobs: add job lock in find_* functions

*. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 14 +- job-qmp.c | 13 - 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index 2e522dd7fe..7a9e9b4a13 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3311,7 +3311,10 @@ out

[PATCH v4 09/19] 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

Re: [PATCH] block: bdrv_set_backing_hd(): use drained section

On 27/01/2022 15:13, Kevin Wolf wrote: > Am 25.01.2022 um 11:12 hat Vladimir Sementsov-Ogievskiy geschrieben: >> 25.01.2022 12:24, Paolo Bonzini wrote: >>> On 1/24/22 18:37, Vladimir Sementsov-Ogievskiy wrote: Graph modifications should be done in drained section. stream_prepare()

Re: [PATCH v6 25/33] block_int-common.h: split function pointers in BdrvChildClass

On 26/01/2022 13:42, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> Signed-off-by: Emanuele Giuseppe Esposito >> --- >>   include/block/block_int-common.h | 67 +++- >>   1 file changed, 40 insertions(+), 27 de

Re: [PATCH v6 04/33] block/export/fuse.c: allow writable exports to take RESIZE permission

On 25/01/2022 17:51, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> Allow writable exports to get BLK_PERM_RESIZE permission >> from creation, in fuse_export_create(). >> In this way, there is no need to give the permission in >> fuse

Re: [PATCH v6 29/33] job.h: assertions in the callers of JobDriver funcion pointers

On 26/01/2022 15:13, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> Signed-off-by: Emanuele Giuseppe Esposito >> --- >>   job.c | 9 + >>   1 file changed, 9 insertions(+) > > Just curious, why did you remove the assert

Re: [PATCH 00/12] Removal of Aiocontext lock through drains: protect bdrv_replace_child_noperm.

On 27/01/2022 14:46, Paolo Bonzini wrote: > On 1/26/22 12:29, Stefan Hajnoczi wrote: >> Still, I'm a bit surprised I didn't notice any >> aio_context_acquire/release() removals in this patch series because I >> thought callers need that before they switch to >> BDRV_POLL_WHILE_UNLOCKED()? > >

Re: [PATCH v6 32/33] crypto: delegate permission functions to JobDriver .pre_run

On 26/01/2022 17:10, Hanna Reitz wrote: > On 21.01.22 18:05, Emanuele Giuseppe Esposito wrote: >> block_crypto_amend_options_generic_luks uses the block layer >> permission API, therefore it should be called with the BQL held. >> >> However, the same function is bein

Re: [PATCH v6 20/33] block: rename bdrv_invalidate_cache_all, blk_invalidate_cache and test_sync_op_invalidate_cache

On 24/01/2022 11:44, Juan Quintela wrote: >> diff --git a/migration/migration.c b/migration/migration.c >> index 0652165610..1f06fd2d18 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -499,7 +499,7 @@ static void process_incoming_migration_bh(void *opaque) >>

Re: [PATCH v6 02/33] include/block/block: split header into I/O and global state API

On 31/01/2022 16:58, Paolo Bonzini wrote: > On 1/31/22 15:54, Kevin Wolf wrote: >> So I guess the decision depends on what you're going to use the >> categories in the future. I get the feeling that we have one more >> category than this series introduces: >> >> * Global State (must run from

Re: [PATCH v6 01/33] main-loop.h: introduce qemu_in_main_thread()

On 31/01/2022 16:49, Paolo Bonzini wrote: > On 1/31/22 15:33, Kevin Wolf wrote: >> I feel "use this function if your code is going to be used by unit >> tests, but use qemu_mutex_iothread_locked() otherwise" is a very strange >> interface. I'm relatively sure that qemu_mutex_iothread_locked()

Re: [PATCH v4 16/19] job: detect change of aiocontext within job coroutine

> As a small improvement, we don't need aio_co_enter() in job_start(), > we can simplify it with qemu_coroutine_enter(). If the aio_context > where it is running is wrong, job_do_yield_locked() will automatically > reschedule the coroutine where it belongs, once the first > job_pause_point is

Re: [PATCH v6 13/33] include/block/blockjob.h: global state API

On 07/02/2022 18:26, Kevin Wolf wrote: > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben: >> blockjob functions run always under the BQL lock. >> >> Signed-off-by: Emanuele Giuseppe Esposito >> Reviewed-by: Stefan Hajnoczi >> ---

Re: [PATCH v6 02/33] include/block/block: split header into I/O and global state API

On 07/02/2022 17:53, Kevin Wolf wrote: > Am 01.02.2022 um 11:30 hat Paolo Bonzini geschrieben: >> On 2/1/22 10:45, Emanuele Giuseppe Esposito wrote: >>>> That said, even if they are a different category, I think it makes sense >>>> to leave them in the same head

Re: [PATCH v6 31/33] include/qemu/job.h: introduce job->pre_run() and use it in amend

On 07/02/2022 19:14, Kevin Wolf wrote: > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben: >> Introduce .pre_run() job callback. This cb will run in job_start, >> before the coroutine is created and runs run() in the job aiocontext. >> >> Theref

[PATCH v5 07/20] job.h: add _locked duplicates for job API functions called with and without job_mutex

) job_cancel_requested (_locked version is static) Signed-off-by: Emanuele Giuseppe Esposito --- include/qemu/job.h | 25 +--- job.c | 48 -- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/include/qemu/job.h

Re: [PATCH v6 00/33] block layer: split block APIs in global state and I/O

On 07/02/2022 19:30, Kevin Wolf wrote: > Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben: >> Each function in the GS API will have an assertion, checking >> that it is always running under BQL. >> I/O functions are instead thread safe (or s

[PATCH v5 00/20] job: replace AioContext lock with job_mutex

OB_LOCK_GUARD * mu(u)ltiple typos in commit messages * 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

[PATCH v5 04/20] job.c: move inner aiocontext lock in callbacks

. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/job.c b/job.c index f13939d3c6..d8c13ac0d1 100644 --- a/job.c +++ b/job.c @@ -149,7 +149,6 @@ static void job_txn_del_job(Job *job) static int job_txn_apply

[PATCH 6/6] jobs: ensure sleep in job_sleep_ns is fully performed

sserts. Signed-off-by: Emanuele Giuseppe Esposito --- job.c | 19 +++ tests/qemu-iotests/030 | 2 +- tests/qemu-iotests/151 | 4 ++-- tests/unit/test-blockjob.c | 2 +- 4 files changed, 15 insertions(+), 12 deletions(-) diff --git a/job.c b/job

[PATCH 0/6] block: bug fixes in preparation of AioContext removal

ill a work in progress and these fixes are pretty much independent, I split that in two separate series. This serie is based on "job: replace AioContext lock with job_mutex" Based-on: <20220208143513.1077229-1-eespo...@redhat.com> Emanuele Giuseppe Esposito (6): block/io.c: fix b

[PATCH v5 13/20] 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 v2 0/3] job: split job API in driver and monitor

t;. Based-on: <20220208143513.1077229-1-eespo...@redhat.com> --- v2: * rebased on new version of job_api Emanuele Giuseppe Esposito (3): jobs: add job-common.h jobs: add job-monitor.h jobs: add job-driver.h include/qemu/job-common.h | 357 +++ include/qemu/job

[PATCH v5 03/20] 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 | 12 +--- 2 files changed, 9 insertions(+), 21 deletions

[PATCH v5 05/20] 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. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/aio-wait.h | 15 +++ 1 file

[PATCH v5 02/20] 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 --git

[PATCH v5 09/20] jobs: add job lock in find_* functions

*. Signed-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 14 +- job-qmp.c | 13 - 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/blockdev.c b/blockdev.c index c5fba4d157..08408cd44b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -3311,7 +3311,10 @@ out

[PATCH 4/6] test-bdrv-drain.c: adapt test to the coming subtree drains

the original after it's been invoked. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 17 - 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 4924ceb562..c52ba2db4e 100644

[PATCH 3/6] block.c: bdrv_replace_child_noperm: first call ->attach(), and then add child

toring the drain with apply_subtree_drain(), leaving the node undrained between the two operations. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/block.c b/block.c index ec346a7e2e..08a6e3a4ef 100644 --- a/block.c

[PATCH v5 14/20] 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 v5 20/20] 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. Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git

[PATCH v5 08/20] 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 | 62 +++--- job-qmp.c | 2 + job.c | 123

[PATCH v2 1/3] jobs: add job-common.h

-by: Emanuele Giuseppe Esposito --- include/qemu/job-common.h | 357 ++ include/qemu/job.h| 328 +- 2 files changed, 358 insertions(+), 327 deletions(-) create mode 100644 include/qemu/job-common.h diff --git a/include

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

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 --- include/qemu/job.h | 24 job.c | 35

[PATCH v5 12/20] 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 75f7c28147..56d2a8d98f 100644 --- a/job.c +++ b/jo

[PATCH v5 16/20] 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. Signed-off-by: Emanuele Giuseppe Esposito --- block/commit.c | 4 ++-- block/mirror.c | 2 +- 2

[PATCH v5 19/20] job.c: enable job lock/unlock and remove Aiocontext locks

ned-off-by: Emanuele Giuseppe Esposito --- blockdev.c | 65 --- include/qemu/job.h | 19 job-qmp.c| 41 - job.c| 77 +++- tests/unit/t

[PATCH v5 06/20] jobs: remove aiocontext locks since the functions are under BQL

/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 --- blockdev.c | 4 job-qmp.c | 4 2 files changed, 8 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8cac3d739c

[PATCH v5 10/20] 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 v5 11/20] 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 --- block

[PATCH v5 15/20] 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 | 37 ++--- job.c

<    3   4   5   6   7   8   9   10   11   12   >