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 <[email protected]> > --- > 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
