| Message ID | 1482137486-9843-4-git-send-email-douly.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <douly.fnst@cn.fujitsu.com> > --- > 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 > > > >
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); } 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; }
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 <douly.fnst@cn.fujitsu.com> --- block/qapi.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)