Skip to content

Commit

Permalink
block: pass size to bdrv_unregister_buf()
Browse files Browse the repository at this point in the history
The only implementor of bdrv_register_buf() is block/nvme.c, where the
size is not needed when unregistering a buffer. This is because
util/vfio-helpers.c can look up mappings by address.

Future block drivers that implement bdrv_register_buf() may not be able
to do their job given only the buffer address. Add a size argument to
bdrv_unregister_buf().

Also document the assumptions about
bdrv_register_buf()/bdrv_unregister_buf() calls. The same <host, size>
values that were given to bdrv_register_buf() must be given to
bdrv_unregister_buf().

gcc 11.2.1 emits a spurious warning that img_bench()'s buf_size local
variable might be uninitialized, so it's necessary to silence the
compiler.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Message-id: 20221013185908.1297568-5-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
  • Loading branch information
stefanhaRH committed Oct 26, 2022
1 parent 1f0fea3 commit 4f38401
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 11 deletions.
4 changes: 2 additions & 2 deletions block/block-backend.c
Expand Up @@ -2551,10 +2551,10 @@ void blk_register_buf(BlockBackend *blk, void *host, size_t size)
bdrv_register_buf(blk_bs(blk), host, size);
}

void blk_unregister_buf(BlockBackend *blk, void *host)
void blk_unregister_buf(BlockBackend *blk, void *host, size_t size)
{
GLOBAL_STATE_CODE();
bdrv_unregister_buf(blk_bs(blk), host);
bdrv_unregister_buf(blk_bs(blk), host, size);
}

int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,
Expand Down
6 changes: 3 additions & 3 deletions block/io.c
Expand Up @@ -3275,16 +3275,16 @@ void bdrv_register_buf(BlockDriverState *bs, void *host, size_t size)
}
}

void bdrv_unregister_buf(BlockDriverState *bs, void *host)
void bdrv_unregister_buf(BlockDriverState *bs, void *host, size_t size)
{
BdrvChild *child;

GLOBAL_STATE_CODE();
if (bs->drv && bs->drv->bdrv_unregister_buf) {
bs->drv->bdrv_unregister_buf(bs, host);
bs->drv->bdrv_unregister_buf(bs, host, size);
}
QLIST_FOREACH(child, &bs->children, next) {
bdrv_unregister_buf(child->bs, host);
bdrv_unregister_buf(child->bs, host, size);
}
}

Expand Down
2 changes: 1 addition & 1 deletion block/nvme.c
Expand Up @@ -1602,7 +1602,7 @@ static void nvme_register_buf(BlockDriverState *bs, void *host, size_t size)
}
}

static void nvme_unregister_buf(BlockDriverState *bs, void *host)
static void nvme_unregister_buf(BlockDriverState *bs, void *host, size_t size)
{
BDRVNVMeState *s = bs->opaque;

Expand Down
5 changes: 4 additions & 1 deletion include/block/block-global-state.h
Expand Up @@ -243,9 +243,12 @@ void bdrv_del_child(BlockDriverState *parent, BdrvChild *child, Error **errp);
* Register/unregister a buffer for I/O. For example, VFIO drivers are
* interested to know the memory areas that would later be used for I/O, so
* that they can prepare IOMMU mapping etc., to get better performance.
*
* Buffers must not overlap and they must be unregistered with the same <host,
* size> values that they were registered with.
*/
void bdrv_register_buf(BlockDriverState *bs, void *host, size_t size);
void bdrv_unregister_buf(BlockDriverState *bs, void *host);
void bdrv_unregister_buf(BlockDriverState *bs, void *host, size_t size);

void bdrv_cancel_in_flight(BlockDriverState *bs);

Expand Down
2 changes: 1 addition & 1 deletion include/block/block_int-common.h
Expand Up @@ -435,7 +435,7 @@ struct BlockDriver {
* DMA mapping for hot buffers.
*/
void (*bdrv_register_buf)(BlockDriverState *bs, void *host, size_t size);
void (*bdrv_unregister_buf)(BlockDriverState *bs, void *host);
void (*bdrv_unregister_buf)(BlockDriverState *bs, void *host, size_t size);

/*
* This field is modified only under the BQL, and is part of
Expand Down
2 changes: 1 addition & 1 deletion include/sysemu/block-backend-global-state.h
Expand Up @@ -107,7 +107,7 @@ void blk_io_limits_update_group(BlockBackend *blk, const char *group);
void blk_set_force_allow_inactivate(BlockBackend *blk);

void blk_register_buf(BlockBackend *blk, void *host, size_t size);
void blk_unregister_buf(BlockBackend *blk, void *host);
void blk_unregister_buf(BlockBackend *blk, void *host, size_t size);

const BdrvChild *blk_root(BlockBackend *blk);

Expand Down
4 changes: 2 additions & 2 deletions qemu-img.c
Expand Up @@ -4371,7 +4371,7 @@ static int img_bench(int argc, char **argv)
struct timeval t1, t2;
int i;
bool force_share = false;
size_t buf_size;
size_t buf_size = 0;

for (;;) {
static const struct option long_options[] = {
Expand Down Expand Up @@ -4593,7 +4593,7 @@ static int img_bench(int argc, char **argv)

out:
if (data.buf) {
blk_unregister_buf(blk, data.buf);
blk_unregister_buf(blk, data.buf, buf_size);
}
qemu_vfree(data.buf);
blk_unref(blk);
Expand Down

0 comments on commit 4f38401

Please sign in to comment.