motorhead/qemu
1
0
Fork 0
mirror of https://github.com/Motorhead1991/qemu.git synced 2025-08-01 23:03:54 -06:00
Code Issues Projects Releases Packages Wiki Activity Actions

block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed

Browse source 
BlockDriver->bdrv_getlength is categorized as IO callback, and it
currently doesn't run in a coroutine. We should let it take a graph
rdlock since the callback traverses the block nodes graph, which however
is only possible in a coroutine.

Therefore turn it into a co_wrapper to move the actual function into a
coroutine where the lock can be taken.

Because now this function creates a new coroutine and polls, we need to
take the AioContext lock where it is missing, for the only reason that
internally co_wrapper calls AIO_WAIT_WHILE and it expects to release the
AioContext lock.

This is especially messy when a co_wrapper creates a coroutine and polls
in bdrv_open_driver, because this function has so many callers in so
many context that it can easily lead to deadlocks. Therefore the new
rule for bdrv_open_driver is that the caller must always hold the
AioContext lock of the given bs (except if it is a coroutine), because
the function calls bdrv_refresh_total_sectors() which is now a
co_wrapper.

Once the rwlock is ultimated and placed in every place it needs to be,
we will poll using AIO_WAIT_WHILE_UNLOCKED and remove the AioContext
lock.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230113204212.359076-7-kwolf@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Emanuele Giuseppe Esposito 2023-01-13 21:42:04 +01:00 • committed by Kevin Wolf
parent c057960c4e
commit c86422c554
35 changed files with 161 additions and 121 deletions
Download patch file Download diff file Expand all files Collapse all files

32
block.c
View file

@ -1035,7 +1035,8 @@ static int find_image_format(BlockBackend *file, const char *filename,
* Set the current 'total_sectors' value
* Return 0 on success, -errno on error.
*/
int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint)
int coroutine_fn bdrv_co_refresh_total_sectors(BlockDriverState *bs,
int64_t hint)
{
BlockDriver *drv = bs->drv;
IO_CODE();
@ -1044,13 +1045,13 @@ int bdrv_refresh_total_sectors(BlockDriverState *bs, int64_t hint)
return -ENOMEDIUM;
}
/* Do not attempt drv->bdrv_getlength() on scsi-generic devices */
/* Do not attempt drv->bdrv_co_getlength() on scsi-generic devices */
if (bdrv_is_sg(bs))
return 0;
/* query actual device if possible, otherwise just trust the hint */
if (drv->bdrv_getlength) {
int64_t length = drv->bdrv_getlength(bs);
if (drv->bdrv_co_getlength) {
int64_t length = drv->bdrv_co_getlength(bs);
if (length < 0) {
return length;
}
@ -1601,6 +1602,11 @@ out:
g_free(gen_node_name);
}
/*
* The caller must always hold @bs AioContext lock, because this function calls
* bdrv_refresh_total_sectors() which polls when called from non-coroutine
* context.
*/
static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name, QDict *options,
int open_flags, Error **errp)
@ -3796,6 +3802,10 @@ out:
* The reference parameter may be used to specify an existing block device which
* should be opened. If specified, neither options nor a filename may be given,
* nor can an existing BDS be reused (that is, *pbs has to be NULL).
*
* The caller must always hold @filename AioContext lock, because this
* function eventually calls bdrv_refresh_total_sectors() which polls
* when called from non-coroutine context.
*/
static BlockDriverState *bdrv_open_inherit(const char *filename,
const char *reference,
@ -4084,6 +4094,11 @@ close_and_fail:
return NULL;
}
/*
* The caller must always hold @filename AioContext lock, because this
* function eventually calls bdrv_refresh_total_sectors() which polls
* when called from non-coroutine context.
*/
BlockDriverState *bdrv_open(const char *filename, const char *reference,
QDict *options, int flags, Error **errp)
{
@ -5800,7 +5815,7 @@ BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,
/**
* Return number of sectors on success, -errno on error.
*/
int64_t bdrv_nb_sectors(BlockDriverState *bs)
int64_t coroutine_fn bdrv_co_nb_sectors(BlockDriverState *bs)
{
BlockDriver *drv = bs->drv;
IO_CODE();
@ -5809,7 +5824,7 @@ int64_t bdrv_nb_sectors(BlockDriverState *bs)
return -ENOMEDIUM;
if (drv->has_variable_length) {
int ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
int ret = bdrv_co_refresh_total_sectors(bs, bs->total_sectors);
if (ret < 0) {
return ret;
}
@ -5821,11 +5836,12 @@ int64_t bdrv_nb_sectors(BlockDriverState *bs)
* Return length in bytes on success, -errno on error.
* The length is always a multiple of BDRV_SECTOR_SIZE.
*/
int64_t bdrv_getlength(BlockDriverState *bs)
int64_t coroutine_fn bdrv_co_getlength(BlockDriverState *bs)
{
int64_t ret = bdrv_nb_sectors(bs);
int64_t ret;
IO_CODE();
ret = bdrv_co_nb_sectors(bs);
if (ret < 0) {
return ret;
}