[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 06/14] block: remove AioContext locking



Am 05.12.2023 um 19:20 hat Stefan Hajnoczi geschrieben:
> This is the big patch that removes
> aio_context_acquire()/aio_context_release() from the block layer and
> affected block layer users.
> 
> There isn't a clean way to split this patch and the reviewers are likely
> the same group of people, so I decided to do it in one patch.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@xxxxxxxxxx>
> Reviewed-by: Eric Blake <eblake@xxxxxxxxxx>
> Reviewed-by: Kevin Wolf <kwolf@xxxxxxxxxx>
> Reviewed-by: Paul Durrant <paul@xxxxxxx>

> diff --git a/migration/block.c b/migration/block.c
> index a15f9bddcb..2bcfcbfdf6 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -313,22 +311,10 @@ static int mig_save_device_bulk(QEMUFile *f, 
> BlkMigDevState *bmds)
>      block_mig_state.submitted++;
>      blk_mig_unlock();
>  
> -    /* We do not know if bs is under the main thread (and thus does
> -     * not acquire the AioContext when doing AIO) or rather under
> -     * dataplane.  Thus acquire both the iothread mutex and the
> -     * AioContext.
> -     *
> -     * This is ugly and will disappear when we make bdrv_* thread-safe,
> -     * without the need to acquire the AioContext.
> -     */
> -    qemu_mutex_lock_iothread();
> -    aio_context_acquire(blk_get_aio_context(bmds->blk));
>      bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * 
> BDRV_SECTOR_SIZE,
>                              nr_sectors * BDRV_SECTOR_SIZE);
>      blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, 
> &blk->qiov,
>                                  0, blk_mig_read_cb, blk);
> -    aio_context_release(blk_get_aio_context(bmds->blk));
> -    qemu_mutex_unlock_iothread();
>  
>      bmds->cur_sector = cur_sector + nr_sectors;
>      return (bmds->cur_sector >= total_sectors);

With this hunk applied, qemu-iotests 183 fails:

(gdb) bt
#0  0x000055aaa7d47c09 in bdrv_graph_co_rdlock () at ../block/graph-lock.c:176
#1  0x000055aaa7d3de2e in graph_lockable_auto_lock (x=<optimized out>) at 
/home/kwolf/source/qemu/include/block/graph-lock.h:215
#2  blk_co_do_preadv_part (blk=0x7f38a4000f30, offset=0, bytes=1048576, 
qiov=0x7f38a40250f0, qiov_offset=qiov_offset@entry=0, flags=0) at 
../block/block-backend.c:1340
#3  0x000055aaa7d3e006 in blk_aio_read_entry (opaque=0x7f38a4025140) at 
../block/block-backend.c:1620
#4  0x000055aaa7e7aa5b in coroutine_trampoline (i0=<optimized out>, 
i1=<optimized out>) at ../util/coroutine-ucontext.c:177
#5  0x00007f38d14dbe90 in __start_context () at /lib64/libc.so.6
#6  0x00007f38b3dfa060 in  ()
#7  0x0000000000000000 in  ()

qemu_get_current_aio_context() returns NULL now. I don't completely
understand why it depends on the BQL, but adding the BQL locking back
fixes it.

Kevin




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.