[RFC PATCH 5/6] test-bdrv-drain.c: adapt test to the new subtree drains

2021-12-13 Thread Emanuele Giuseppe Esposito
, or multiple times. We do not want that, so override the callback only when we actually want to use it. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/unit/test-bdrv-drain.c b

[RFC PATCH 4/6] block.c: add subtree_drains where needed

2021-12-13 Thread Emanuele Giuseppe Esposito
Protect bdrv_replace_child_noperm, as it modifies the graph by adding/removing elements to .children and .parents list of a bs. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 24 1 file changed, 24 insertions(+) diff --git a/block.c b/block.c index 3c3c90704c

[RFC PATCH 6/6] block/io.c: enable assert_bdrv_graph_writable

2021-12-13 Thread Emanuele Giuseppe Esposito
Signed-off-by: Emanuele Giuseppe Esposito --- block/io.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/io.c b/block/io.c index a031691860..c2f1a494c4 100644 --- a/block/io.c +++ b/block/io.c @@ -759,6 +759,7 @@ void assert_bdrv_graph_writable(BlockDriverState *bs) * Once

[RFC PATCH 1/6] tests/unit/test-bdrv-drain.c: graph setup functions can't run in coroutines

2021-12-13 Thread Emanuele Giuseppe Esposito
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 --- tests/unit/test-bdrv-drain.c | 118 ++- 1 file changed, 73 insertions(+), 45 deletions

[RFC PATCH 2/6] introduce BDRV_POLL_WHILE_UNLOCKED

2021-12-13 Thread Emanuele Giuseppe Esposito
Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/block-global-state.h | 5 + 1 file changed, 5 insertions(+) diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h index 7a6b065101

Re: [PATCH v5 10/31] block.c: modify .attach and .detach callbacks of child_of_bds

2021-12-16 Thread Emanuele Giuseppe Esposito
On 16/12/2021 15:57, Hanna Reitz wrote: On 24.11.21 07:43, Emanuele Giuseppe Esposito wrote: According to the assertions put in the previous patch, we should first drain and then modify the ->children list. In this way we prevent other iothreads to read the list while it is being upda

Re: [PATCH v5 13/31] block.c: add assertions to static functions

2021-12-16 Thread Emanuele Giuseppe Esposito
On 16/12/2021 17:08, Hanna Reitz wrote: On 24.11.21 07:44, Emanuele Giuseppe Esposito wrote: Following the assertion derived from the API split, propagate the assertion also in the static functions. Signed-off-by: Emanuele Giuseppe Esposito ---   block.c | 45

Re: [RFC PATCH 0/6] Removal of Aiocontext lock and usage of subtree drains in aborted transactions

2021-12-14 Thread Emanuele Giuseppe Esposito
On 13/12/2021 15:52, Stefan Hajnoczi wrote: Off-topic: I don't understand the difference between the effects of bdrv_drained_begin() and bdrv_subtree_drained_begin(). Both call aio_disable_external(aio_context) and aio_poll(). bdrv_drained_begin() only polls parents and itself, while

[PATCH v3 1/3] block_int: make bdrv_backing_overridden static

2021-12-15 Thread Emanuele Giuseppe Esposito
bdrv_backing_overridden is only used in block.c, so there is no need to leave it in block_int.h Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- block.c | 4 +++- include/block/block_int.h | 3 --- 2 files changed, 3 insertions(+), 4 deletions

[PATCH v3 0/3] block: minor refactoring in preparation to the block layer API split

2021-12-15 Thread Emanuele Giuseppe Esposito
functions in blockdev.h and block_int.h. Signed-off-by: Emanuele Giuseppe Esposito --- v3: * Apply Kevin comments, remove getter method added in v2 and do not touch drive_add(). v2: * Apply Philippe comments, instead of renaming a make if_name public, create a getter method (discard old patch 2).

[PATCH v3 2/3] include/sysemu/blockdev.h: remove drive_mark_claimed_by_board and inline drive_def

2021-12-15 Thread Emanuele Giuseppe Esposito
drive_def is only a particular use case of qemu_opts_parse_noisily, so it can be inlined. Also remove drive_mark_claimed_by_board, as it is only defined but not implemented (nor used) anywhere. Signed-off-by: Emanuele Giuseppe Esposito --- block/monitor/block-hmp-cmds.c | 2 +- blockdev.c

Re: [PATCH v2 3/4] include/sysemu/blockdev.h: move drive_add and inline drive_def

2021-12-15 Thread Emanuele Giuseppe Esposito
On 15/12/2021 11:00, Kevin Wolf wrote: Am 15.12.2021 um 10:19 hat Emanuele Giuseppe Esposito geschrieben: On 14/12/2021 15:35, Kevin Wolf wrote: Am 30.11.2021 um 10:46 hat Emanuele Giuseppe Esposito geschrieben: drive_add is only used in softmmu/vl.c, so it can be a static function

[PATCH v3 3/3] include/sysemu/blockdev.h: remove drive_get_max_devs

2021-12-15 Thread Emanuele Giuseppe Esposito
Remove drive_get_max_devs, as it is not used by anyone. Last use was removed in commit 8f2d75e81d5 ("hw: Drop superfluous special checks for orphaned -drive"). Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Stefan Hajnoczi ---

Re: [PATCH v5 09/31] block: introduce assert_bdrv_graph_writable

2021-12-14 Thread Emanuele Giuseppe Esposito
On 10/12/2021 18:43, Hanna Reitz wrote: On 24.11.21 07:43, Emanuele Giuseppe Esposito wrote: We want to be sure that the functions that write the child and parent list of a bs are under BQL and drain. BQL prevents from concurrent writings from the GS API, while drains protect from I/O

Re: [PATCH v5 05/31] block-backend: special comments for blk_set/get_perm due to fuse

2021-12-15 Thread Emanuele Giuseppe Esposito
On 10/12/2021 15:38, Hanna Reitz wrote: On 24.11.21 07:43, Emanuele Giuseppe Esposito wrote: Fuse logic can be classified as I/O, so there is no BQL held during its execution. And yet, it uses blk_{get/set}_perm functions, that are classified as BQL and clearly require the BQL lock. Since

Re: [PATCH v2 3/4] include/sysemu/blockdev.h: move drive_add and inline drive_def

2021-12-15 Thread Emanuele Giuseppe Esposito
On 14/12/2021 15:35, Kevin Wolf wrote: Am 30.11.2021 um 10:46 hat Emanuele Giuseppe Esposito geschrieben: drive_add is only used in softmmu/vl.c, so it can be a static function there, and drive_def is only a particular use case of qemu_opts_parse_noisily, so it can be inlined. Also remove

Re: [PATCH v5 05/31] block-backend: special comments for blk_set/get_perm due to fuse

2021-12-15 Thread Emanuele Giuseppe Esposito
On 15/12/2021 09:57, Emanuele Giuseppe Esposito wrote: On 10/12/2021 15:38, Hanna Reitz wrote: On 24.11.21 07:43, Emanuele Giuseppe Esposito wrote: Fuse logic can be classified as I/O, so there is no BQL held during its execution. And yet, it uses blk_{get/set}_perm functions

[PATCH v7 17/31] block.c: add assertions to static functions

2022-02-11 Thread Emanuele Giuseppe Esposito
Following the assertion derived from the API split, propagate the assertion also in the static functions. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 47 ++- block/block-backend.c | 3 +++ 2 files changed, 49 insertions(+), 1

[PATCH v7 05/31] IO_CODE and IO_OR_GS_CODE for block I/O API

2022-02-11 Thread Emanuele Giuseppe Esposito
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.c | 35 +++- block/dirty-bitmap.c | 1 + block/io.c

[PATCH v7 13/31] IO_CODE and IO_OR_GS_CODE for block_int I/O API

2022-02-11 Thread Emanuele Giuseppe Esposito
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.c | 14 +- block/block-backend.c| 2 ++ block/dirty-bitmap.c | 3 +++ block/io.c

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

2022-02-11 Thread Emanuele Giuseppe Esposito
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 d1f5cd2b39..4297431812 100644 --- a/block.c +++ b/block.c @@ -1497,7 +1497,7 @@ const BdrvChildClass child_of_bds = { AioContext

[PATCH v7 22/31] include/block/snapshot: global state API + assertions

2022-02-11 Thread Emanuele Giuseppe Esposito
Snapshots run also under the BQL, so they all are in the global state API. The aiocontext lock that they hold is currently an overkill and in future could be removed. Signed-off-by: Emanuele Giuseppe Esposito --- block/snapshot.c | 28 include/block

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

2022-02-11 Thread Emanuele Giuseppe Esposito
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 e5708702db..d8a77a5832 100644 --- a/block/block-backend.c +++ b/block

[PATCH v7 24/31] block/coroutines: I/O and "I/O or GS" API

2022-02-11 Thread Emanuele Giuseppe Esposito
block coroutines functions run in different aiocontext, and are not protected by the BQL. Therefore are I/O. On the other side, generated_co_wrapper functions use BDRV_POLL_WHILE, meaning the caller can either be the main loop or a specific iothread. Signed-off-by: Emanuele Giuseppe Esposito

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

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v7 16/31] GS and IO CODE macros for blockjob_int.h

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v7 19/31] assertions for blockjob.h global state API

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v7 31/31] job.h: assertions in the callers of JobDriver function pointers

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v7 25/31] block_int-common.h: split function pointers in BlockDriver

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v7 26/31] block_int-common.h: assertions in the callers of BlockDriver function pointers

2022-02-11 Thread Emanuele Giuseppe Esposito
Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 18 ++ block/create.c | 2 ++ 2 files changed, 20 insertions(+) diff --git a/block.c b/block.c index 7cacb5a1a7..d1f5cd2b39 100644 --- a/block.c +++ b/block.c @@ -529,6 +529,7 @@ static void coroutine_fn

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

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v7 30/31] job.h: split function pointers in JobDriver

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v7 20/31] include/sysemu/blockdev.h: global state API

2022-02-11 Thread Emanuele Giuseppe Esposito
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 v2 00/10] block: bug fixes in preparation of AioContext removal

2022-03-14 Thread Emanuele Giuseppe Esposito
ill a work in progress and these fixes are pretty much independent, I split that in two separate series. --- v2: * change comments in patch 2 and 3, .attach() and .detach() callbacks. * remove job_sleep_ns patch, as it was causing random deadlocks in test 030 Emanuele Giuseppe Esposito (10): drains:

[PATCH v2 01/10] drains: create bh only when polling

2022-03-14 Thread Emanuele Giuseppe Esposito
we create coroutine and bh regardless on whether we eventually poll or not. There is no need to do so, if no poll takes place. Signed-off-by: Emanuele Giuseppe Esposito --- block/io.c | 31 +++ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/block/io.c b

[PATCH v2 10/10] tests/qemu-iotests/030: test_stream_parallel should use auto_finalize=False

2022-03-14 Thread Emanuele Giuseppe Esposito
failing. run_job() will then take care of calling job-finalize. Signed-off-by: Emanuele Giuseppe Esposito Suggested-by: Kevin Wolf --- tests/qemu-iotests/030 | 12 +++- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 index

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

2022-03-14 Thread Emanuele Giuseppe Esposito
ote that assert_bdrv_graph_writable is not yet fully enabled. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index 372f16f4a0..d870ba5393 100644 --- a/block.c +++ b/block.c @@ -1448,6 +1448,11 @@ static v

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

2022-03-14 Thread Emanuele Giuseppe Esposito
toring the drain with apply_subtree_drain(), leaving the node undrained between the two operations. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/block.c b/block.c index d870ba5393..c6a550f9c6

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

2022-03-14 Thread Emanuele Giuseppe Esposito
*. 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 8722e5d4b9..39e6fe4d59 100644

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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 v6 17/18] job.c: enable job lock/unlock and remove Aiocontext locks

2022-03-14 Thread Emanuele Giuseppe Esposito
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 06/18] jobs: protect jobs with job_lock/unlock

2022-03-14 Thread Emanuele Giuseppe Esposito
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 15/18] job: detect change of aiocontext within job coroutine

2022-03-14 Thread Emanuele Giuseppe Esposito
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 v2 03/10] block/io.c: fix bdrv_child_cb_drained_begin invocations from a coroutine

2022-03-14 Thread Emanuele Giuseppe Esposito
and poll will accomplish the same thing (invoking bdrv_do_drained_begin_quiesce) but will firstly check if we are already in a coroutine, and exit from that via bdrv_co_yield_to_drain(). Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 +- block/io.c | 8

[PATCH v2 02/10] bdrv_parent_drained_begin_single: handle calls from coroutine context

2022-03-14 Thread Emanuele Giuseppe Esposito
ain in the main loop, and enter the coroutine only once it is done. Just as the other drains, check the coroutine case only when effectively polling. As a consequence of this, remove the coroutine assertion in bdrv_do_drained_begin_quiesce. We are never polling in that case. Signed-off-by: Emanuele Giuse

[PATCH v2 07/10] test-bdrv-drain.c: remove test_detach_by_parent_cb()

2022-03-14 Thread Emanuele Giuseppe Esposito
s://lists.nongnu.org/archive/html/qemu-block/2018-05/msg01132.html but as explained above I believe that it is not valid anymore, and can be discarded. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- tests/unit/test-bdrv-drain.c | 46 +---

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

2022-03-14 Thread Emanuele Giuseppe Esposito
PI 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_mutex

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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 v6 03/18] job.c: API functions not used outside should be static

2022-03-14 Thread Emanuele Giuseppe Esposito
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 v2 08/10] tests/unit/test-bdrv-drain.c: graph setup functions can't run in coroutines

2022-03-14 Thread Emanuele Giuseppe Esposito
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

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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 v2 06/10] test-bdrv-drain.c: adapt test to support additional subtree drains

2022-03-14 Thread Emanuele Giuseppe Esposito
in bdrv_replace_child_noperm(), the test callback would be called too early and/or multiple times. Override the callback only when we actually want to use it, and put back the original after it's been invoked. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 15

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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 14/18] commit and mirror: create new nodes using bdrv_get_aio_context, and not the job aiocontext

2022-03-14 Thread Emanuele Giuseppe Esposito
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 01/18] job.c: make job_mutex and job_lock/unlock() public

2022-03-14 Thread Emanuele Giuseppe Esposito
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 | 24

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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 | 17

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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 v2 09/10] child_job_drained_poll: override polling condition only when in home thread

2022-03-14 Thread Emanuele Giuseppe Esposito
allow drv->drained_poll() to be called only by the iothread, and not the main loop. We distinguish it by using in_aio_context_home_thread(), that returns false if @ctx is not the same as the thread that runs it. Co-Developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- blo

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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.

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

2022-03-16 Thread Emanuele Giuseppe Esposito
, next); and QLIST_INSERT_HEAD(_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 child node to a non-

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

2022-03-16 Thread Emanuele Giuseppe Esposito
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_s

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

2022-03-16 Thread Emanuele Giuseppe Esposito
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_thr

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

2022-03-16 Thread Emanuele Giuseppe Esposito
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: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

2022-03-16 Thread Emanuele Giuseppe Esposito
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 prot

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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 3e9e632a18..298432ce00 100644 --- a/job.c +++ b/jo

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

2022-03-14 Thread Emanuele Giuseppe Esposito
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

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

2022-03-14 Thread Emanuele Giuseppe Esposito
) 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 09/18] block/mirror.c: use of job helpers in drivers to avoid TOC/TOU

2022-03-14 Thread Emanuele Giuseppe Esposito
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

Re: [PATCH v5 18/20] jobs: protect job.aio_context with BQL and job_mutex

2022-03-10 Thread Emanuele Giuseppe Esposito
Am 08/03/2022 um 14:41 schrieb Stefan Hajnoczi: > It's not clear to me that .get_parent_aio_context() should only be > called from the main thread. The API is read-only so someone might try > to call from I/O code in the future expecting it to work like other > read-only graph APIs that are

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

2022-03-10 Thread Emanuele Giuseppe Esposito
Am 08/03/2022 um 15:04 schrieb Stefan Hajnoczi: > On Tue, Feb 08, 2022 at 09:35:12AM -0500, Emanuele Giuseppe Esposito wrote: >> diff --git a/include/qemu/job.h b/include/qemu/job.h >> index ca46e46f5b..574110a1f2 100644 >> --- a/include/qemu/job.h >> +++ b/incl

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

2022-03-09 Thread Emanuele Giuseppe Esposito
Am 02/03/2022 um 12:07 schrieb Vladimir Sementsov-Ogievskiy: > 01.03.2022 17:21, Emanuele Giuseppe Esposito wrote: >> This serie tries to provide a proof of concept and a clear explanation >> on why we need to use drains (and more precisely subtree_drains) >> to replac

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

2022-03-09 Thread Emanuele Giuseppe Esposito
Am 02/03/2022 um 10:47 schrieb Stefan Hajnoczi: > On Tue, Mar 01, 2022 at 09:21:08AM -0500, Emanuele Giuseppe Esposito wrote: >> This serie tries to provide a proof of concept and a clear explanation >> on why we need to use drains (and more precisely subtree_drains

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

2022-02-17 Thread Emanuele Giuseppe Esposito
On 14/02/2022 12:57, Paolo Bonzini wrote: > On 2/14/22 11:27, Emanuele Giuseppe Esposito wrote: >> Anyways, I think that in addition to the fix in this patch, we should >> also fix bdrv_parent_drained_begin_single(poll=true) in >> bdrv_replace_child_noperm, with some

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

2022-02-24 Thread Emanuele Giuseppe Esposito
On 17/02/2022 14:45, Stefan Hajnoczi wrote: > On Tue, Feb 08, 2022 at 09:34:57AM -0500, Emanuele Giuseppe Esposito wrote: >> Instead of having the lock in job_tnx_apply, move it inside > > s/tnx/txn/ > >> in the callback. This will be helpful for next commits, when

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

2022-02-24 Thread Emanuele Giuseppe Esposito
On 17/02/2022 14:45, Stefan Hajnoczi wrote: > On Tue, Feb 08, 2022 at 09:34:57AM -0500, Emanuele Giuseppe Esposito wrote: >> Instead of having the lock in job_tnx_apply, move it inside > > s/tnx/txn/ > >> in the callback. This will be helpful for next commits, when

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

2022-02-24 Thread Emanuele Giuseppe Esposito
On 17/02/2022 15:12, Stefan Hajnoczi wrote: > On Tue, Feb 08, 2022 at 09:34:59AM -0500, 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 >

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

2022-02-24 Thread Emanuele Giuseppe Esposito
On 17/02/2022 15:20, Stefan Hajnoczi wrote: > On Tue, Feb 08, 2022 at 09:35:00AM -0500, Emanuele Giuseppe Esposito wrote: >> static void job_exit(void *opaque) >> { >> Job *job = (Job *)opaque; >> AioContext *ctx; >> +JOB_LOCK_

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

2022-02-24 Thread Emanuele Giuseppe Esposito
On 17/02/2022 16:00, Stefan Hajnoczi wrote: > On Tue, Feb 08, 2022 at 09:35:02AM -0500, Emanuele Giuseppe Esposito wrote: >> diff --git a/blockdev.c b/blockdev.c >> index c5fba4d157..08408cd44b 100644 >> --- a/blockdev.c >> +++ b/blockdev.c &g

Re: [PATCH v5 08/20] jobs: protect jobs with job_lock/unlock

2022-02-24 Thread Emanuele Giuseppe Esposito
On 17/02/2022 15:48, Stefan Hajnoczi wrote: > On Tue, Feb 08, 2022 at 09:35:01AM -0500, Emanuele Giuseppe Esposito wrote: >> diff --git a/block/replication.c b/block/replication.c >> index 55c8f894aa..a03b28726e 100644 >> --- a/block/replication.c >> +++ b/block

Re: [PATCH v5 08/20] jobs: protect jobs with job_lock/unlock

2022-02-24 Thread Emanuele Giuseppe Esposito
On 24/02/2022 17:48, Stefan Hajnoczi wrote: > On Thu, Feb 24, 2022 at 01:45:48PM +0100, Emanuele Giuseppe Esposito wrote: >> >> >> On 17/02/2022 15:48, Stefan Hajnoczi wrote: >>> On Tue, Feb 08, 2022 at 09:35:01AM -0500, Emanuele Giuseppe Esposito wrote: >>&

[RFC PATCH 3/5] block/io.c: introduce bdrv_subtree_drained_{begin/end}_unlocked

2022-03-01 Thread Emanuele Giuseppe Esposito
Same as the locked version, but use BDRV_POLL_UNLOCKED. We are going to add drains to all graph modifications, and they are generally performed without the AioContext lock taken. Signed-off-by: Emanuele Giuseppe Esposito --- block/io.c| 48

[RFC PATCH 5/5] test-bdrv-drain: ensure draining from main loop stops iothreads

2022-03-01 Thread Emanuele Giuseppe Esposito
bdrv_subtree_drained_{begin/end}_unlocked to try avoid using the aiocontext lock as much as possible, since it will eventually go away. Signed-off-by: Emanuele Giuseppe Esposito --- tests/unit/test-bdrv-drain.c | 218 +++ 1 file changed, 218 insertions(+) diff

[RFC PATCH 2/5] introduce BDRV_POLL_WHILE_UNLOCKED

2022-03-01 Thread Emanuele Giuseppe Esposito
Same as BDRV_POLL_WHILE, but uses AIO_WAIT_WHILE_UNLOCKED. See doc comment for more info. Signed-off-by: Emanuele Giuseppe Esposito --- include/block/block.h | 5 + 1 file changed, 5 insertions(+) diff --git a/include/block/block.h b/include/block/block.h index e1713ee306..5a7a850c16

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

2022-03-01 Thread Emanuele Giuseppe Esposito
eries I already sent, and are included here just to allow subtree_drained_begin/end_unlocked implementation. Emanuele Giuseppe Esposito (5): aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED introduce BDRV_POLL_WHILE_UNLOCKED block/io.c: introduce bdrv_subtree_drained_{begin/end}_unlocked child

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

2022-03-01 Thread Emanuele Giuseppe Esposito
I would really love to hear opinions on this, since we already had some discussions on other similar patches. Thank you, Emanuele On 01/03/2022 15:21, Emanuele Giuseppe Esposito wrote: > This serie tries to provide a proof of concept and a clear explanation > on why we need to use

[RFC PATCH 4/5] child_job_drained_poll: override polling condition only when in home thread

2022-03-01 Thread Emanuele Giuseppe Esposito
allow drv->drained_poll() to be called only by the iothread, and not the main loop. We distinguish it by using in_aio_context_home_thread(), that returns false if @ctx is not the same as the thread that runs it. Co-Developed-by: Paolo Bonzini Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 3

[RFC PATCH 1/5] aio-wait.h: introduce AIO_WAIT_WHILE_UNLOCKED

2022-03-01 Thread Emanuele Giuseppe Esposito
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

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

2022-03-03 Thread Emanuele Giuseppe Esposito
Thank you for the script! > > Error: bdrv_get_full_backing_filename() is IO_CODE(), but calls > GLOBAL_STATE_CODE() code >bdrv_get_full_backing_filename() -> bdrv_make_absolute_filename() -> > bdrv_dirname() -> GLOBAL_STATE_CODE() So this was something you caught in the first pass of

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

2022-03-03 Thread Emanuele Giuseppe Esposito
ns are added in the next patch. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 3 + block/meson.build | 7 +- include/block/block-common.h | 418 ++ include/block/block-global-state.h | 252 + include/bl

[PATCH v8 05/31] IO_CODE and IO_OR_GS_CODE for block I/O API

2022-03-03 Thread Emanuele Giuseppe Esposito
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.c | 38 ++- block/dirty-bitmap.c | 1 + block/io.c

[PATCH v8 14/31] block: introduce assert_bdrv_graph_writable

2022-03-03 Thread Emanuele Giuseppe Esposito
. Because adding drains requires additional discussions, they will be added in future series. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 4 include/block/block_int-global-state.h | 17 + 2 files changed, 21 insertions(+) diff --git

[PATCH v8 17/31] block.c: add assertions to static functions

2022-03-03 Thread Emanuele Giuseppe Esposito
Following the assertion derived from the API split, propagate the assertion also in the static functions. Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 46 ++- block/block-backend.c | 3 +++ 2 files changed, 48 insertions(+), 1

[PATCH v8 24/31] block/coroutines: I/O and "I/O or GS" API

2022-03-03 Thread Emanuele Giuseppe Esposito
block coroutines functions run in different aiocontext, and are not protected by the BQL. Therefore are I/O. On the other side, generated_co_wrapper functions use BDRV_POLL_WHILE, meaning the caller can either be the main loop or a specific iothread. Signed-off-by: Emanuele Giuseppe Esposito

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

2022-03-03 Thread Emanuele Giuseppe Esposito
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 v8 09/31] IO_CODE and IO_OR_GS_CODE for block-backend I/O API

2022-03-03 Thread Emanuele Giuseppe Esposito
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 | 58 +++ include/sysemu/block-backend-io.h | 2 ++ 2 files changed, 60 insertions(+) diff --g

[PATCH v8 10/31] block.c: assertions to the block layer permissions API

2022-03-03 Thread Emanuele Giuseppe Esposito
Now that we "covered" the three main cases where the permission API was being used under BQL (fuse, amend and invalidate_cache), we can safely assert for the permission functions implemented in block.c Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 12 1 file c

[PATCH v8 22/31] include/block/snapshot: global state API + assertions

2022-03-03 Thread Emanuele Giuseppe Esposito
Snapshots run also under the BQL, so they all are in the global state API. The aiocontext lock that they hold is currently an overkill and in future could be removed. Signed-off-by: Emanuele Giuseppe Esposito --- block/snapshot.c | 28 include/block

[PATCH v8 04/31] assertions for block global state API

2022-03-03 Thread Emanuele Giuseppe Esposito
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.c| 124

[PATCH v8 06/31] block/export/fuse.c: allow writable exports to take RESIZE permission

2022-03-03 Thread Emanuele Giuseppe Esposito
to set RESIZE, it will be blocked. Also assert in fuse_do_truncate that if we give the RESIZE permission we can then restore the original ones. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Hanna Reitz --- block/export/fuse.c | 25 ++--- 1 file changed, 18 insertions

[PATCH v8 13/31] IO_CODE and IO_OR_GS_CODE for block_int I/O API

2022-03-03 Thread Emanuele Giuseppe Esposito
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.c | 14 +- block/block-backend.c| 2 ++ block/dirty-bitmap.c | 3 +++ block/io.c

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