Re: [Qemu-devel] [PATCH RFC v3 0/2] block/qapi: refactor and optimize the qmp_query_blockstats()

[Markus Armbruster <armbru@redhat.com>]

Dou Liyang <douly.fnst@cn.fujitsu.com> writes:

> Change log v2 -> v3:
>  1. Remove the unnecessary code for the bdrv_next_node().
>  2. Remove the change of the locking rules.
>     Even if this change can improve the performance, but it may
>     effect the consistency.
>
> For the multi-disks guest, we can use the dataplane feature to
> hold performance does not drop, if we execute some slow monitor
> commands, such as "info blockstats". But, without this feature, 
> How to reduce the decline in performance?
>
> These patches aim to refactor the qmp_query_blockstats() and
> improve the performance by reducing the running time of it.
>
> There are the two jobs:
>
> 1 For the performance:
>
> 1.1 the time it takes(ns) in each time:
> the disk numbers     | 10    | 500
> -------------------------------------
> before these patches | 19429 | 667722 
> after these patches  | 18536 | 627945
>
> 1.2 the I/O performance is degraded(%) during the monitor:
>
> the disk numbers     | 10    | 500
> -------------------------------------
> before these patches | 1.3   | 14.2
> after these patches  | 1.0   | 11.3
>
> used the dd command likes this to test: 
> dd if=date_1.dat of=date_2.dat conv=fsync oflag=direct bs=1k count=100k.
>
> 2 refactor qmp_query_blockstats():
>
> From:
>
> +--------------+      +---------------------+
>  | 1            |      | 4.                  |
>  |next_query_bds|      |bdrv_query_bds_stats +---+
>  |              |      |                     |   |
>  +--------^-----+      +-------------^-------+   |
>           |                          |           |
> +---------+----------+      +--------+-------+   |
> | 0.                 |      | 2.             |   |
> |qmp_query_blockstats+------>bdrv_query_stats<----
> |                    |      |                |
> +--------------------+      +--------+-------+
>                                      |
>                        +-------------v-------+
>                        | 3.                  |
>                        |bdrv_query_blk_stats |
>                        |                     |
>                        +---------------------+
>
> To:
>
>                                     +--------------+
>                                     |              |
>                            +--------v-----------+  |
>                        +--->  3.                |  |
> +-------------------+  |   |bdrv_query_bds_stats+--+
> | 1.                +--+   |                    |
> |                   +      +--------------------+
> |qmp_query_blockstats--+
> |                   |  |
> +-------------------+  |   +--------------------+
>                        |   | 2.                 |
>                        +--->                    |
>                            |bdrv_query_blk_stats|
>                            |                    |
>                            +--------------------+

Thanks for the picture, it helps.

Your patches make the code easier to follow, and shorter, too.  The
performance impact is unexpected.  But the code cleanup is worthwhile
even without it.  Please develop this into a non-RFC patch submission.