[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 3/4] block/qapi: acquire a reference inst
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 3/4] block/qapi: acquire a reference instead of a lock during querying blockstats |
Date: |
Mon, 19 Dec 2016 14:34:52 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Mon, Dec 19, 2016 at 04:51:25PM +0800, Dou Liyang wrote:
> This patch works to improve the performance of the query requests.
>
> From the commit 13344f3a, it adds a lock to make query-blockstats
> safe by the aio_context_acquire(). the qmp_query_blockstats func
> requires/releases the AioContext lock, which takes some time and
> blocks the I/O processing. It affects the performance, especially
> in the multi-disks guests.
>
> As the low-level details of block statistics inside QEMU, we can
> acquire a reference instead of the lock.
>
> Signed-off-by: Dou Liyang <address@hidden>
> ---
> block/qapi.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
This patch changes the locking rules for blk_get_stats() (this covers a
lot of fields), bdrv_get_node_name(), and blk_name().
You must document the new locking rules for these fields in
block-backend.h, block_int.h, etc.
> diff --git a/block/qapi.c b/block/qapi.c
> index bc622cd..2262918 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -457,17 +457,21 @@ static BlockStats *bdrv_query_bds_stats(const
> BlockDriverState *bs,
> }
>
> static BlockStats *bdrv_query_stats(BlockBackend *blk,
> - const BlockDriverState *bs,
> + BlockDriverState *bs,
> bool query_backing)
> {
> BlockStats *s;
>
> + bdrv_ref(bs);
> s = bdrv_query_bds_stats(bs, query_backing);
> + bdrv_unref(bs);
>
> if (blk) {
> + blk_ref(blk);
> s->has_device = true;
> s->device = g_strdup(blk_name(blk));
> bdrv_query_blk_stats(s->stats, blk);
> + blk_unref(blk);
This does not look correct. The caller passed in bs and blk so they
must already have a reference. If not, then what protects bs and blk
from deletion before/after this function is called?
> }
>
> return s;
> @@ -523,13 +527,8 @@ BlockStatsList *qmp_query_blockstats(bool
> has_query_nodes,
>
> while (next_query_bds(&blk, &bs, query_nodes)) {
> BlockStatsList *info = g_malloc0(sizeof(*info));
> - AioContext *ctx = blk ? blk_get_aio_context(blk)
> - : bdrv_get_aio_context(bs);
>
> - aio_context_acquire(ctx);
> info->value = bdrv_query_stats(blk, bs, !query_nodes);
> - aio_context_release(ctx);
> -
> *p_next = info;
> p_next = &info->next;
> }
> --
> 2.5.5
>
>
>
>
signature.asc
Description: PGP signature
- [Qemu-devel] [PATCH RFC v2 0/4] block/qapi: refactor and optimize the qmp_query_blockstats(), Dou Liyang, 2016/12/19
- [Qemu-devel] [PATCH RFC v2 3/4] block/qapi: acquire a reference instead of a lock during querying blockstats, Dou Liyang, 2016/12/19
- Re: [Qemu-devel] [PATCH RFC v2 3/4] block/qapi: acquire a reference instead of a lock during querying blockstats,
Stefan Hajnoczi <=
- [Qemu-devel] [PATCH RFC v2 2/4] block/qapi: reduce the coupling between the bdrv_query_stats and bdrv_query_bds_stats, Dou Liyang, 2016/12/19
- [Qemu-devel] [PATCH RFC v2 1/4] block: refactor bdrv_next_node for readability, Dou Liyang, 2016/12/19
- [Qemu-devel] [PATCH RFC v2 4/4] block/qapi: optimize the query function of the blockstats, Dou Liyang, 2016/12/19
- Re: [Qemu-devel] [PATCH RFC v2 0/4] block/qapi: refactor and optimize the qmp_query_blockstats(), Stefan Hajnoczi, 2016/12/19