[RFC PATCH 1/1] block: add vhost-blk backend

Although QEMU virtio is quite fast, there is still some room for improvements. Disk latency can be reduced if we handle virito-blk requests in host kernel istead of passing them to QEMU. The patch adds vhost-blk backend which sets up vhost-blk kernel module to process requests. test setup and resu

[RFC patch 0/1] block: vhost-blk backend

Although QEMU virtio-blk is quite fast, there is still some room for improvements. Disk latency can be reduced if we handle virito-blk requests in host kernel so we avoid a lot of syscalls and context switches. The biggest disadvantage of this vhost-blk flavor is raw format. Luckily Kirill Thai pr

Re: [PULL 06/18] vfio-user: build library

> On Jul 25, 2022, at 10:50 AM, Daniel P. Berrangé wrote: > > On Mon, Jul 25, 2022 at 02:45:09PM +, Jag Raman wrote: >> Hi Daniel, >> >> We’ve created the following issue to update QEMU’s libvfio-user mirror >> to the latest: >> https://gitlab.com/qemu-project/libvfio-user/-/issues/1 >> >

Re: [PULL 06/18] vfio-user: build library

On Mon, Jul 25, 2022 at 02:45:09PM +, Jag Raman wrote: > Hi Daniel, > > We’ve created the following issue to update QEMU’s libvfio-user mirror > to the latest: > https://gitlab.com/qemu-project/libvfio-user/-/issues/1 > > Will update QEMU’s submodule once this mirror is updated. That sounds

Re: [PULL 06/18] vfio-user: build library

Hi Daniel, We’ve created the following issue to update QEMU’s libvfio-user mirror to the latest: https://gitlab.com/qemu-project/libvfio-user/-/issues/1 Will update QEMU’s submodule once this mirror is updated. Thank you! -- Jag On Jul 21, 2022, at 6:25 AM, Daniel P. Berrangé mailto:berra...@r

[PATCH v2 11/11] block: remove bdrv_try_set_aio_context and replace it with bdrv_try_change_aio_context

No functional change intended. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 14 -- block/export/export.c | 2 +- blockdev.c | 22 +++--- docs/devel/multiple-iothreads.txt | 4 ++-- include/b

[PATCH v2 09/11] block: remove all unused ->can_set_aio_ctx and ->set_aio_ctx callbacks

Together with all _can_set_ and _set_ APIs, as they are not needed anymore. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 196 - block/block-backend.c | 33 - blockjob.c | 35 -- inclu

[PATCH v2 07/11] block-backend: implement .change_aio_ctx in child_root

blk_root_change_aio_ctx() is very similar to blk_root_can_set_aio_ctx(), but implements a new transaction so that if all check pass, the new transaction's .commit will take care of changing the BlockBackend AioContext. blk_root_set_aio_ctx_commit() is the same as blk_root_set_aio_ctx(). Note: bdrv

[PATCH v2 01/11] block.c: assert bs->aio_context is written under BQL and drains

Also here ->aio_context is read by I/O threads and written under BQL. Reviewed-by: Hanna Reitz Signed-off-by: Emanuele Giuseppe Esposito --- block.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block.c b/block.c index 7559965dbc..58a9cfc8b7 100644 --- a/block.c +++ b/block.c @@ -7282,6

[PATCH v2 03/11] bdrv_change_aio_context: use hash table instead of list of visited nodes

Minor performance improvement, but given that we have hash tables available, avoid iterating in the visited nodes list every time just to check if a node has been already visited. The data structure is not actually a proper hash map, but an hash set, as we are just adding nodes and not key,value p

[PATCH v2 02/11] block: use transactions as a replacement of ->{can_}set_aio_context()

Simplify the way the aiocontext can be changed in a BDS graph. There are currently two problems in bdrv_try_set_aio_context: - There is a confusion of AioContext locks taken and released, because we assume that old aiocontext is always taken and new one is taken inside. - It doesn't look very

[PATCH v2 10/11] block: rename bdrv_child_try_change_aio_context in bdrv_try_change_aio_context

No functional changes intended. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 19 --- block/block-backend.c | 3 +-- include/block/block-global-state.h | 12 +--- 3 files changed, 14 insertions(+), 20 deletions(-) diff -

[PATCH v2 08/11] block: use the new _change_ API instead of _can_set_ and _set_

Replace all direct usage of ->can_set_aio_ctx and ->set_aio_ctx, and call bdrv_child_try_change_aio_context() in bdrv_try_set_aio_context(), the main function called through the whole block layer. >From this point onwards, ->can_set_aio_ctx and ->set_aio_ctx won't be used anymore. Signed-off-by:

[PATCH v2 04/11] bdrv_child_try_change_aio_context: add transaction parameter

This enables the caller to use the same transaction to also keep track of aiocontext changes. Signed-off-by: Emanuele Giuseppe Esposito --- block.c| 31 -- include/block/block-global-state.h | 5 + 2 files changed, 30 insertions(+), 6

[PATCH v2 05/11] blockjob: implement .change_aio_ctx in child_job

child_job_change_aio_ctx() is very similar to child_job_can_set_aio_ctx(), but it implements a new transaction so that if all check pass, the new transaction's .commit() will take care of changin the BlockJob AioContext. child_job_set_aio_ctx_commit() is similar to child_job_set_aio_ctx(), but it d

[PATCH v2 06/11] block: implement .change_aio_ctx in child_of_bds

bdrv_child_cb_change_aio_ctx() is identical to bdrv_child_cb_can_set_aio_ctx(), as we only need to recursively go on the parent bs. Note: bdrv_child_try_change_aio_context() is not called by anyone at this point. Reviewed-by: Hanna Reitz Signed-off-by: Emanuele Giuseppe Esposito --- block.c |

[PATCH v2 00/11] Refactor bdrv_try_set_aio_context using transactions

The aim of this series is to reorganize bdrv_try_set_aio_context and drop BDS ->set_aio_context and ->can_set_aio_ctx callbacks in favour of a new one, ->change_aio_ctx. More informations in patch 3 (which is also RFC, due to the doubts I have with AioContext locks). Patch 1 just add assertions i

Re: [RFC PATCH 3/8] RFC: block: use transactions as a replacement of ->{can_}set_aio_context()

Am 20/07/2022 um 16:09 schrieb Vladimir Sementsov-Ogievskiy: > On 7/13/22 00:19, Emanuele Giuseppe Esposito wrote: >> +/* >> + * @visited will accumulate all visited BdrvChild object. The caller is >> + * responsible for freeing the list afterwards. >> + */ >> +bool bdrv_change_aio_context(Block

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

Change the job_{lock/unlock} and macros to use job_mutex. Now that they are not nop anymore, remove the aiocontext to avoid deadlocks. Therefore: - when possible, remove completely the aiocontext lock/unlock pair - if it is used by some other function too, reduce the locking section as much as

[PATCH v10 05/21] job.c: add job_lock/unlock while keeping job.h intact

With "intact" we mean that all job.h functions implicitly take the lock. Therefore API callers are unmodified. This means that: - many static functions that will be always called with job lock held become _locked, and call _locked functions - all public functions take the lock internally if need

[PATCH v10 20/21] blockjob: remove unused functions

These public functions are not used anywhere, thus can be dropped. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Stefan Hajnoczi --- blockjob.c | 16 ++-- include/block/blockjob.h | 31 --- 2 files changed, 14 insertions(+), 33 dele

[PATCH v10 19/21] 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: Vladimir Sementsov-Ogievskiy Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- blockj

[PATCH v10 17/21] blockjob: protect iostatus field in BlockJob struct

iostatus is the only field (together with .job) that needs protection using the job mutex. It is set in the main loop (GLOBAL_STATE functions) but read in I/O code (block_job_error_action). In order to protect it, change block_job_iostatus_set_err to block_job_iostatus_set_err_locked(), always ca

[PATCH v10 21/21] job: remove unused functions

These public functions are not used anywhere, thus can be dropped. Also, since this is the final job API that doesn't use AioContext lock and replaces it with job_lock, adjust all remaining function documentation to clearly specify if the job lock is taken or not. Signed-off-by: Emanuele Giuseppe

[PATCH v10 12/21] 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: Vladimir Sementsov-Ogievskiy Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele G

[PATCH v10 16/21] blockjob: rename notifier callbacks as _locked

They all are called with job_lock held, in job_event_*_locked() Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Stefan Hajnoczi Signed-off-by: Emanuele Giuseppe Esposito --- blockjob.c | 25 +++-- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/blockj

[PATCH v10 15/21] blockjob.h: categorize fields in struct BlockJob

The same job lock is being used also to protect some of blockjob fields. Categorize them just as done in job.h. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Emanuele Giuseppe Esposito --- include/block/blockjob.h | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(

[PATCH v10 13/21] 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 v10 06/21] job: move and update comments from blockjob.c

This comment applies more on job, it was left in blockjob as in the past the whole job logic was implemented there. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. No functional change intended. Reviewed-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Emanuele Giusep

[PATCH v10 03/21] 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 Reviewed-by: Stefan Hajnoczi Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/qemu/job.h | 18 -- job.c

[PATCH v10 14/21] jobs: protect job.aio_context with BQL and job_mutex

In order to make it thread safe, implement a "fake rwlock", where we allow reads under BQL *or* job_mutex held, but writes only under BQL *and* job_mutex. The only write we have is in child_job_set_aio_ctx, which always happens under drain (so the job is paused). For this reason, introduce job_set

[PATCH v10 10/21] 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*. Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by:

[PATCH v10 09/21] jobs: use job locks also in the unit tests

Add missing job synchronization in the unit tests, with explicit locks. We are deliberately using _locked functions wrapped by a guard instead of a normal call because the normal call will be removed in future, as the only usage is limited to the tests. In other words, if a function like job_paus

[PATCH v10 11/21] jobs: group together API calls under the same job lock

Now that the API offers also _locked() functions, take advantage of it and give also the caller control to take the lock and call _locked functions. This makes sense especially when we have for loops, because it makes no sense to have: for(job = job_next(); ...) where each job_next() takes the l

[PATCH v10 08/21] jobs: add job lock in find_* functions

Both blockdev.c and job-qmp.c have TOC/TOU conditions, because they first search for the job and then perform an action on it. Therefore, we need to do the search + action under the same job mutex critical section. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Review

[PATCH v10 04/21] 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 Reviewed-by: Stefan Hajnoczi Reviewed-by: Vladimir Sementsov-Ogie

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

job mutex will be used to protect the job struct elements and list, replacing AioContext locks. Right now use a shared lock for all jobs, in order to keep things simple. Once the AioContext lock is gone, we can introduce per-job locks. To simplify the switch from aiocontext to job lock, introduce

[PATCH v10 07/21] blockjob: introduce block_job _locked() APIs

Just as done with job.h, create _locked() functions in blockjob.h These functions will be later useful when caller has already taken the lock. All blockjob _locked functions call job _locked functions. Note: at this stage, job_{lock/unlock} and job lock guard macros are *nop*. Reviewed-by: Vladi

[PATCH v10 02/21] 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 Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/qemu/job.h | 61 +++--- 1 file changed, 36

[PATCH v10 00/21] job: replace AioContext lock with job_mutex

In this series, we want to remove the AioContext lock and instead use the already existent job_mutex to protect the job structures and list. This is part of the work to get rid of AioContext lock usage in favour of smaller granularity locks. In order to simplify reviewer's job, job lock/unlock fun